-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Fixes to completions #355
Fixes to completions #355
Conversation
# FIXME: when generating completions via `python -m script completions`, | ||
# we incorrectly infer `script_name` as `__main__.py` | ||
script_name = self._io.input.script_name or inspect.stack()[-1][1] | ||
script_name = self._io.input.script_name or self._get_prog_name_from_stack() |
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.
did this work for you? ik it works for click, but would need tweaking
what about unifying the logic from the help
command, which prefers self._application.name
if defined? that feels more consistent and what I would expect:
cleo/src/cleo/commands/base_command.py
Lines 61 to 64 in 7448f55
if self._application: | |
current_script = self._application.name | |
else: | |
current_script = inspect.stack()[-1][1] |
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 have only tested the _get_prog_name_from_stack
method and it has returned the proper module name. Aplication.name
is not always good, because you might have your program aliased on shell level (like having pipx
installation with suffix).
*[ | ||
f"complete -c {script_name} -A " | ||
f"-n '__fish_seen_subcommand_from {sanitize(command_name)}' " |
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.
just to make sure I understand, the syntax error was already fixed here with 6cc808d right? we just haven't had a release since then
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 fix was making '__fish_seen_subcommand_from \'command name\''
entry, while this fix changes it to being '__fish_seen_subcommand_from "command name"'
which feels like a more "proper" way to nest quotes.
Superseded by #358 |
Closes: #231