-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix moving up and down in REPL #350
Conversation
src/repl.jl
Outdated
php_idx = VERSION >= v"1.11.0-rc1" ? 6 : 5 | ||
p = mirepl.interface.modes[php_idx]::LineEdit.PrefixHistoryPrompt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems brittle. Better to find first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I have improved the implementation by using findfirst
. (8b6dc6f)
src/repl.jl
Outdated
mirepl = isdefined(repl,:mistate) ? repl.mistate : repl | ||
interface_modes = mirepl.interface.modes | ||
main_mode = interface_modes[1] | ||
php_idx = findfirst(isa.(interface_modes, LineEdit.PrefixHistoryPrompt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
php_idx = findfirst(isa.(interface_modes, LineEdit.PrefixHistoryPrompt)) | |
php_idx = findfirst(isa(LineEdit.PrefixHistoryPrompt), interface_modes) |
Avoid allocating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, isa
requires two arguments explicitly. Here is a workaround:
findfirst(Base.Fix2(isa, LineEdit.PrefixHistoryPrompt), interface_modes)
However, I'm uncertain about using Base.Fix2
since it hasn't been exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. It's fine to use Fix2, it's public, just not exported https://docs.julialang.org/en/v1/base/base/#Base.Fix2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me. Assuming the main mode is first is probably ok
Thanks for the quick fix! If you do not wish to hard code by assuming main mode is first entry, one can replace the get_prompt_str = m -> LineEdit.prompt_string(isdefined(m, :prompt) ? m.prompt : "")
mm_idx_ = findfirst(m -> get_prompt_str(m) == REPL.JULIA_PROMPT, interface_modes) # main mode index
mm_idx = isnothing(mm_idx_) ? 1 : mm_idx_
main_mode = interface_modes[mm_idx] |
Fix #349 corresponding to JuliaLang/julia#54674
https://github.com/JuliaLang/julia/pull/54674/files#diff-730f90e0bf9ca018477a1d4a52e0d7398af2bb8cf9e401568fd86690eec81bb7L1453-R1488