Change help commands to use name from scope instead of the name from the declaration#14490
Change help commands to use name from scope instead of the name from the declaration#14490fdncred merged 1 commit intonushell:mainfrom
help commands to use name from scope instead of the name from the declaration#14490Conversation
|
Sounds great! This has been an issue (and tough to explain in documentation) for a while now. |
| self.permanent_state.find_decl(name, &removed_overlays) | ||
| } | ||
|
|
||
| pub fn find_decl_name(&self, decl_id: DeclId) -> Option<&[u8]> { |
There was a problem hiding this comment.
As this whole find_decl stuff is a bit confusing and stretched over the different state parts, maybe worth a doccomment what the particular methods are useful for.
There was a problem hiding this comment.
Good point, I can take a look at adding some here and in engine_state.rs
| .get_decls_sorted(false) | ||
| .into_iter() | ||
| .filter_map(|(_, decl_id)| { | ||
| let decl = engine_state.get_decl(decl_id); | ||
| (decl.name() == name).then_some(decl) | ||
| }) | ||
| .map(|cmd| get_full_help(cmd, engine_state, stack)) | ||
| .collect::<Vec<String>>(); |
There was a problem hiding this comment.
Do we need some separate behavior to find all the possibly now shadowed decls?
Wondering if there is a situation where we want to find more than one like this.
Is this useful if you have multiple same named decls accross modules that are not use'd by name into the scope?
There was a problem hiding this comment.
I thought about this, but I struggle to imagine a scenario where you would actually want to do this.
Is this useful if you have multiple same named decls accross modules that are not
use'd by name into the scope?
I'm not quite certain what you mean by this, could you provide an example?
There was a problem hiding this comment.
When you have two modules used by just there module name like:
use foo.nu
use bar.nuand want to learn about the command baz defined in both to find out which to import fully
There was a problem hiding this comment.
If I'm understanding, you should be able to still view the help for each:
foo.nu:
# foo spam
export def spam [] {}bar.nu:
# bar spam
export def spam [] {}use foo.nu
use bar.nu
help foo spam
# => foo spam
# =>
# => Usage:
# => > spam
# =>
# => Flags:
# => -h, --help: Display the help message for this command
# =>
# => Input/output types:
# => ╭───┬───────┬────────╮
# => │ # │ input │ output │
# => ├───┼───────┼────────┤
# => │ 0 │ any │ any │
# => ╰───┴───────┴────────╯
# =>
help bar spam
# => bar spam
# =>
# => Usage:
# => > spam
# =>
# => Flags:
# => -h, --help: Display the help message for this command
# =>
# => Input/output types:
# => ╭───┬───────┬────────╮
# => │ # │ input │ output │
# => ├───┼───────┼────────┤
# => │ 0 │ any │ any │
# => ╰───┴───────┴────────╯
# => |
Thanks for tackling this! That sounds like a great clear up of issues. We need to go through your full list and close when we land :D |
|
That one has been bugging me for so long as I use modules for everything! |
|
Are we ready to land this? |
|
I can add the doccomments @sholderbach mentioned in either this PR or a separate PR, so I'm fine with landing this now if there aren't any other concerns. Wrote a note for myself so I don't forget 😄 |
|
ok, let's do that separately then. Thanks for all your work here. |
|
ok, thanks. i closed those and re-opened the others. |
|
@132ikl Thank you so much for this! I had stupid workarounds because of this issue like prefixing module files with underscores and using the module name instead of |
#14750) # Description Adds some doccomments to some of the methods in `engine_state.rs` and `state_working_set.rs`. Also grouped together some of the `find` methods in `engine_state.rs`, but didn't do so in `state_working_set.rs` since they seem to already be grouped according to decl/overlay/module. Follow-up to #14490. # User-Facing Changes None # Tests + Formatting N/A # After Submitting N/A
Description
Before this PR,
help commandsuses the name from a command's declaration rather than the name in the scope. This is problematic when trying to view the help page for themaincommand of a module. For example,std bench:This can also cause confusion when importing specific commands from modules. Furthermore, if there are multiple commands with the same name from different modules, the help text for both will appear when querying their help text (this is especially problematic for
maincommands, see #14033):This PR changes
help commandsto use the name as it is in scope, so prefixing any command in scope withhelpwill show the correct help text.Additionally, the IR code generation for commands called with the
--helptext has been updated to reflect this change.This does have one side effect: when a module has a
maincommand defined, runninghelp <name>(which checkshelp aliases, thenhelp commands, thenhelp modules) will show the help text for themaincommand rather than the module. The help text for the module is still accessible withhelp modules <name>.Fixes #10499, #10311, #11609, #13470, #14033, and #14402.
Partially fixes #10707.
Does not fix #11447.
User-Facing Changes
help <command name>, where the command name is the same thing you would type in order to execute the command. Previously, it was the name of the function as written in the source file.spamwith commandmeow:meowisused, the help text is viewable by runninghelp meow.use spam: Themeowcommand is executed by runningspam meowand thehelptext is viewable by runninghelp spam meow.use spam foo: Themeowcommand is executed by runningmeowand thehelptext is viewable by runningmeow.maincommand defined,help <module name>will return help for the main command, rather than the module. To access the help for the module, usehelp modules <module name>.Tests + Formatting
toolkit fmttoolkit clippytoolkit testtoolkit test stdlibAfter Submitting
N/A