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

use _default instead of nothing as dest in complete_zsh #65

Open
tshu-w opened this issue Feb 18, 2022 · 7 comments · May be fixed by #83
Open

use _default instead of nothing as dest in complete_zsh #65

tshu-w opened this issue Feb 18, 2022 · 7 comments · May be fixed by #83
Assignees

Comments

@tshu-w
Copy link

tshu-w commented Feb 18, 2022

Hi, it will be much better if we use _default instead of nothing as dest in complete_zsh https://github.com/iterative/shtab/blob/master/shtab/__init__.py#L476 and https://github.com/iterative/shtab/blob/master/shtab/__init__.py#L486. User can get basic default competition without setting.

@tshu-w tshu-w changed the title use _default instead of `` as dest in complete_zsh use _default instead of nothing as dest in complete_zsh Feb 18, 2022
@casperdcl
Copy link
Collaborator

Hi @tshu-w - not sure I follow but would be happy to accept a PR :)

@tshu-w
Copy link
Author

tshu-w commented Feb 19, 2022

Hi, let me explain my thought further. As mentioned in omni-us/jsonargparse#108 (comment), I use shtab to enable shell completion for PythorchLightning CLI. However, It's hard for me to modify each option complete as they are many and complex. Therefore it is inconvenient for some options that can accept files or directories because by default there is no completion after enter this options. Here is an example:

run fit --model configs/model/default.yaml --data # I cannot got complete here

I would like to use _default instead of nothing as dest in complete_zsh, which mean I will change this line from

"({})".format(" ".join(map(str, opt.choices)))) if opt.choices else "",

to

"({})".format(" ".join(map(str, opt.choices)))) if opt.choices else "_default",

This would be incompatible and may not be what everyone expects, so for a formal PR, I think it would be a good idea to set a variable for the user to modify like:

DEFAULT_FUNCTIONS = {
    "bash": "_shtab_compgen_files",
    "zsh": "_default",
    "tcsh": ""
}

and

"({})".format(" ".join(map(str, opt.choices)))) if opt.choices else f"{DEFAULT_FUNCTIONS['zsh']}",

@casperdcl
Copy link
Collaborator

casperdcl commented Jun 17, 2022

Hi sorry for the long delay.

Makes sense; interface could be shtab.add_argument_to(..., default_complete: dict[str, str]).

@tshu-w
Copy link
Author

tshu-w commented Jun 17, 2022

@casperdcl Yes, I'm looking forward to this enhancement.

@casperdcl
Copy link
Collaborator

actually... is this incompatible with #46?

casperdcl added a commit that referenced this issue Jun 17, 2022
@tshu-w
Copy link
Author

tshu-w commented Jun 18, 2022

Not really. What I want is a better default completion target in shell, #46 is to autofill the %(default)s in the help message.

What #46 wants is to look like:

# Basic syntax:
parser.add_argument("-t", "--threads", default=10, 
                    help="number of threads to use [default: %(default)s]")

# this will print help messages that look like:
-t THREADS, --threads THREADS
                        number of threads to use [default: 10]

@casperdcl casperdcl linked a pull request Jun 21, 2022 that will close this issue
@casperdcl
Copy link
Collaborator

Does #83 work for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants