Multiline support for evcxr_repl #98
Conversation
| @@ -0,0 +1,86 @@ | |||
| // Copyright 2018 Google Inc. | |||
There was a problem hiding this comment.
You could change this to "Copyright 2020 The Evcxr Authors".
Also welcome to leave it as-is and I'll do a bulk update at some point.
| /// quote. | ||
| Normal, | ||
| /// Raw string. Closed after we see a " followed by the right num of | ||
| /// backslashes, |
There was a problem hiding this comment.
Did you mean "hashes" here?
| /// after seeing a closing \", do we need to think we're done), or None if | ||
| /// something looks dodgy. | ||
| /// | ||
| /// information that we should hopefully know whether or not the string closed, |
There was a problem hiding this comment.
Is this last paragraph complete? Looks like it might have been chopped off.
| /// Expects to be called after `iter` has *fully consumed* the initial `//`. | ||
| /// | ||
| /// Consumes the entire comment, including the `\n` and returns true, or returns | ||
| /// false if we exhausted `iter` before finding a `\n`. |
There was a problem hiding this comment.
Looks like it doesn't return anything. I guess that's OK, since presumably the input will always end in a '\n'?
| } | ||
| } | ||
|
|
||
| /// This should be right called after `input` reads a `'`. See `do_eat_char` for |
There was a problem hiding this comment.
Swap "right" and "called"?
| @@ -13,9 +13,10 @@ edition = "2018" | |||
|
|
|||
| [dependencies] | |||
| evcxr = { version = "=0.4.7", path = "../evcxr" } | |||
| rustyline = "5.0.2" | |||
| rustyline = "6.0.0" | |||
There was a problem hiding this comment.
Looks like rustyline 6 uses the non_exhaustive feature, which was only stabilised in 1.40.0. I think I'm OK with dropping support for older versions of rust so long as it gives us something. Can you update .travis.yml to bump the minimum supported version to 1.40.0?
|
I'd like to merge this. Let me know if you'd like more time to address the comments I made before, otherwise I'll just merge and attempt to address them myself. Thanks again for this awesome change and for such well commented code :) |
|
Sorry, meant to get back to this, not at my computer this weekend, so feel free to land it yourself! |
|
How to use this multiline feature? The |
|
It's automatically multiline if there are unclosed items, e.g. open parentheses, brackets, strings, etc. |
|
Note that some issues with it exist, and are blocked on kkawakam/rustyline#334. (As well as other bugs in that repo) |
Fixes #4
This is something I've wanted for a while, and have been poking at off and on.
It ended up being a lot more involved than I had hoped. This is because essentially nothing that parses or scans rust seems to handle unmatched brackets at all, since they all operate on token trees, which must be well formed. I even asked wg-grammar channel on discord, and basically got told that I was going to have to go my own way, or use something like tree-sitter (which would only solve half the problem (lexing), requires taking a C dependency, and then either writing a tree-sitter grammar for rust or vendoring one from one of the text editors...
I considered the naive solution of just scanning for parens/brackets and ignoring anything else, but in practice it's pretty annoying: it's not really okay for the repl to see a
"("and then never send your input since it thinks it's invalid -- so you need to at least lex the input well enough to notice strings/chars/comments, so that's what I did.Unfortunately, even just this much is pretty involved, as rust's syntax for these is surprisingly tricky: Block comments can nest, chars are hard to distinguish from lifetimes, and it has
rawstrings.That said, it works well in practice, has lots of comments, and pretty good test coverage, so hopefully it's not too controversial.
I'd also like to improve the repl more in the near future -- rustyline seems to have support for a few more things that would be useful, and it's possible we can use
synto handle many of the cases this misses (in particular detecting missing semicolons on stuff likelet x = 3probably requires syn in order to be robust), but for now this patch was already pretty huge, so I left it as is.Note: I've only tested this manually on macos at the moment, but have access to other platforms and will try to get around to it soon.