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-only: Print a hint if the user types exit in the REPL #38522

Closed
wants to merge 2 commits into from
Closed

REPL-only: Print a hint if the user types exit in the REPL #38522

wants to merge 2 commits into from

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Nov 21, 2020

Closes #21362
Closes #38307

This hint will only be shown if the input is exit.

julia> exit
exit (generic function with 2 methods)

To exit Julia, type exit() and press enter

julia> fs = [exit];

julia> fs[1]
exit (generic function with 2 methods)

@DilumAluthge
Copy link
Member Author

@JeffBezanson Alright, I think I have finally figured out how to show the hint only when the REPL input is exit.

This does not affect how the exit object is shown.

@simonbyrne simonbyrne added the REPL Julia's REPL (Read Eval Print Loop) label Nov 24, 2020
stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved
stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved
print_exit_hint(repl::LineEditREPL, line) = print_exit_hint(repl.t, line)
print_exit_hint(repl::StreamREPL, line) = print_exit_hint(repl.stream, line)
function print_exit_hint(io::IO, line)
if lowercase(strip(line)) == "exit"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to also print this if the input is "quit" (that's what e.g. GAP uses) ?

Anyway, I really like this PR, it fixes one of those tiny papercuts which seem irrelevant in isolation but add up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't object to also doing this for "quit", but I'll defer to @JeffBezanson.

I'd want to keep this limited to a very small set of inputs. For example, you can exit R by typing q() and pressing enter. But I definitely wouldn't want to print a hint if someone types q in the Julia REPL.

I think it makes sense for exit because the corresponding function exit() is a function that actually exists in Julia. But we don't actually have a quit function in Julia.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I don't feel strongly either way. Happy to add quit, also happy to leave it out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think we should add it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what the python repl does:

>>> quit
Use quit() or Ctrl-D (i.e. EOF) to exit
>>> quit = 2
>>> quit
2

It stops printing the message if quit is defined as a variable. Stylish! I think we should do this too.
Can be checked with Base.isbindingresolved(Main, :exit) && isdefined(Main, :exit).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In python, that's not magic, it is just literally the definition of show for that default variable:

>>> exit.__class__
<class 'site.Quitter'>
>>> quit.__class__
<class 'site.Quitter'>

The equivalent here would be:

julia> using Base: exit as quit; # @eval Base const quit = exit; @eval Base export quit;

julia> Base.show(io::IO, ::MIME"text/plain", ::typeof(exit)) = print(io, "Use exit() or Ctrl-D (i.e. EOF) to quit")

julia> repr(exit)
"exit"

julia> show(exit)
exit
julia> summary(exit)
"typeof(exit)"

julia> display(exit)
Use exit() or Ctrl-D (i.e. EOF) to quit

julia> exit
Use exit() or Ctrl-D (i.e. EOF) to quit

julia> exit()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which would have been #21362

doc/build/build.md Outdated Show resolved Hide resolved
@musm musm mentioned this pull request Dec 15, 2020
stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@mkitti
Copy link
Contributor

mkitti commented Aug 24, 2021

@DilumAluthge asked if I would be interested in making the final push on this. Here's are a few thoughts:

  1. quit should be handled in exactly the same manner as help in Display help message upon UndefVarError(:help) using REPL.REPLCompletitions.UndefVarError_hint #41754. Perhaps that should be a separate PR entirely.
  2. For exit, we're adding an additional parsing step for every command entered into the REPL by hard coding in a call to print_exit_hint which then calls lowercase(strip(line)).
  3. As alternative, we could just add a default ast_transform and we can match the abstract syntax tree rather than trying to parse the line again. If someone doesn't like this, they could just pop!(Base.active_repl_backend.ast_transforms).

An example ast_transform might work like this:

julia> function print_exit_hint(ast)
           if !isempty(ast.args) && ast.args[end] == :exit
               quote
                   println("To exit Julia, use Ctrl-D, or type exit() and press enter.")
                   exit
               end
           else
               ast
           end
       end
print_exit_hint (generic function with 1 method)

julia> push!(Base.active_repl_backend.ast_transforms, print_exit_hint)
2-element Vector{Any}:
 softscope (generic function with 1 method)
 print_exit_hint (generic function with 1 method)

julia> exit
To exit Julia, use Ctrl-D, or type exit() and press enter.
exit (generic function with 2 methods)

julia> out = ans
exit (generic function with 2 methods)

julia> typeof(out)
typeof(exit)

julia> pop!(Base.active_repl_backend.ast_transforms)
print_exit_hint (generic function with 1 method)

julia> exit
exit (generic function with 2 methods)

@mkitti
Copy link
Contributor

mkitti commented Aug 24, 2021

The hint for quit is over at #41990 which addresses @JeffBezanson 's #38522 (comment) .

If we wanted to go the ast transform route, I've prepared this commit on my branch of dpa/exit-hint-repl
mkitti@6bb3909

That version is stricter than the version above and tries to parallel the softscope ast transform in form.

julia> Base.isbindingresolved(Main, :exit)
true

julia> isdefined(Main, :exit)
true

julia> exit = 2
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace:
 [1] top-level scope
   @ REPL[10]:1

julia> exit
exit (generic function with 2 methods)

julia> function print_exit_hint(@nospecialize ex)
           if ex isa Expr && ex.head === :toplevel && length(ex.args) == 2 && ex.args[2] === :exit
               quote
                   println("suggestion: To exit Julia, use Ctrl-D, or type exit() and press enter.")
                   exit
               end
           else
               return ex
           end
       end
print_exit_hint (generic function with 1 method)

julia> push!(Base.active_repl_backend.ast_transforms, print_exit_hint)
2-element Vector{Any}:
 softscope (generic function with 1 method)
 print_exit_hint (generic function with 1 method)

julia> exit
suggestion: To exit Julia, use Ctrl-D, or type exit() and press enter.
exit (generic function with 2 methods)

julia> repr(exit)
"exit"

julia> summary(exit)
"exit (generic function with 2 methods)"

julia> display(exit)
exit (generic function with 2 methods)

julia> filter!(!=(print_exit_hint), Base.active_repl_backend.ast_transforms)
1-element Vector{Any}:
 softscope (generic function with 1 method)

julia> exit
exit (generic function with 2 methods)

julia> exit()

I also suggest adding the prefix "suggestion: " to be consistent with the other REPL hints.

Also note above that we cannot assign a new value to exit, so I am unsure what @JeffBezanson meant in the last sentence of #38522 (comment) .

I would appreciate if reviewers could address these questions:

  1. Do we stick with the original implementation or refactor to an abstract syntax tree transform?
  2. If we do use the AST transform, is the order of the lines and the whitespace acceptable?
  3. Are there any other unaddressed issues?

@DilumAluthge DilumAluthge marked this pull request as draft August 24, 2021 22:06
@DilumAluthge
Copy link
Member Author

Also note above that we cannot assign a new value to exit, so I am unsure what @JeffBezanson meant in the last sentence of #38522 (comment) .

You can't assign a new value to exit if the binding has already been resolved:

julia> exit
exit (generic function with 2 methods)

julia> exit = 123
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace:
 [1] top-level scope
   @ REPL[1]:1

But you can assign a new value to exit if the binding has not yet been resolved. For example, run the following in a brand-new REPL session:

julia> exit = 123
123

julia> exit
123

julia> exit()
ERROR: MethodError: objects of type Int64 are not callable
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

@DilumAluthge
Copy link
Member Author

So I think you'd need to check something like Main.exit === Base.exit to figure out if the user has redefined exit to something else.

@mkitti
Copy link
Contributor

mkitti commented Aug 24, 2021

So I think you'd need to check something like Main.exit === Base.exit to figure out if the user has redefined exit to something else.

How does one redefine exit in Main?

julia> exit = 2
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace: [1] top-level scope @ REPL[10]:1

@DilumAluthge
Copy link
Member Author

So I think you'd need to check something like Main.exit === Base.exit to figure out if the user has redefined exit to something else.

How does one redefine exit in Main?

julia> exit = 2
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace: [1] top-level scope @ REPL[10]:1

Huh. If I run exit = 2 in a brand-new REPL session in Julia 1.6.2, it works fine for me.

What Julia version did you try it in?

Also, make sure it's a brand-new session.

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Aug 25, 2021

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.6.2 (2021-07-14)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> exit = 123
123

julia> exit
123

julia> exit()
ERROR: MethodError: objects of type Int64 are not callable
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.7.0-beta2 (2021-06-20)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> exit = 456
456

julia> exit
456

julia> exit()
ERROR: MethodError: objects of type Int64 are not callable
Maybe you forgot to use an operator such as *, ^, %, / etc. ?
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.0-DEV.404 (2021-08-25)
 _/ |\__'_|_|_|\__'_|  |  Commit 7a784de2bf (0 days old master)
|__/                   |

julia> exit = 789
789

julia> exit
789

julia> exit()
ERROR: MethodError: objects of type Int64 are not callable
Maybe you forgot to use an operator such as *, ^, %, / etc. ?
Stacktrace:
 [1] top-level scope
   @ REPL[2]:1

@mkitti
Copy link
Contributor

mkitti commented Aug 25, 2021

I see where I was getting confused. Pushing the ast transform was forcing exit to resolve in Main. That is not desirable. Time to take the metaprogramming to the next level.

@mkitti
Copy link
Contributor

mkitti commented Aug 25, 2021

It seems that even compiling Base.isdefined(Main, :exit) results in the :exit binding being resolved.

julia> Base.isbindingresolved(Main, :exit)
false

julia> Base.isdefined(Main, :exit)
true

julia> exit = 2
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace:
 [1] top-level scope
   @ REPL[6]:1

julia> Base.isbindingresolved(Main, :exit)
true

Apparently some compilation happens when I push! a function containing it into Base.active_repl_backend.ast_transforms

julia> function dummy(ex)
           isdefined(Main, :exit)
           ex
       end
dummy (generic function with 1 method)

julia> Base.isbindingresolved(Main, :exit)
false

julia> push!(Base.active_repl_backend.ast_transforms, dummy)
2-element Vector{Any}:
 softscope (generic function with 1 method)
 dummy (generic function with 1 method)

julia> Base.isbindingresolved(Main, :exit)
true

julia> exit = 2
ERROR: cannot assign a value to variable Base.exit from module Main
Stacktrace:
 [1] top-level scope
   @ REPL[6]:1

@DilumAluthge
Copy link
Member Author

I'm closing this PR in favor of @mkitti's PR #42011

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

Successfully merging this pull request may close these issues.

6 participants