Autocompletion of commands and files

[dnabanita7]

Fixes #2069
The previous code only showed the file names, it didn't autocomplete the commands. I have changed the code. How can I test this if it works or not?

[efiop]

Do we need to copy these over or could we use _dvc_add from above?

[dnabanita7]

We can use _dvc_add and use it as global variable, but I thought if anyone wants to make changes inside the function _dvc() then it will affect the global variables. Shall I change them?

[efiop]

Sorry, maybe I'm not quite following. Could you elaborate why would anyone want to change _dvc() but not change _dvc_* ?

[dnabanita7]

Later if someone makes any change to the global variables like _dvc_add or likewise, it won't affect the autocompletion. I mean we still have issues for get-url and import-url. So, if it is resolved later it won't affect the function _dvc().

[efiop]

But it should update autocompletion, shouldn't it? Maybe I'm missing something, but I still don't quite understand why you copy all of this stuff again. I don't even see where old _dvc_add is used. It used to be used in local options_list="_dvc_${COMP_WORDS[1]}" line, but now you are just copying the contents of it here. It seems like we should either remove one or the other. Your current approach is nice because it is explicit 🙂

The previous code only showed the file names, it didn't autocomplete the commands. I have changed the code.

So your PR is meant to support subcommands? Like dvc metrics [tab] -> dvc metrics show? I might be missing something, but shouldn't it be implemented as 3) in COMP_CWORD?

How can I test this if it works or not?

How did you test your last patch? 🙂

[dnabanita7]

All the dvc commands such as are of heirarchy 1 and the subcommands such as force, all-tags, show come under heirarchy 2.

It is not showing any error by using python -m tests or python3 -m tests or by running prettier.

[dnabanita7]

All the dvc commands such as are of heirarchy 1 and the subcommands such as force, all-tags, show come under heirarchy 2.

It is not showing any error by using python -m tests or python3 -m tests or by running prettier.``

[efiop]

All the dvc commands such as are of heirarchy 1 and the subcommands such as force, all-tags, show come under heirarchy 2.

But force, all-tags etc are not subcommands, they are flags. Subcommands are show, add, remove for dvc metrics, show, list for dvc pipeline and so on. And for those, subcommands are coming as level 3, not 2. 2 is indeed --force/other flags + metrics, pipeline,etc(so all the commands that have subcommands). E.g.

dvc metrics show --all-tags
[1]   [2]     [3]         [4]

so it should be something like

...
_dvc_metrics='add show remove'
_dvc_metrics_add='-t --type -x --xpath $(compgen -G *)'
_dvc_metrics_show='-t --type -x --xpath -a --all-branches -T --all-tags -R --recursive $(compgen -G *)'
_dvc_metrics_remove='$(compgen -G *)'
_dvc_pipeline='show list'
_dvc_pipeline_show=...
_dvc_pipeline_list=...

_dvc () {
  local word="${COMP_WORDS[COMP_CWORD]}"

  COMPREPLY=()

  if [ "${COMP_CWORD}" -eq 1 ]; then
    case "$word" in
      -*) COMPREPLY=($(compgen -W "$_dvc_options" -- "$word")) ;;
      *)  COMPREPLY=($(compgen -W "$_dvc_commands" -- "$word")) ;;
    esac
  elif [ "${COMP_CWORD}" -eq 2 ]; then
    local options_list="_dvc_${COMP_WORDS[1]}"

    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
  elif [ "${COMP_CWORD}" -eq 3 ]; then
    local options_list="_dvc_${COMP_WORDS[1]}_${COMP_WORDS[2]}"

    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
  fi

  return 0
}

unless I'm missing something 🙂

It is not showing any error by using python -m tests or python3 -m tests or by running prettier.

But our tests don't test bash completion. In order to test it, you need to install it as noted in https://dvc.org/doc/user-guide/autocomplete and try it out to see if it works as expected.

[efiop]

As to import-url and get-url, you will need to run a convertion on ${COMP_WORDS[1]} first that would replace all - with _, so that option_list becomes _dvc_import_url and not _dvc_import-url

[dnabanita7]

Thanks a lot. I am new to bash. But I will read these and improve.

[efiop]

Also, what is this additional indent is about in this whole code block?

Suggested change

	COMPREPLY=()
	COMPREPLY=()

[dnabanita7]

Sorry about indentation, I haven't checked them. Its only meant for clear view of the code.

[efiop]

@naba7 have you tried running this?

[dnabanita7]

Yes, and it didn't work .

[efiop]

@naba7 Your patch didn't work? Or something specific? Did the last implementation work?

[efiop]

@naba7 If you know that the patch doesn't work, please use WIP prefix in the PR name until it is ready for review. If you have any questions, also don't hesitate to ask. Otherwise it is suboptimal that someone has to come and review something that doesn't work and the fact of it not working not even noted anywhere.

[dnabanita7]

@efiop alright noted. Yes this patch didn't work. No the previous one too didn't work. I will fix some changes and using WIP in my PR. Thanks

[dnabanita7]

@efiop This script works and shows all the flags as well as sub-commands. I think to include files we have to explicitly use commands because there are some commands that lists only dvc files and directories while some lists all the files and directories. Writing it one line command is quite not possible. I mean there are many cases so if we want to automate filenames then we have to explicitly use the dvc commands. For autocompleting filenames one has to know about the files at first place, imo which is not flexible.

[efiop]

@naba7 Could you please clarify what doesn't work with files? An example(s) would be great 🙂

[efiop]

@naba7 btw, about #2226 (comment) . Do you want to adress that in this PR or later?

[dnabanita7]

@naba7 Could you please clarify what doesn't work with files? An example(s) would be great slightly_smiling_face

If in the script we use $(ls ./*) then subdirectories or files are listed, if we use $(ls ./*.*) then all files are displayed, if we use $(ls ./*.dvc) then dvc files are displayed. By displayed I mean autocompleted.

Some dvc commands such as unlock, status, repro, remove, push, pull, pipeline_show, lock, fetch, commit, config displays or autocompletes dvc files whereas unprotect, move, all subcommands of metrics, add displays or autocompletes all the files and subdirectories. So, to show either dvc files or all files we have to uses cases to check the dvc commands. That is where it needs to be explicitly coded only at level 3. If not all the dvc commands only the mentioned ones needs to be explicitly coded.

Otherwise using only $(ls ./*) will display all the files and subdirectories under the directory and it is un-necessary as git ls command will ultimately show the files and subdirectories.

[dnabanita7]

@naba7 btw, about #2226 (comment) . Do you want to adress that in this PR or later?

No, I will open an issue on this and address it later.
Thanks:)

[efiop]

@naba7

If in the script we use $(ls ./) then subdirectories or files are listed, if we use $(ls ./.) then all files are displayed, if we use $(ls ./.dvc) then dvc files are displayed. By displayed I mean autocompleted.

Some dvc commands such as unlock, status, repro, remove, push, pull, pipeline_show, lock, fetch, commit, config displays or autocompletes dvc files whereas unprotect, move, all subcommands of metrics, add displays or autocompletes all the files and subdirectories. So, to show either dvc files or all files we have to uses cases to check the dvc commands. That is where it needs to be explicitly coded only at level 3. If not all the dvc commands only the mentioned ones needs to be explicitly coded.

Otherwise using only $(ls ./*) will display all the files and subdirectories under the directory and it is un-necessary as git ls command will ultimately show the files and subdirectories.

This is actually not how it works. See $(compgen -G *) in your patch, which completes all files and directories and $(compgen -G *.dvc) which completes all dvcfiles. And those actually work already. Unless I'm not quite getting your point.

No, I will open an issue on this and address it later.
Thanks:)

Ok, thanks for the PR. 🙂

[dnabanita7]

This is actually not how it works. See $(compgen -G *) in your patch, which completes all files and directories and $(compgen -G *.dvc) which completes all dvcfiles. And those actually work already. Unless I'm not quite getting your point.

@efiop thanks :)
No, I am not talking about the global variables. I am talking about the _dvc() as in something like https://unix.stackexchange.com/questions/28283/autocomplete-of-filename-in-directory