- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.7k
Missing package add prompt: Restrict which exprs to search in #43457
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
Missing package add prompt: Restrict which exprs to search in #43457
Conversation
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.
Disabling the feature for eval expressions sound reasonable to me.
        
          
                stdlib/REPL/src/REPL.jl
              
                Outdated
          
        
      | end | ||
| end | ||
| for arg in ast.args | ||
| arg == Symbol("@eval") && break # don't search beyond an `@eval` | 
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.
The same kind of situation will also happen for Core.eval(xxx, ...) as well as xxx.eval(...).
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.
@simeonschaub's suggestion below seems clean so I've gone for that
| Instead of recursing into any expression, could we not just recurse into  | 
0ec75fe    to
    6ba8680      
    Compare
  
    @eval(cherry picked from commit 8f192bd)
| end | ||
| for arg in ast.args | ||
| arg isa Expr && modules_to_be_loaded(arg, mods) | ||
| if arg isa Expr && arg.head in [:block, :if, :using, :import] | 
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 arg isa Expr && arg.head in [:block, :if, :using, :import] | |
| if arg isa Expr && arg.head in (:block, :if, :using, :import) | 
The [] form allocates the same tuple as above, then splats it to a new array, on every iteration of the loop.
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.
Or even more concisely:
| if arg isa Expr && arg.head in [:block, :if, :using, :import] | |
| if isexpr(arg, (:block, :if, :using, :import)) | 
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.
Makes sense, thanks #43708
Fixes #43446
.. by not looking beyond an
@evalwhen collecting a list of modules that are going to be loaded for the missing package install prompt.