[Lua API] Non-blocking subprocess runner #675
Conversation
|
This sounds promising. Is this similar to how vim8 jobs / channels works? |
|
I am not quite familiar with vim channels, but at quick glance it looks like there are some similarities indeed. All data is being sent to a process asyncroniously without waiting for response like it is in vim, although instead of passing a callback for handling the process answers my solution utilizes the event system already implemented in vis. |
lua/vis.lua
Outdated
| @@ -152,6 +152,7 @@ local events = { | |||
| WIN_OPEN = "Event::WIN_OPEN", -- see @{win_open} | |||
| WIN_STATUS = "Event::WIN_STATUS", -- see @{win_status} | |||
| TERM_CSI = "Event::TERM_CSI", -- see @{term_csi} | |||
| PROCESS_RESPONCE = "Event::PROCESS_RESPONCE", -- see @{process_responce} | |||
There was a problem hiding this comment.
Is responce perhaps a typo of the word response?
There was a problem hiding this comment.
It is. Thank you for noticing!
|
Would it be possible to rebase this patch on the top of the current |
Done. Hopefully I did not break anything. |
|
@xomachine see the linked issue report: this patch is suspected to be the cause of the crash. |
|
Any updates on this? My understanding is that this is needed to implement LSP, and losing LSP without manually patching is a big point for many that may prevent switching to vis (e.g. it is the sole reason why I haven't switched from nvim). |
There was a problem hiding this comment.
Merged into https://git.sr.ht/~mcepl/vis/tree and after more than half of year of testing, it seems to be working.
|
Hmm, when I pulled in also new tests, I got this one test failing on the aarch64 architecture (not on Intels, there it works just fine): |
|
That's weird. My only guess is different behavior of the |
|
I am thinking how to persuade people in charge here (@martanne , @rnpnr , @ninewise ) to review this pull request, and I have to admit that when I tried myself, I don’t find the task easy. Would it be possible, please, to reorganize the patches to be topical ones (something like this or like this), i.e., each change should contain all changes in one area? Commits like “Fixes of …” or “Correct implementation of …” just make reviewer’s life more complicated and less willing to help. |
|
That would definitely be appreciated. There seems to be a lot of commits that should be squashed together at a minimum. I do want to get around to reviewing this one since I think it has the potential to be very useful but I've been trying to work through some less complicated things first. |
Rationale A modern text editor usually includes tools for helping user to avoid mistakes in texts. Those tools include spell checkers and programming language integrations. Though vis explicitly states that the full featured IDE is not a goal, implementing some of the tools might be achieved using its Lua API. Unfortunatelly the API misses the ability to start a process and to perform a communication with it without completely blocking the editor UI, which is crucial for any tool that performs background tracking of the inserted text (e. g. language servers). Implementation details New feature introduces new API method: communicate. The method start a new process and returns a handle to communicate with the process instantly. The patch inserts stderr and stdout file descriptors of the process to the pselect call of the main loop used for reading user input to track the process state without blocking the main loop until the process is finished. Any changes in the process state cause the iteration of the main loop and are being exposed to the Lua API as new event: PROCESS_RESPONSE.
|
I guess the best thing I can do with this history is squashing everything into one commit and rewriting the commit message. There are no logical changes in this patch set, it is just a trial-and-error story of implementing the feature. |
|
Well, if I do diff --git a/Makefile b/Makefile
index c862643..4c473bf 100644
--- a/Makefile
+++ b/Makefile
@@ -26,6 +26,7 @@ SRC = array.c \
vis-prompt.c \
vis-registers.c \
vis-text-objects.c \
+ vis-subprocess.c \
$(REGEX_SRC)
ELF = vis vis-menu vis-digraph
diff --git a/lua/vis.lua b/lua/vis.lua
index 39649c1..f2f9421 100644
--- a/lua/vis.lua
+++ b/lua/vis.lua
@@ -152,6 +152,7 @@ local events = {
WIN_OPEN = "Event::WIN_OPEN", -- see @{win_open}
WIN_STATUS = "Event::WIN_STATUS", -- see @{win_status}
TERM_CSI = "Event::TERM_CSI", -- see @{term_csi}
+ PROCESS_RESPONSE = "Event::PROCESS_RESPONSE", -- see @{process_response}
}
events.file_close = function(...) events.emit(events.FILE_CLOSE, ...) end
@@ -167,6 +168,7 @@ events.win_highlight = function(...) events.emit(events.WIN_HIGHLIGHT, ...) end
events.win_open = function(...) events.emit(events.WIN_OPEN, ...) end
events.win_status = function(...) events.emit(events.WIN_STATUS, ...) end
events.term_csi = function(...) events.emit(events.TERM_CSI, ...) end
+events.process_response = function(...) events.emit(events.PROCESS_RESPONSE, ...) end
local handlers = {}
diff --git a/vis-lua.c b/vis-lua.c
index 9bf5629..1bfeabb 100644
--- a/vis-lua.c
+++ b/vis-lua.c
@@ -23,6 +23,7 @@
#include "vis-lua.h"
#include "vis-core.h"
+#include "vis-subprocess.h"
#include "text-motions.h"
#include "util.h"
@@ -52,6 +53,13 @@
#define debug(...) do { } while (0)
#endif
+typedef struct {
+ /* Lua stream structure for the process input stream */
+ FILE *f;
+ lua_CFunction closef;
+ Process *handler;
+} ProcessStream;
+
static void window_status_update(Vis *vis, Win *win) {
char left_parts[4][255] = { "", "", "", "" };
char right_parts[4][32] = { "", "", "", "" };
@@ -162,6 +170,9 @@ void vis_lua_win_close(Vis *vis, Win *win) { }
void vis_lua_win_highlight(Vis *vis, Win *win) { }
void vis_lua_win_status(Vis *vis, Win *win) { window_status_update(vis, win); }
void vis_lua_term_csi(Vis *vis, const long *csi) { }
+void vis_lua_process_response(Vis *vis, const char *name,
+ char *buffer, size_t len, ResponseType rtype) { }
+
#else
@@ -1367,6 +1378,47 @@ static int redraw(lua_State *L) {
vis_redraw(vis);
return 0;
}
+/***
+ * Closes a stream returned by @{Vis.communicate}.
+ *
+ * @function close
+ * @tparam io.file inputfd the stream to be closed
+ * @treturn bool the same with @{io.close}
+ */
+static int close_subprocess(lua_State *L) {
+ luaL_Stream *file = luaL_checkudata(L, -1, "FILE*");
+ int result = fclose(file->f);
+ if (result == 0) {
+ file->f = NULL;
+ file->closef = NULL;
+ }
+ return luaL_fileresult(L, result == 0, NULL);
+}
+/***
+ * Open new process and return its input handler.
+ * When the process will quit or will output anything to stdout or stderr,
+ * the @{process_response} event will be fired.
+ *
+ * The editor core won't be blocked while the external process is running.
+ *
+ * @function communicate
+ * @tparam string name the name of subprocess (to distinguish processes in the @{process_response} event)
+ * @tparam string command the command to execute
+ * @return the file handle to write data to the process, in case of error the return values are equivalent to @{io.open} error values.
+ */
+static int communicate_func(lua_State *L) {
+ Vis *vis = obj_ref_check(L, 1, "vis");
+ const char *name = luaL_checkstring(L, 2);
+ const char *cmd = luaL_checkstring(L, 3);
+ ProcessStream *inputfd = (ProcessStream *)lua_newuserdata(L, sizeof(ProcessStream));
+ luaL_setmetatable(L, LUA_FILEHANDLE);
+ inputfd->handler = vis_process_communicate(vis, name, cmd, (void **)(&(inputfd->closef)));
+ if (inputfd->handler) {
+ inputfd->f = fdopen(inputfd->handler->inpfd, "w");
+ inputfd->closef = &close_subprocess;
+ }
+ return inputfd->f ? 1 : luaL_fileresult(L, inputfd->f != NULL, name);
+}
/***
* Currently active window.
* @tfield Window win
@@ -1524,6 +1576,7 @@ static const struct luaL_Reg vis_lua[] = {
{ "exit", exit_func },
{ "pipe", pipe_func },
{ "redraw", redraw },
+ { "communicate", communicate_func },
{ "__index", vis_index },
{ "__newindex", vis_newindex },
{ NULL, NULL },
@@ -3148,5 +3201,34 @@ void vis_lua_term_csi(Vis *vis, const long *csi) {
}
lua_pop(L, 1);
}
+/***
+ * The response received from the process started via @{Vis:communicate}.
+ * @function process_response
+ * @tparam string name the name of process given to @{Vis:communicate}
+ * @tparam string response_type can be "STDOUT" or "STDERR" if new output was received in corresponding channel, "SIGNAL" if the process was terminated by a signal or "EXIT" when the process terminated normally
+ * @tparam string|int buffer the available content sent by process; it becomes the exit code number if response\_type is "EXIT", or the signal number if response\_type is "SIGNAL"
+ */
+void vis_lua_process_response(Vis *vis, const char *name,
+ char *buffer, size_t len, ResponseType rtype) {
+ lua_State *L = vis->lua;
+ if (!L)
+ return;
+ vis_lua_event_get(L, "process_response");
+ if (lua_isfunction(L, -1)) {
+ lua_pushstring(L, name);
+ if (rtype == EXIT || rtype == SIGNAL)
+ lua_pushinteger(L, len);
+ else
+ lua_pushlstring(L, buffer, len);
+ switch (rtype){
+ case STDOUT: lua_pushstring(L, "STDOUT"); break;
+ case STDERR: lua_pushstring(L, "STDERR"); break;
+ case SIGNAL: lua_pushstring(L, "SIGNAL"); break;
+ case EXIT: lua_pushstring(L, "EXIT"); break;
+ }
+ pcall(vis, L, 3, 0);
+ }
+ lua_pop(L, 1);
+}
#endif
diff --git a/vis-lua.h b/vis-lua.h
index da64233..914f590 100644
--- a/vis-lua.h
+++ b/vis-lua.h
@@ -7,10 +7,11 @@
#include <lauxlib.h>
#else
typedef struct lua_State lua_State;
+typedef void* lua_CFunction;
#endif
#include "vis.h"
-
+#include "vis-subprocess.h"
/* add a directory to consider when loading lua files */
bool vis_lua_path_add(Vis*, const char *path);
/* get semicolon separated list of paths to load lua files
@@ -38,5 +39,6 @@ void vis_lua_win_close(Vis*, Win*);
void vis_lua_win_highlight(Vis*, Win*);
void vis_lua_win_status(Vis*, Win*);
void vis_lua_term_csi(Vis*, const long *);
+void vis_lua_process_response(Vis *, const char *, char *, size_t, ResponseType);
#endif
diff --git a/vis-subprocess.c b/vis-subprocess.c
new file mode 100644
index 0000000..fa8f7cd
--- /dev/null
+++ b/vis-subprocess.c
@@ -0,0 +1,176 @@
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/wait.h>
+#include "vis-lua.h"
+#include "vis-subprocess.h"
+
+/* Maximum amount of data what can be read from IPC pipe per event */
+#define MAXBUFFER 1024
+
+/* Pool of information about currently running subprocesses */
+static Process *process_pool;
+
+Process *new_in_pool() {
+ /* Adds new empty process information structure to the process pool and
+ * returns it */
+ Process *newprocess = (Process *)malloc(sizeof(Process));
+ if (!newprocess) return NULL;
+ newprocess->next = process_pool;
+ process_pool = newprocess;
+ return newprocess;
+}
+
+void destroy(Process **pointer) {
+ /* Removes the subprocess information from the pool, sets invalidator to NULL
+ * and frees resources. */
+ Process *target = *pointer;
+ if (target->outfd != -1) close(target->outfd);
+ if (target->errfd != -1) close(target->errfd);
+ if (target->inpfd != -1) close(target->inpfd);
+ /* marking stream as closed for lua */
+ if (target->invalidator) *(target->invalidator) = NULL;
+ if (target->name) free(target->name);
+ *pointer = target->next;
+ free(target);
+}
+
+Process *vis_process_communicate(Vis *vis, const char *name,
+ const char *command, void **invalidator) {
+ /* Starts new subprocess by passing the `command` to the shell and
+ * returns the subprocess information structure, containing file descriptors
+ * of the process.
+ * Also stores the subprocess information to the internal pool to track
+ * its status and responses.
+ * `name` - the string than should contain an unique name of the subprocess.
+ * This name will be passed to the PROCESS_RESPONSE event handler
+ * to distinguish running subprocesses.
+ * `invalidator` - a pointer to the pointer which shows that the subprocess
+ * is invalid when set to NULL. When subprocess dies, it is being set to NULL.
+ * If the pointer is set to NULL by an external code, the subprocess will be
+ * killed on the next main loop iteration. */
+ int pin[2], pout[2], perr[2];
+ pid_t pid = (pid_t)-1;
+ if (pipe(perr) == -1) goto closeerr;
+ if (pipe(pout) == -1) goto closeouterr;
+ if (pipe(pin) == -1) goto closeall;
+ pid = fork();
+ if (pid == -1)
+ vis_info_show(vis, "fork failed: %s", strerror(errno));
+ else if (pid == 0){ /* child process */
+ sigset_t sigterm_mask;
+ sigemptyset(&sigterm_mask);
+ sigaddset(&sigterm_mask, SIGTERM);
+ if (sigprocmask(SIG_UNBLOCK, &sigterm_mask, NULL) == -1) {
+ fprintf(stderr, "failed to reset signal mask");
+ exit(EXIT_FAILURE);
+ }
+ dup2(pin[0], STDIN_FILENO);
+ dup2(pout[1], STDOUT_FILENO);
+ dup2(perr[1], STDERR_FILENO);
+ }
+ else { /* main process */
+ Process *new = new_in_pool();
+ if (!new) {
+ vis_info_show(vis, "Can not create process: %s", strerror(errno));
+ goto closeall;
+ }
+ new->name = strdup(name);
+ if (!new->name) {
+ vis_info_show(vis, "Can not copy process name: %s", strerror(errno));
+ /* pop top element (which is `new`) from the pool */
+ destroy(&process_pool);
+ goto closeall;
+ }
+ new->outfd = pout[0];
+ new->errfd = perr[0];
+ new->inpfd = pin[1];
+ new->pid = pid;
+ new->invalidator = invalidator;
+ close(pin[0]);
+ close(pout[1]);
+ close(perr[1]);
+ return new;
+ }
+closeall:
+ close(pin[0]);
+ close(pin[1]);
+closeouterr:
+ close(pout[0]);
+ close(pout[1]);
+closeerr:
+ close(perr[0]);
+ close(perr[1]);
+ if (pid == 0) { /* start command in child process */
+ execlp(vis->shell, vis->shell, "-c", command, (char*)NULL);
+ fprintf(stderr, "exec failed: %s(%d)\n", strerror(errno), errno);
+ exit(1);
+ }
+ else
+ vis_info_show(vis, "process creation failed: %s", strerror(errno));
+ return NULL;
+}
+
+int vis_process_before_tick(fd_set *readfds) {
+ /* Adds file descriptors of currently running subprocesses to the `readfds`
+ * to track their readiness and returns maximum file descriptor value
+ * to pass it to the `pselect` call */
+ Process **pointer = &process_pool;
+ int maxfd = 0;
+ while (*pointer) {
+ Process *current = *pointer;
+ if (current->outfd != -1) {
+ FD_SET(current->outfd, readfds);
+ maxfd = maxfd < current->outfd ? current->outfd : maxfd;
+ }
+ if (current->errfd != -1) {
+ FD_SET(current->errfd, readfds);
+ maxfd = maxfd < current->errfd ? current->errfd : maxfd;
+ }
+ pointer = ¤t->next;
+ }
+ return maxfd;
+}
+
+void read_and_fire(Vis* vis, int fd, const char *name, ResponseType rtype) {
+ /* Reads data from the given subprocess file descriptor `fd` and fires
+ * the PROCESS_RESPONSE event in Lua with given subprocess `name`,
+ * `rtype` and the read data as arguments. */
+ static char buffer[MAXBUFFER];
+ size_t obtained = read(fd, &buffer, MAXBUFFER-1);
+ if (obtained > 0)
+ vis_lua_process_response(vis, name, buffer, obtained, rtype);
+}
+
+void vis_process_tick(Vis *vis, fd_set *readfds) {
+ /* Checks if `readfds` contains file discriptors of subprocesses from
+ * the pool. If so, reads the data from them and fires corresponding events.
+ * Also checks if subprocesses from pool is dead or need to be killed then
+ * raises event or kills it if necessary. */
+ Process **pointer = &process_pool;
+ while (*pointer) {
+ Process *current = *pointer;
+ if (current->outfd != -1 && FD_ISSET(current->outfd, readfds))
+ read_and_fire(vis, current->outfd, current->name, STDOUT);
+ if (current->errfd != -1 && FD_ISSET(current->errfd, readfds))
+ read_and_fire(vis, current->errfd, current->name, STDERR);
+ int status;
+ pid_t wpid = waitpid(current->pid, &status, WNOHANG);
+ if (wpid == -1) vis_message_show(vis, strerror(errno));
+ else if (wpid == current->pid) goto just_destroy;
+ else if(!*(current->invalidator)) goto kill_and_destroy;
+ pointer = ¤t->next;
+ continue;
+kill_and_destroy:
+ kill(current->pid, SIGTERM);
+ waitpid(current->pid, &status, 0);
+just_destroy:
+ if (WIFSIGNALED(status))
+ vis_lua_process_response(vis, current->name, NULL, WTERMSIG(status), SIGNAL);
+ else
+ vis_lua_process_response(vis, current->name, NULL, WEXITSTATUS(status), EXIT);
+ destroy(pointer);
+ }
+}
diff --git a/vis-subprocess.h b/vis-subprocess.h
new file mode 100644
index 0000000..ae25e21
--- /dev/null
+++ b/vis-subprocess.h
@@ -0,0 +1,23 @@
+#ifndef VIS_SUBPROCESS_H
+#define VIS_SUBPROCESS_H
+#include "vis-core.h"
+#include <sys/select.h>
+
+struct Process {
+ char *name;
+ int outfd;
+ int errfd;
+ int inpfd;
+ pid_t pid;
+ void **invalidator;
+ struct Process *next;
+};
+
+typedef struct Process Process;
+typedef enum { STDOUT, STDERR, SIGNAL, EXIT } ResponseType;
+
+Process *vis_process_communicate(Vis *, const char *command, const char *name,
+ void **invalidator);
+int vis_process_before_tick(fd_set *);
+void vis_process_tick(Vis *, fd_set *);
+#endif
diff --git a/vis.c b/vis.c
index f21efa8..24e7f65 100644
--- a/vis.c
+++ b/vis.c
@@ -28,6 +28,7 @@
#include "vis-core.h"
#include "sam.h"
#include "ui.h"
+#include "vis-subprocess.h"
static void macro_replay(Vis *vis, const Macro *macro);
@@ -1429,7 +1430,8 @@ int vis_run(Vis *vis) {
vis_update(vis);
idle.tv_sec = vis->mode->idle_timeout;
- int r = pselect(1, &fds, NULL, NULL, timeout, &emptyset);
+ int r = pselect(vis_process_before_tick(&fds) + 1, &fds, NULL, NULL,
+ timeout, &emptyset);
if (r == -1 && errno == EINTR)
continue;
@@ -1437,6 +1439,7 @@ int vis_run(Vis *vis) {
/* TODO save all pending changes to a ~suffixed file */
vis_die(vis, "Error in mainloop: %s\n", strerror(errno));
}
+ vis_process_tick(vis, &fds);
if (!FD_ISSET(STDIN_FILENO, &fds)) {
if (vis->mode->idle)That’s 424 line, so I hoped whether you can get some organization to this. |
|
I am still not a real C programmer, so just one (probably nonsensical) comment:
|
Maybe I missed the point, but according to the links you provided, the patch should implement one feature - no more, no less. So there is no point in splitting the patch to multiple parts that will lead to inability to compile the editor on some intermediate commit or to the dead code. On other hand I can try to implement partially working feature. E. g. in the first commit only ability to run a process and send data to its stdin, the second one - event handling, the third - ability to stop a process by closing its handle and so on... But I doubt that it will be more readable and is a good practice overall.
I always try to keep headers as small as possible because their content is included to many compilation units therefore increases build time. So if the definition is not needed outside one compilation unit, I put it into a source file. It is especially important in C++ with its monstrous templates and long compile times, and I thought that it also a good practice in C. |
|
I agree. I don't see anyway of splitting this up into subcommits. I also don't think Is there a minimal example of some lua code that makes use the exposed features? |
One example can be found in the test code (https://github.com/martanne/vis-test/pull/12/files#diff-8e2c47bd7e90844a45b8b2e5c9c6995fc3431086494baf17617c924dca8abf8dR67) |
|
There are some smaller things that should be changed too. For example, it's unnecessary to cast the result of a malloc(3) call. Empty parameter lists should be |
Just for the record that’s not exactly what those links meant (AFAIK): the main point is that instead of the chronological commits (“Let’s try this …”, “Whoops, that didn’t work …”, “It’s Friday, saving my week work before the weekend” etc.) the branch for review should be organized topically (“necessary changes in C making the ground for everything else”, “implementation of Lua interface”, “documentation”, “tests” or something like it). How big or small those topics are and how many commits should be in the branch is completely orthogonal to this organization. And yes, 400+ lines patch is probably somewhere on the edge of being small enough not to split it at all. |
* Place curly braces around all one-line if statements * Replace while with for
|
Hopefully I answered on all review comments (by commenting or committing). @rnpnr Is there anything I missed? |
I think you have corrected most of the issues but there are still a
If anyone disagrees with me feel free to speak up. This is just what |
|
Also let me just reiterate: The most important thing about this patch for me is the interface. As long |
vis-lua.c
Outdated
| static int close_subprocess(lua_State *L) { | ||
| luaL_Stream *file = luaL_checkudata(L, -1, "FILE*"); | ||
| if (file) { | ||
| lua_pop(L, 1); |
There was a problem hiding this comment.
I'm sorry for making you do this again but the lua_pop() is actually
incorrect. You just need:
if (!file)
return luaL_argerror(...)
After reading obj_ref_get() in more detail I noticed it was modifying
the lua stack. This wasn't documented where it should be. It won't
matter to you but I am going to either add a comment or change it to
not do that so that I don't fall into that trap again.
There was a problem hiding this comment.
I looked to the Lua source code and now I am not even sure that remaining condition is necessary. The luaL_checkudata function never returns NULL according to its source code.
It calls luaL_testudata which does return NULL, but after that it calls a macro luaL_argexpected which will call the luaL_typeerror function and eventually luaL_argerror with proper error message when the return value is NULL.
There was a problem hiding this comment.
Yep you are right. Sorry for the wild goose chase. I'm also pretty new
to the Lua-C API.
I do wish the documentation was a little more specific. The only indication
of this is here:
... luaL_check* or luaL_opt*. All of these functions throw an error if the check is not satisfied.
Which doesn't really tell you that the function doesn't return in that case.
|
If we are going to break the interface anyway, let's at least make it more convenient. So I would like to get another proposals for better interface. One of them was about performing reading the spawned process notifications via callback functions. Would it be better to rewrite my patch to implement this interface? Or may be there are another ideas? |
|
I guess it depends on what you mean by callback functions. I'm all |
Actually, if we are changing API of |
I think this would be much more powerful but it is also a little more My original idea wasn't really fleshed out but I think one possibility
@xomachine what are your thoughts? I think what you have already wrote |
It could be achieved in multiple ways but all of them have disadvantages.
In my opinion the first way does not make any significant improvement in interface due to inability to call vis API. The second one is quite dangerous and error prone as any multithreading. So the third way looks reliable, but requires full rewrite of the patch (basically writing new patch) and I can not tell when it will be ready. Or we can concentrate on improvement of the process spawning and add callbacks instead of events to get rid of the process naming. |
I agree, let’s shelve Lua subprocess for now, and return to the shell-only stuff. |
|
After pondering about the way to use callback functions instead of events I came to conclusion that it will be a bit tricky. As far as I understand, there is no way to save a Lua function in the C code and use it later. All operations with Lua functions should be performed without leaving the Lua stack. So to achieve the interface with callbacks I need to use some kind of registry in a global variable in the Lua stack and match every element against the C structure representing the process. Are we ok with this approach? I am starting to think that implementing async interface based on coroutines might be an easier task. At least it is possible to save coroutine in the C code. |
If thats the case it kinda sounds like we are reimplementing something local process_handlers = { ["some_name"] = { ["STDOUT"] = stdout_fn, ["STDERR"] = stderr_fn } }
vis.events.subscribe(vis.events.PROCESS_RESPONSE, function(name, resp_type, exit_code, data)
local fn = process_handlers[name][resp_type]
if fn ~= nil then
fn(exit_code, data)
end
end)I would still like the points in this comment covered before merging. |
The separate argument is used to send an exit code or the signal number
|
I've changed the interface of the |
|
Thanks for your hard work!
As it turns out the header is already included via diff --git a/vis-lua.c b/vis-lua.c
index 2349061..f6388c2 100644
--- a/vis-lua.c
+++ b/vis-lua.c
@@ -1420,9 +1420,9 @@ static int communicate_func(lua_State *L) {
Vis *vis = obj_ref_check(L, 1, "vis");
const char *name = luaL_checkstring(L, 2);
const char *cmd = luaL_checkstring(L, 3);
- ProcessStream *inputfd = (ProcessStream *)lua_newuserdata(L, sizeof(ProcessStream));
+ ProcessStream *inputfd = lua_newuserdata(L, sizeof(ProcessStream));
luaL_setmetatable(L, LUA_FILEHANDLE);
- inputfd->handler = vis_process_communicate(vis, name, cmd, (void**)(&(inputfd->stream.closef)));
+ inputfd->handler = vis_process_communicate(vis, name, cmd, &(inputfd->stream.closef));
if (inputfd->handler) {
inputfd->stream.f = fdopen(inputfd->handler->inpfd, "w");
inputfd->stream.closef = &close_subprocess;
diff --git a/vis-subprocess.c b/vis-subprocess.c
index 83b36cc..1aff421 100644
--- a/vis-subprocess.c
+++ b/vis-subprocess.c
@@ -70,7 +70,7 @@ static void destroy_process(Process **pointer) {
* killed on the next main loop iteration.
*/
Process *vis_process_communicate(Vis *vis, const char *name,
- const char *command, void **invalidator) {
+ const char *command, int (**invalidator)(lua_State *)) {
int pin[2], pout[2], perr[2];
pid_t pid = (pid_t)-1;
if (pipe(perr) == -1) {
diff --git a/vis-subprocess.h b/vis-subprocess.h
index 569b753..0fd4944 100644
--- a/vis-subprocess.h
+++ b/vis-subprocess.h
@@ -11,14 +11,14 @@ struct Process {
int errfd;
int inpfd;
pid_t pid;
- void **invalidator;
+ int (**invalidator)(lua_State *);
Process *next;
};
typedef enum { STDOUT, STDERR, SIGNAL, EXIT } ResponseType;
Process *vis_process_communicate(Vis *, const char *command, const char *name,
- void **invalidator);
+ int (**invalidator)(lua_State *));
int vis_process_before_tick(fd_set *);
void vis_process_tick(Vis *, fd_set *);
#endif |
|
These changes are looking good! I will give it another overall look over |
|
Applied in 0b01532! Thanks again for all the hard work! There is one thing that might come up later which is that the |
\o/ So, what remains from 0.9 (and hopefully subsequent merge of tests, see https://lists.sr.ht/~martanne/devel/%3C04ffb382-b86a-b8be-c812-8a001ed21973%40cepl.eu%3E)? |
To improve the plugins API it would be nice to have a mechanism to run and communicate with a subprocess without whole editor hang. E. g. such a mechanism needed to make better implementation of my Nim language plugin.
The plugin spawns a process which caries all information about source code and suggests/checks/gives info in interactive mode using stdin/stdout to communicate with caller. At the moment, the communication problem is solved by kicking vis from the shell using window resize signal and some other dirty hacks to make pipes non-blocking.
Attached implementation provides the
vis.communicatefunction which starts a process in the shell and returns a file handle to write data to itsstdin. If Lua code closes the file handle, the process is being killed. When the process writes anything tostdoutorstderr, quits or gets signal - thePROCESS_RESPONCEevent is being fired with corresponding arguments. The implementation allows multiple subprocesses running in the same time. They can be distinguished by thenameargument passed to thevis.communicatefunction and to the event handler when the event fires. The responsibility of thenameuniqueness is laid on the users code.