Skip to content
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

REPL: add an Option type to handle REPL options #23637

Merged
merged 3 commits into from
Sep 10, 2017
Merged

Conversation

rfourquet
Copy link
Member

It was previously not practical to disable the "align" feature
of backspace (#22939): overwriting the '\b' binding with:
'\b' => (s,o...) -> Base.LineEdit.edit_backspace(s, false)
worked to a certain extent, but for example it was disabling
the feature that '\b' must get you out of shell/help mode.

A new experimental Option type is introduced here, which is
meant to conveniently allow users to pass options controlling
the REPL behavior. For example:

atreplinit() do repl
    repl.options = Base.REPL.Options(backspace_align = false)
end

Also the Options alternative is offered to setup the extra_repl_keymap (and hascolor), instead of using the less direct setup_interface (but no more complicated though). Eventually, probably only one way should be supported. We could even have the following simplified API for simple but common needs:

atreplinit(backspace_align=true, extra_keymap=Dict(...))

I didn't document this yet, until this is approved or becomes less experimental.

cc. @vtjnash who had asked me how to disable backspace_align.

@rfourquet rfourquet added the stdlib:REPL Julia's REPL (Read Eval Print Loop) label Sep 8, 2017
@vtjnash
Copy link
Sponsor Member

vtjnash commented Sep 8, 2017

Thanks for tackling this! As an alternative to Options, I think it may be appropriate to use simply a Dict?

It was previously not practical to disable the "align"
feature of backspace: overwriting the '\b' binding with:
`'\b' => (s,o...) -> Base.LineEdit.edit_backspace(s, false)`
worked to a certain extent, but for example it was disabling
the feature that '\b' must get you out of shell/help mode.

A new experimental Options type is introduced here, which is
meant to conveniently allow users to pass options controlling
the REPL behavior. For example:

atreplinit() do repl
    repl.options = Base.REPL.Options(backspace_align = false)
end
@rfourquet
Copy link
Member Author

As an alternative to Options, I think it may be appropriate to use simply a Dict?

It may work, but a dedicated type is convenient to encode the defaults, and for data validation (check that an option passed by the user really exists, and make it easier that some options are made compatible, e.g. backspace_align and backspace_adjust here (admitedly non-essential!)). But for the user-facing API a Dict could be nicer indeed than to touch the Option type (or a function with keywords like the like example in the OP)

@rfourquet
Copy link
Member Author

I will merge for now. The exact API doesn't have to be set in stone, but it responds to a need, and I would like be able to include more configuration options in follow-up PRs.

@rfourquet rfourquet merged commit 0a7fd04 into master Sep 10, 2017
@rfourquet rfourquet deleted the rf/repl/options branch September 10, 2017 08:23
@vtjnash
Copy link
Sponsor Member

vtjnash commented Sep 12, 2017

Thanks!

@maleadt
Copy link
Member

maleadt commented Sep 19, 2017

Just FYI, I've added REPL start-up plotting at http:https://users.elis.ugent.be/~tbesard/julia/repl.html and it looks like this PR has regressed REPL start-up user time with ~20%.

@rfourquet
Copy link
Member Author

rfourquet commented Sep 20, 2017

it looks like this PR has regressed REPL start-up user time with ~20%.

I've really no idea why this would be so... what command can I run to benchmark the REPL start-up time? When I run $ time ./julia --startup-file=no -i -e 'exit()', I can't observe this 20% increase.

(EDIT: nice graphs BTW!)

@maleadt
Copy link
Member

maleadt commented Sep 20, 2017

I use (something like) this horrible script: https://gist.github.com/maleadt/901833b0317570230e88a5868963fa05

@KristofferC
Copy link
Sponsor Member

Maybe the precompile.jl file need to be updated?

@rfourquet
Copy link
Member Author

Ok thanks to both of you, I will investigate when I can (on travel now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib:REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants