-
-
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
Apply syntax highlighting to Exprs in REPL #54446
Conversation
If anyone's up for it, I think this should be a pretty quick and easy review & merge 🙂 |
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 should be done without type piracy and respect the REPL IOContext:
Before:
julia> :(quote 123.32132132132 end)
:($(Expr(:quote, quote
#= REPL[1]:1 =#
123.32132132132
end)))
julia> Base.active_repl.options.iocontext[:compact] = true
true
julia> :(quote 123.32132132132 end)
:($(Expr(:quote, quote
#= REPL[3]:1 =#
123.321
end)))
After
julia> :(quote 123.32132132132 end)
:($(Expr(:quote, quote
#= REPL[1]:1 =#
123.32132132132
end)))
julia> Base.active_repl.options.iocontext[:compact] = true
true
julia> :(quote 123.32132132132 end)
:($(Expr(:quote, quote
#= REPL[3]:1 =#
123.32132132132
end)))
ddae7e0
to
8b98888
Compare
@fredrikekre / @KristofferC any thoughts on the current version of the code? |
stdlib/REPL/src/precompile.jl
Outdated
@@ -23,6 +23,7 @@ using Base.Meta | |||
|
|||
import Markdown | |||
import StyledStrings | |||
import JuliaSyntaxHighlighting |
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 really needed?
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.
I'm actually not sure, there was some conversation I had (on Slack) which concluded with the import StyledStrings
line being needed in #51869 even though it's not used in precompile.jl
, so I've just "followed the pattern" here.
I could just try dropping the line and seeing what happens?
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.
I don't see a mechanism for how it should be needed so it's probably good to try to remove it and see what happens.
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.
Trying... 🤞
The current version seems ok to me at least. Does it still obey |
Yep, since it's just printing via. |
Large exprs (such as those produced by @macroexpand) can be hard to read and interpret. This is a problem made easier by JuliaSyntaxHighlighting, and with it we can easily apply syntax highlighting when showing exprs. Normally this would be implemented in Base's show method for Exprs, however since JuliaSyntaxHighlighting is a stdlib we must look elsewhere, and REPL seems like a sensible place. To ensure that the IOContext of the REPL IO is properly respected, we change the show invocation within the REPL display method to use show_repl which falls back to just calling show, as before. We then add a show_repl(::IO, ::MIME"text/plain", ::Expr) specialisation, which prints expressions with syntax highlighting.
8b98888
to
2929aa9
Compare
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 looks fine to me now, but it'd be great if @KristofferC and/or @fredrikekre could have a final look (and then perhaps directly merge it if they are happy)
Bump @KristofferC / @fredrikekre |
This looks really simple and straight-forward to me, I hope it can be merged soon -- it's been ready (at least from my POV) for over 2 months |
Large exprs (such as those produced by @macroexpand) can be hard to read and interpret. This is a problem made easier by JuliaSyntaxHighlighting, and with it we can easily apply syntax highlighting when showing exprs. Normally this would be implemented in Base's show method for Exprs, however since JuliaSyntaxHighlighting is a stdlib we must look elsewhere, and REPL seems like a sensible place. To ensure that the IOContext of the REPL IO is properly respected, we change the show invocation within the REPL display method to use show_repl which falls back to just calling show, as before. We then add a show_repl(::IO, ::MIME"text/plain", ::Expr) specialisation, which prints expressions with syntax highlighting.
As discussed on slack a while ago, it's now near trivial to make inspecting large Exprs much more pleasant with JuliaSyntax highlighting.
See the commit message for more details.
Screenshots