immerrr
commented
4 years ago Hey, sorry for the delay... your suggestions make sense.
compilation-shell-minor mode usage should be completely optional
yes, a configuration parameter should help, right?
comint-prompt-regexp should be set inside lua-start-process, with setq-local.
ouch, yes, definitely!
There is no reason for lua-process-init-code to be this convoluted.
Yep, we can actually move that part out of the lisp source and put it into scripts/ directory which is now part of the lua-mode package: https://github.com/melpa/melpa/pull/6219. And the replace-regexp-in-string part might get just a little bit more complicated, because a stray #! shebang or a -- comment may ruin the day if all newlines are replaced with spaces blindly.
The current way that the code is sent to the process is too convoluted and breaks with process-send-string limits. Temporary files are a kludge and don't work over sockets.
Temporary files are indeed not quite portable, that's why I removed the temporary files-based approach when I started working on lua-mode and was reluctant to add it back.
Regarding reading io:read from luamode_loadstring, I would prefer not to mess with IO file descriptors more than necessary, because it's easy to step on someone else's toes or to break compatibility with some obscure lua runtime. That said, I am not entirely against it, it may work. One suggestion from the get go is to consider building the "protocol" around newlines to minimize the amount of remote initialization, because AFAIR Lua does not support "reading until delimiter" out of the box unless the delimiter is a newline (io.read("*line")).
Another option would be to implement the "protocol" as a multi-part interface in which lua-mode itself would send the command in parts
luamode_multipart_start() -- notify that the command is going to be sent in parts
luamode_multipart_add(part_of_the_command) -- send part1
luamode_multipart_add(part_of_the_command) -- send part2
...
luamode_multipart_run() -- combine parts on remote side and execute them allGo one step further, and get rid of the `lineno' kludge by splitting the output from the process into normal output and error output on top of the previously mentioned protocol.
It may work, but I'd really like to discuss the implementation before it is done, because it could prove to be a significant effort and I don't want it to go to waste because of not being on the same page.
I may tackle some of these myself, assuming I find some free time, but in the meantime I thought it'd be useful if I posted a list of things that annoy me here (maybe others can strike some of these off):
compilation-shell-minor mode usage should be completely optional. Switching it on without an option is not kosher. Worse, advising compilation-* functions on lua-mode.el load is unacceptable!
comint-prompt-regexp should be set inside lua-start-process, with setq-local. Right now it's done inside define-derived-mode and the global comint-prompt-regexp is overwritten (not good).
There is no reason for lua-process-init-code to be this convoluted. There's no need for (mapconcat 'identity ...) there, a multiline Lisp string will do just fine with the following diff in lua-start-process:
The current way that the code is sent to the process is too convoluted and breaks with process-send-string limits. Temporary files are a kludge and don't work over sockets. A much better way is to have luamode_loadstring call io:read() in a loop until it hits a delimiter. A proper input/output protocol can be defined that works over that channel.
Go one step further, and get rid of the `lineno' kludge by splitting the output from the process into normal output and error output on top of the previously mentioned protocol. These two separate streams should be multiplexed over the common channel and post-processing applied to the error output to fix the line number (when needed). For a local lua process, this means that stderr will not be used at all. All output will be multiplexed on top of stdout.