JuliaSyntax parser-based REPL completions overhaul #57767
Conversation
xal-0
added
bugfix
Adds another permitted return type for complete_line, where the second element
of the tuple is a Region (a Pair{Int, Int}) describing the region of text to be
replaced. This is useful for making completions work consistently when the
closing delimiter may or may not be present: the cursor can be made to "jump"
out of the delimiters regardless of whether it is there already.
"exam| =TAB=> "example.jl"|
"exam|" =TAB=> "example.jl"|
This commit replaces the heuristic parsing done by REPLCompletions.completions with a new approach that parses the entire input buffer once with JuliaSyntax. In addition to fixing bugs, the more precise parsing should allow for new features in the future. Some features now work in more situations "for free", like dictionary key completion (the expression evaluated to find the keys is now more precise) and method suggestions (arguments beyond the cursor can be used to narrow the list). The tests have been updated to reflect slightly differing behaviour for string and Cmd-string completion: the new code returns a character range encompassing the entire string when completing paths (not observable by the user), and the behaviour of '~'-expansion has be tweaked to be consistent across all places where paths can be completed. Some escaping issues have also been fixed. Fixes: JuliaLang#55420, JuliaLang#55518, JuliaLang#55520, JuliaLang#55842, JuliaLang#56389, JuliaLang#57611
|
This sounds great! I think we should backport it to at least 1.12 given it fixes so many issues. |
IanButterworth
added
the
backport 1.12
|
Bravo. Truly excellent quality-of-life PR 👏🏻 |
Best part is that this PR has a negative diff, depsite the fact it added many tests. |
…JuliaLang#57307, JuliaLang#57624 Also fix test failing for silly reason
Since we parse the entire input now, we need to avoid triggering method completion when the cursor is on the function name of a valid call.
stdlib/REPL/src/REPLCompletions.jl
Outdated
|
|
||
| # Expand '~' if the user hits TAB after exhausting completions (either | ||
| # because we have found an existing file, or there is no such file). | ||
| full_path = ispath(path) || isempty(paths) |
There was a problem hiding this comment.
I found two cases that can cause ispath to error, so I think we should make a safe_ispath
-
ArgumentError: embedded NULs are not allowed in C strings: "invalid\0"
forlet name = \"valid.txt\", name⁺ = \"invalid\\0.txt\" -
IOError name too long (ENAMETOOLONG) for strings that are too long.
Something like this? Or just never throw and just return false?
function safe_ispath(path::AbstractString)
try
return ispath(path)
catch err
if err isa Base.IOError # i.e. name too long (ENAMETOOLONG)
return false
elseif err isa Base.ArgumentError && occursin("embedded NULs", err.msg)
return false
else
rethrow()
end
end
end
There was a problem hiding this comment.
Maybe call it ispath_noerror or make it a keyword to ispath? The implication that ispath is unsafe or that this is "safer" feels unwarranted—it's just that there's situations where you don't want any errors.
There was a problem hiding this comment.
I'll avoid modifying Base for the time being because we already convert some access(2) return values to false. It feels a little hacky to keep adding exceptions here.
Lines 376 to 379 in bc98abc
Of the errors that could be returned for F_OK that we might want to handle, it looks like we're missing EACCES, EIO, ELOOP, and ENAMETOOLONG.
Co-authored-by: Ian Butterworth <[email protected]>
Co-authored-by: Ian Butterworth <[email protected]>
Co-authored-by: Mosè Giordano <[email protected]>
Overview
As we add REPL features, bugs related to the ad-hoc parsing done by
REPLCompletions.completionshave crept in. This pull request replaces most ofthe manual parsing (regex,
find_start_brace) with a new approach that parsesthe entire input buffer once, before and after the cursor, using JuliaSyntax.
We then query the parsed syntax tree to determine the kind of completion
to be done.
Changes
New, JuliaSyntax-based completions mechanism.
The
complete_lineinterface now has the option of replacing arbitraryregions of text in the input buffer by returning a
Region(Pair{Int, Int}for consistency with the convention in LineEdit, and
posbeing a 0-basedbyte offset).
This is used to improve the handling of auto-inserted closing. Leaving this unchanged for now.delimiters.
Fixes parsing-related bugs:
ModuleName.functioncall is broken #55420@timecalls involving another macro #55429ModuleName.@macro_nameis broken #55518@benchmark A .= A setup=(A=is broken #55520begin-endblock #56389v = Module.Submodule.<TAB>is broken #57611Fixes some bugs that exist on 28d3bd5 that were found by fuzzing:
x \"+TABthrows aFieldErrorexceptionThe duplicate code for path completion in strings,
Cmd-strings, and theshell has been removed, causing paths to complete the same way for all three.
Now,
~is expanded in two situations:If
fooexists, or iffoodoes not exist but there are nopossible completions:
If the current path ends with a
/and you hit TAB again:Future work
Method completions could be changed to look for methods with exactly the given
number of arguments if the closing
)is present, and search for signatureswith the right prefix otherwise.
by putting
::Tin place of arguments).Other REPL features could benefit from JuliaSyntax, so it might be worth
sharing the parse tree between completions and other features:
C-M-f/C-M-b/C-M-u, etc.It would be nice if hints worked even when the cursor is between text.
CursorNodeis a slightly tweaked copy ofSyntaxNodefrom JuliaSyntax thattracks the parent node but includes all trivia. It is used with
seek_pos,which navigates to the innermost node at a given position so we can examine
nearby nodes and the parent. This could probably duplicate less code from
JuliaSyntax.