Miscellaneous tweaks to OpenOS #2636
Conversation
|
I believe github's "Squash and merge" option for PRs would help us squash this PR into a single commit. Not to worry too much about the multiplicity of commits. Though, yes, we definitely want PRs to be few commits |
| @@ -1,6 +1,7 @@ | |||
| local package = {} | |||
|
|
|||
| package.path = "/lib/?.lua;/usr/lib/?.lua;/home/lib/?.lua;./?.lua;/lib/?/init.lua;/usr/lib/?/init.lua;/home/lib/?/init.lua;./?/init.lua" | |||
| package.config = "/\n;\n?\n!\n-\n" | |||
|
|
|||
There was a problem hiding this comment.
package.config would have limited value
- The filesystem is always going to use / as a directory separator because OpenOS expects that, and the jvm real fs api then also uses /
- Custom separators in the path could help with unusual package names and paths (though
;and?are rather excellent defaults). However your PR doesn't include actually using this. - This isn't on windows, but OpenOS, and we're not using
!to determine the library path - We're not using
-
We could add logic to use these fields, but I feel this is adding complexity with very little user benefit. Unless you have a strong counter to keep this, I'd ask that we remove this part from the PR
There was a problem hiding this comment.
Custom separators in the path could help with unusual package names and paths
Modifying package.config doesn't actually modify how the loader operates: they are compile-time constants.
Unless you have a strong counter to keep this, I'd ask that we remove this part from the PR
There isn't much use for programs written specifically targeting OpenOS. However, if you're trying to run programs written for a general Lua environment it's nice to have. I don't really mind either way, as it's pretty trivial to write some sort of wrapper script.
| code, reason = load("return " .. command, "=stdin", "t", env) | ||
| if not code then | ||
| code, reason = load(command, "=stdin", "t", env) | ||
| end | ||
| end |
There was a problem hiding this comment.
should the return check actually be checked first instead of second?
consider a() which could be return a()
There was a problem hiding this comment.
This is how Lua does it. https://github.com/LuaDist/lua/blob/lua-5.3/src/lua.c#L364-L380
There was a problem hiding this comment.
The code will already attempt to use return a() before trying a(). I may be misunderstanding what you're saying though.
There was a problem hiding this comment.
oh you're right (it tries "return " first), ha ... I must be going senile
| if handler.initial then | ||
| cursor:update(handler.initial) | ||
| end | ||
|
|
||
| while true do |
There was a problem hiding this comment.
can you justify why a tty library needs a way to specify an initial string? We have a similar need for writing to the stdin (yes, writing) in response to vt100 code request for the cursor position, what my solution there lacks is that the data is not echo'd (via cursor:update() ). But I personally don't think it matters that much that case. Anyways - give me a real use case of this.
There was a problem hiding this comment.
Some of the fancier REPLs (such as IPython) will automatically indent lines following a block (such as within an if/while, etc...). I wanted to recreate a similar behaviour in OC. I realise this is pretty niche, so no complaints if it's a little messy.
I did experiment with alternatives (such as initialising the cursor with data), but sadly this is the only place where one can actual display the text to the user.
There was a problem hiding this comment.
the way the tty read is being developed, the right way to do this might be to create a subtype of the cursor object, one that doesn't actually "return" its input until the code is complete. This custom cursor object could also prefix at will. I'll write up a prototype of this you can play with
| end | ||
| handler[0]=nil | ||
| return nil, data .. "\n" |
There was a problem hiding this comment.
not a bad suggestion! what do you think of text.trim(data) ~= handler[1] ?
edit: i'm actually referring to the code a few lines above this -- sorry
If an input does not start with a leading "=", this will parse the input with "return " appended and, if that fails, will parse as a normal statement. This allows for normal expressions to be entered into the repl (such as `2 + 2`) but does mean the parse errors for malformed inputs are confusing. For instance, `3 + ` will error at '3' rather than '<eof>'.
This mimics the behaviour of shells such as bash or zsh, where whitespace-only lines are not entered into history, nor are ones equal to the previous input. This makes history navigation slightly easier.
I realise this is a bit of a hodgepodge of a PR, but it seemed cleaner to put these changes into one than creating lots of small ones. There isn't really any common theme to these commits: it's just features I found myself missing when messing with OpenOS.
The commit messages describe each feature in a little more detail and the rational behind them. Feel free to cherry pick the ones you want (or I can create separate PRs for them if preferred).
package.configconstant.term.read/tty.read's history.term.read/tty.read. The implementation for this is a little ugly, so better implementations are welcome!