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

Autocompletion not filling dependencies #4552

Closed
geblanco opened this issue Sep 9, 2020 · 10 comments · Fixed by #4659
Closed

Autocompletion not filling dependencies #4552

geblanco opened this issue Sep 9, 2020 · 10 comments · Fixed by #4659
Labels
completion bash and zsh completion scripts enhancement Enhances DVC good first issue help wanted p2-medium Medium priority, should be done, but less important

Comments

@geblanco
Copy link
Contributor

geblanco commented Sep 9, 2020

Bug Report

Please provide information about your setup

Output of dvc version:

$ dvc version
DVC version: 1.6.6 (pip)
---------------------------------
Platform: Python 3.8.3 on Linux-5.4.49-1-lts-x86_64-with-glibc2.2.5
Supports: http, https, s3
Cache types: symlink
Repo: dvc, git

Additional Information (if any):

I am using zsh, when trying tab-autocompletion for run command, it does not fill anything for dependencies.
Also, when trying repro command, stages do not get offered either, just files and directories in the cwd.

Steps to reproduce: I don't think this is a project specific bug, so probably a fresh start could do.

@efiop efiop added the completion bash and zsh completion scripts label Sep 9, 2020
@efiop
Copy link
Contributor

efiop commented Sep 9, 2020

Stages are not auto-completed because of a missing functionality #3743 required for that.

Not completing deps is indeed a bug, looks like we are missing

).complete = completion.FILE
in
https://github.com/iterative/dvc/blob/master/dvc/command/run.py for deps. So we just need to

    run_parser.add_argument(
        "-d",
        "--deps",
        action="append",
        default=[],
        help="Declare dependencies for reproducible cmd.",
        metavar="<path>",
    ).complete = completion.FILE

to make that work.

@efiop efiop added enhancement Enhances DVC good first issue help wanted p2-medium Medium priority, should be done, but less important labels Sep 9, 2020
@geblanco
Copy link
Contributor Author

Hello @efiop,

This is a friendly ping, how is this going? The error persist in the latest releases.

Best,

@efiop
Copy link
Contributor

efiop commented Oct 1, 2020

@geblanco No progress so far. We would be glad to accept a contribution though 🙂

@geblanco
Copy link
Contributor Author

geblanco commented Oct 2, 2020

@efiop following your guide, here is the fixed code.

I have further considerations (I can contribute on that also), also for the run command:

  • Parameters are a list of params, optionally pertaining to a file.
  • Outputs is a filename, which can be inside an already created directory.

Could it be reasonable to autocomplete in the same way (with FILE/DIR autocompletion?). Both could benefit from autocompleting with already created files/directories.

Lastly, the ultimate part of the run command is the actual command/script to run. Is there any way in shtab for default completions? e.g.: when typing python ... autocompletion works, and is the default, is there a way to indicate that in shtab?

@efiop
Copy link
Contributor

efiop commented Oct 3, 2020

@geblanco Thank you so much for the PR! 🙏

Indeed, those dvc run options would also benefit from such completion. Please feel free to add them in the same PR you have already.

I don't think shtab supports command completion yet 🙁 But could probably be done somehow, not sure about the complexity there though. CC @casperdcl

@casperdcl
Copy link
Contributor

  1. having shtab complete files by default - do open a feature request at https://github.com/iterative/shtab/issues if you need to. Don't think that's an approach dvc should use, though. In general also I think it's probably not a good idea. Whitelist completions make more sense that blacklist. If you need to enable completions by default you could always wrap the add_argument() function to do this.
  2. stage completion would depend on Quick listing of stages #3743 as @efiop said

@geblanco
Copy link
Contributor Author

geblanco commented Oct 5, 2020

@efiop okay, I will add two commits to the PR, one for fixing outputs and another for parameters. Is it okay to fill the latter with filenames even though parameters can be outside files?

@casperdcl I'm not entirely sure what you mean by whitelist or blacklist completions, I understand the concept but have no idea how it translates into code, also, is there any example of a wrapped add_argument() that I can see to get it? Do you think the best approach for default completions in the command part would be wrapping the function?

geblanco added a commit to geblanco/dvc that referenced this issue Oct 5, 2020
geblanco added a commit to geblanco/dvc that referenced this issue Oct 5, 2020
efiop pushed a commit that referenced this issue Oct 5, 2020
* completions: fix run command dependencies autocompletion

Fixes #4552

* completions: fix run command outputs autocompletion

Fixes #4552

* completions: fix run command parameters autocompletion

Fixes #4552
@efiop
Copy link
Contributor

efiop commented Oct 5, 2020

Is it okay to fill the latter with filenames even though parameters can be outside files?

Yeah, that's okay for now, as we don't even have a command to aid the listing of params in the file, so that will do for now. Thank you!

@casperdcl
Copy link
Contributor

casperdcl commented Oct 5, 2020

@geblanco going off on a tangent, but I mean this sort of thing. Perhaps could be mentioned in shtab docs...

def add_argument_wrapper(func, default=shtab.FILE):
	@functools.wraps(func)
	def inner(*args, **kwargs):
		func(*args, **kwargs).complete = default
	return inner


add_argument = add_argument_wrapper(parser.add_argument)

# parser.add_argument("-s", "--such-fun", ...).complete = shtab.FILE  # old way
add_argument("-s", "--such-fun", ...)  # new way

@geblanco
Copy link
Contributor Author

geblanco commented Oct 5, 2020

Okay, I see...

Pretty nice library shtab :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completion bash and zsh completion scripts enhancement Enhances DVC good first issue help wanted p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants