-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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: allow tweaking the implicit IOContext of the REPL #29249
Conversation
Almost one year later... I miss this functionality, for example I worked with small arrays yesterday which often don't fit the screen but whose content I want to inspect, I know of two ways to do that As an interactive feature, this does not bind us much, if we want to evolve the API later. Any objections? |
6613f8c
to
5aa66f5
Compare
Bump. I will be tempted to just merge if no-one expresses objections/suggestions. |
In its current state it isn't really usable by anyone really (no docs) but perhaps you plan on adding that later? |
5aa66f5
to
b386f5f
Compare
Good point thanks, docs just added. |
The failed test look unrelated, but it would be good if someone can confirm. |
Bump. |
stdlib/REPL/docs/src/index.md
Outdated
In particular, the `:limit` attribute is set to `true`. | ||
Other attributes can receive in certain `show` methods a default value if it's not already set, | ||
like `:compact`. | ||
It's possible to specify the attributes used by the REPL via the `Base.active_repl.iocontext` |
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.
Is this the first time Base.active_repl
is introduced? In that case, it might make sense to create a dedicated function for this in the REPL
module.
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.
It seems to be the first time that Base.active_repl
is introduced. I'm not clear of the point of a dedicated function to access it, and I think it's fine allow it for now (this field is likely to stay for a while). That said, I should probably document how to do that in the startup file.
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.
Well, the point is to not expose some weird global variable in Base that is only defined in at a certain point in the startup procedure.
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.
Also, there is an options
field in active_repl
(unfortunately not yet documented), whose fields can be interactively explored with tab completion. AFAICT, if active_repl
was accessible only via a call to a function, there would not be autocompletion.
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.
If we put this in the documentation then the active_repl.iocontext
is an official API and it can never ever be changed. It seems likely we want to have some better way to set options
in the REPL than this in the future so committing to this "setting fields in a global variable in Base API" doesn't seem that great.
If there isn't a good way of setting options in the REPL right now I would prefer merging without exposing the current implementation as a stable API.
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.
But I won't define the function get_active_repl() = Base.active_repl.
Heh, no.
My thinking is something along the lines of having:
REPL.options.iocontext = Dict(:compat => false)
REPL.options.bell = false
...
where REPL.options
is documented (and tab completable).
The active repl creates a reference to the options
object when it gets initialized and the options
object is always defined.
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.
OK, this is beyond the scope of this PR though. Do you suggest that REPL.options
be global? It could be an object with overloaded setproperty!
and getproperty
, to make it a proxy to Base.active_repl.options
... Indeed looks like it may be better than Base.active_repl
.
If we put this in the documentation then the active_repl.iocontext is an official API and it can never ever be changed.
OK, this was not my understanding, as it's interactive. So what about, in this PR, leaving the atreplinit
portion of the doc as the official way, and pointing at the Base.active_repl
as an experimental way to play with the options?
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.
Sure, if we denote it as experimental, that's fine.
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.
So I just marked the whole feature as experimental anyway, as once it's used, we may find out a better way to achieve the same, but we just need to start somewhere.
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.
@KristofferC Just FYI Module.CONFIG
is now used in at least three packages for this kind of purpose (disclaimer: I did the implementation in all cases):
https://github.com/queryverse/ElectronDisplay.jl
https://github.com/JuliaDebug/Cthulhu.jl
https://github.com/tkf/InteractiveCodeSearch.jl
Although I guess I'll follow stdlib approach if REPL.options
is implemented.
9a720f2
to
caf2950
Compare
caf2950
to
f54477e
Compare
I've updated to put the I will merge next time CI is green enough :) |
This adds an `iocontext::Dict{Symbol,Any}` field to `LineEditREPL.options`, which can be initialized with `atreplinit`, and updated interactively, e.g. `Base.active_repl.options.iocontext[:compact] = true`. Fixes #20509.
f54477e
to
4212cc9
Compare
This adds an `iocontext::Dict{Symbol,Any}` field to `LineEditREPL.options`, which can be initialized with `atreplinit`, and updated interactively, e.g. `Base.active_repl.options.iocontext[:compact] = true`. Fixes #20509.
I would note that this fantastic feature isn't usable in a startup file; probably because the REPL is not initiated yet? What would be a good solution to this is having a second startup file... for REPL only. |
Right... it's even in the documentation. That said, this probably deserves its own section heading, on par with to setting color and key bindings. Thanks again. |
This adds an
iocontext::Dict{Symbol,Any}
field toLineEditREPL
,which can be initialized with
atreplinit
, and updatedinteractively, e.g.
Base.active_repl.iocontext[:compact] = true
.Fixes #20509.
An alternative would be providing specific functions, e.g.
setiocontext(:key, value)
,resetiocontext(:key)
or something. I think I prefer the dictionnary approach, because aDict
already has a convenient API for the needs here, and as an interactive feature, this can be changed relatively easily if needed.I wonder whether we should think about a namespace policy to use this, as package
X
could say for example "useiocontext[:format] = :bin
for printing with binary digits", while packageY
could say haveiocontext[:format] = whatever
to control formatting of other stuff, and it becomes impossible to have control over both packages at the same time.Another option would therefore to have a predetermined list of settable properties (
:compact
,:limit
etc), in which case using amutable struct
with corresponding fields would probably be a more appropriate implementation than aDict
.