Repository: iterative/dvc
Pull request: Replace hyphen with underscore for command auto-completion
Fixes #2279 

Although I found one potential bug in auto-completion.
![Screenshot from 2019-10-11 01-56-05](https://user-images.githubusercontent.com/35191225/66607163-3e741d00-ebd1-11e9-9973-82223bb651f7.png)

This is happening because of `${!options_list}` as `-h` is passed as `option_list`. Since, its variable doesn't exist, it throws this error.



Problem statement:
completion: support import-url and get-url
Add support for `dvc import-url` and `dvc get-url` into bash auto-completion script.

**What should be done:**

As to `import-url` and `get-url`, you would need to convert on `${COMP_WORDS[1]}` first that would replace all `-` with `_`, so that `option_list` becomes `_dvc_import_url` and not `_dvc_import-url`.

Hints from issue discussion:
Context https://github.com/iterative/dvc/pull/2226#issuecomment-511155421
@shcheklein Shall I take up this issue?
@algomaster99 Sure!

Commit message: Write function to replace hyphen and check for flags

Patch to review:
diff --git a/scripts/completion/dvc.bash b/scripts/completion/dvc.bash
index d9f2babf99..abe5d98c65 100644
--- a/scripts/completion/dvc.bash
+++ b/scripts/completion/dvc.bash
@@ -71,6 +71,10 @@ _dvc_version=''
 #       x="hello"
 #       ${!x} ->  ${hello} ->  "world"
 #
+replace_hyphen () {
+  echo $(echo $1 | sed 's/-/_/g')
+}
+
 _dvc () {
   local word="${COMP_WORDS[COMP_CWORD]}"
 
@@ -82,13 +86,21 @@ _dvc () {
       *)  COMPREPLY=($(compgen -W "$_dvc_commands" -- "$word")) ;;
     esac
   elif [ "${COMP_CWORD}" -eq 2 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}"
+    if [[ ${COMP_WORDS[1]:0:1} == "-" ]]; then
+      return 0
+    else
+      local options_list="_dvc_$(replace_hyphen ${COMP_WORDS[1]})"
 
-    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+      COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+    fi
   elif [ "${COMP_CWORD}" -eq 3 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}_${COMP_WORDS[2]}"
+    if [[ ${COMP_WORDS[2]:0:1} == "-" ]]; then
+      return 0
+    else
+      local options_list="_dvc_${COMP_WORDS[1]}_$(replace_hyphen ${COMP_WORDS[2]})"
 
-    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+      COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+    fi
   fi
 
   return 0


Code review comments:
diff_hunk: @@ -82,13 +86,21 @@ _dvc () {
       *)  COMPREPLY=($(compgen -W "$_dvc_commands" -- "$word")) ;;
     esac
   elif [ "${COMP_CWORD}" -eq 2 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}"
+    if [[ ${COMP_WORDS[1]:0:1} == "-" ]]; then
+      return 0
+    else
+      local options_list="_dvc_$(replace_hyphen ${COMP_WORDS[1]})"
 
-    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+      COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+    fi
   elif [ "${COMP_CWORD}" -eq 3 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}_${COMP_WORDS[2]}"
+    if [[ ${COMP_WORDS[2]:0:1} == "-" ]]; then
path: scripts/completion/dvc.bash
text: @user1:
Same here, no need for this `if` block.

diff_hunk: @@ -82,13 +86,21 @@ _dvc () {
       *)  COMPREPLY=($(compgen -W "$_dvc_commands" -- "$word")) ;;
     esac
   elif [ "${COMP_CWORD}" -eq 2 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}"
+    if [[ ${COMP_WORDS[1]:0:1} == "-" ]]; then
+      return 0
+    else
+      local options_list="_dvc_$(replace_hyphen ${COMP_WORDS[1]})"
 
-    COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+      COMPREPLY=($(compgen -W "$_dvc_global_options ${!options_list}" -- "$word"))
+    fi
   elif [ "${COMP_CWORD}" -eq 3 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}_${COMP_WORDS[2]}"
+    if [[ ${COMP_WORDS[2]:0:1} == "-" ]]; then
+      return 0
+    else
+      local options_list="_dvc_${COMP_WORDS[1]}_$(replace_hyphen ${COMP_WORDS[2]})"
path: scripts/completion/dvc.bash
text: @user1:
You need to replace hyphen in `COMP_WORDS[1]` here too, otherwise you'll get an error on `import-url` and `get-url`, because it would evaluate to `_dvc_import-url_...`, which is invalid.

diff_hunk: @@ -82,13 +86,21 @@ _dvc () {
       *)  COMPREPLY=($(compgen -W "$_dvc_commands" -- "$word")) ;;
     esac
   elif [ "${COMP_CWORD}" -eq 2 ]; then
-    local options_list="_dvc_${COMP_WORDS[1]}"
+    if [[ ${COMP_WORDS[1]:0:1} == "-" ]]; then
path: scripts/completion/dvc.bash
text: @user1:
No need for this if block, the fact that you are replacing `-` with `_` will simply evaluate the empty variable, so it is going to simply work.

diff_hunk: @@ -71,6 +71,10 @@ _dvc_version=''
 #       x="hello"
 #       ${!x} ->  ${hello} ->  "world"
 #
+replace_hyphen () {
path: scripts/completion/dvc.bash
text: @user1:
Not quite sure how bash is handling namespacing for completion scripts, so a bit worried about it colliding with something and so wouldn't place this function like that, but would just define it inside of `_dvc` function as a nested function. That way there is no chance of it colliding with anything.