{"thread":{"id":"17141","subject":"[PATCH] Simplest update to bash completions to prevent unbounded variable errors","startedAt":"2009-01-13T04:58:07Z","lastAt":"2009-01-13T15:33:08Z","messageCount":4,"participants":["Ted Pavlic","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"100234","messageId":"496C1F5F.9020604@tedpavlic.com","threadId":"17141","inReplyTo":null,"subject":"[PATCH] Simplest update to bash completions to prevent unbounded variable errors","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T04:58:07Z","receivedAt":"2009-01-13T04:58:07Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"Another try at fixing bash completions in \"set -u\" environments.\n\nHere, I've gone back to changing $# to ${#-}, but only where necessary.\n\nAdditionally added some comments and omitted things like Vim modelines.\n\n\nSigned-off-by: Ted Pavlic <ted@tedpavlic.com>\n---\n  contrib/completion/git-completion.bash |   42 \n++++++++++++++++++++++---------\n  1 files changed, 30 insertions(+), 12 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash \nb/contrib/completion/git-completion.bash\nindex 7b074d7..323829e 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1,3 +1,4 @@\n+#!bash\n  #\n  # bash completion support for core Git.\n  #\n@@ -50,9 +51,11 @@ case \"$COMP_WORDBREAKS\" in\n  *)   COMP_WORDBREAKS=\"$COMP_WORDBREAKS:\"\n  esac\n\n+# __gitdir accepts 0 or 1 arguments (i.e., location)\n+# returns location of .git repo\n  __gitdir ()\n  {\n-\tif [ -z \"$1\" ]; then\n+\tif [ $# -eq 0 ] || [ -z \"$1\" ]; then\n  \t\tif [ -n \"$__git_dir\" ]; then\n  \t\t\techo \"$__git_dir\"\n  \t\telif [ -d .git ]; then\n@@ -67,6 +70,8 @@ __gitdir ()\n  \tfi\n  }\n\n+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)\n+# returns text to add to bash PS1 prompt (includes branch name)\n  __git_ps1 ()\n  {\n  \tlocal g=\"$(git rev-parse --git-dir 2>/dev/null)\"\n@@ -111,7 +116,7 @@ __git_ps1 ()\n  \t\t\tfi\n  \t\tfi\n\n-\t\tif [ -n \"$1\" ]; then\n+\t\tif [ $# -gt 0 ] && [ -n \"$1\" ]; then\n  \t\t\tprintf \"$1\" \"${b##refs/heads/}$r\"\n  \t\telse\n  \t\t\tprintf \" (%s)\" \"${b##refs/heads/}$r\"\n@@ -119,6 +124,7 @@ __git_ps1 ()\n  \tfi\n  }\n\n+# __gitcomp_1 requires 2 arguments\n  __gitcomp_1 ()\n  {\n  \tlocal c IFS=' '$'\\t'$'\\n'\n@@ -131,6 +137,8 @@ __gitcomp_1 ()\n  \tdone\n  }\n\n+# __gitcomp accepts 1, 2, 3, or 4 arguments\n+# generates completion reply with compgen\n  __gitcomp ()\n  {\n  \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n@@ -143,22 +151,23 @@ __gitcomp ()\n  \t\t;;\n  \t*)\n  \t\tlocal IFS=$'\\n'\n-\t\tCOMPREPLY=($(compgen -P \"$2\" \\\n-\t\t\t-W \"$(__gitcomp_1 \"$1\" \"$4\")\" \\\n+\t\tCOMPREPLY=($(compgen -P \"${2-}\" \\\n+\t\t\t-W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n  \t\t\t-- \"$cur\"))\n  \t\t;;\n  \tesac\n  }\n\n+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)\n  __git_heads ()\n  {\n-\tlocal cmd i is_hash=y dir=\"$(__gitdir \"$1\")\"\n+\tlocal cmd i is_hash=y dir=\"$(__gitdir \"${1-}\")\"\n  \tif [ -d \"$dir\" ]; then\n  \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n  \t\t\trefs/heads\n  \t\treturn\n  \tfi\n-\tfor i in $(git ls-remote \"$1\" 2>/dev/null); do\n+\tfor i in $(git ls-remote \"${1-}\" 2>/dev/null); do\n  \t\tcase \"$is_hash,$i\" in\n  \t\ty,*) is_hash=n ;;\n  \t\tn,*^{}) is_hash=y ;;\n@@ -168,15 +177,16 @@ __git_heads ()\n  \tdone\n  }\n\n+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)\n  __git_tags ()\n  {\n-\tlocal cmd i is_hash=y dir=\"$(__gitdir \"$1\")\"\n+\tlocal cmd i is_hash=y dir=\"$(__gitdir \"${1-}\")\"\n  \tif [ -d \"$dir\" ]; then\n  \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n  \t\t\trefs/tags\n  \t\treturn\n  \tfi\n-\tfor i in $(git ls-remote \"$1\" 2>/dev/null); do\n+\tfor i in $(git ls-remote \"${1-}\" 2>/dev/null); do\n  \t\tcase \"$is_hash,$i\" in\n  \t\ty,*) is_hash=n ;;\n  \t\tn,*^{}) is_hash=y ;;\n@@ -186,9 +196,10 @@ __git_tags ()\n  \tdone\n  }\n\n+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)\n  __git_refs ()\n  {\n-\tlocal i is_hash=y dir=\"$(__gitdir \"$1\")\"\n+\tlocal i is_hash=y dir=\"$(__gitdir \"${1-}\")\"\n  \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\" format refs\n  \tif [ -d \"$dir\" ]; then\n  \t\tcase \"$cur\" in\n@@ -218,6 +229,7 @@ __git_refs ()\n  \tdone\n  }\n\n+# __git_refs2 requires 1 argument (to pass to __git_refs)\n  __git_refs2 ()\n  {\n  \tlocal i\n@@ -226,6 +238,7 @@ __git_refs2 ()\n  \tdone\n  }\n\n+# __git_refs_remotes requires 1 argument (to pass to ls-remote)\n  __git_refs_remotes ()\n  {\n  \tlocal cmd i is_hash=y\n@@ -470,6 +483,7 @@ __git_aliases ()\n  \tdone\n  }\n\n+# __git_aliased_command requires 1 argument\n  __git_aliased_command ()\n  {\n  \tlocal word cmdline=$(git --git-dir=\"$(__gitdir)\" \\\n@@ -482,6 +496,7 @@ __git_aliased_command ()\n  \tdone\n  }\n\n+# __git_find_subcommand requires 1 argument\n  __git_find_subcommand ()\n  {\n  \tlocal word subcommand c=1\n@@ -1766,13 +1781,16 @@ _gitk ()\n  \t__git_complete_revlist\n  }\n\n-complete -o default -o nospace -F _git git\n-complete -o default -o nospace -F _gitk gitk\n+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \\\n+\t|| complete -o default -o nospace -F _git git\n+complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \\\n+\t|| complete -o default -o nospace -F _gitk gitk\n\n  # The following are necessary only for Cygwin, and only are needed\n  # when the user has tab-completed the executable name and consequently\n  # included the '.exe' suffix.\n  #\n  if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n-complete -o default -o nospace -F _git git.exe\n+complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \\\n+\t|| complete -o default -o nospace -F _git git.exe\n  fi\n-- \n1.6.1.87.g15624\n"},{"id":"100279","messageId":"20090113152047.GO10179@spearce.org","threadId":"17141","inReplyTo":"496C1F5F.9020604@tedpavlic.com","subject":"Re: [PATCH] Simplest update to bash completions to prevent unbounded variable errors","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-13T15:20:47Z","receivedAt":"2009-01-13T15:20:47Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ted Pavlic <ted@tedpavlic.com> wrote:\n> Another try at fixing bash completions in \"set -u\" environments.\n\nI agree with Junio; setting -u in your interactive shell is as bad\nas export CDPATH.  Its crazy.\n\n> Additionally added some comments and omitted things like Vim modelines.\n\nThese are orthogonal to the -u corrections.  They should be in a\ndifferent patch.  The comments are wecome.  The '#!bash' looks like\na good idea.  But a vim specific modeline, I don't like, for the\nreasons Junio has already stated.\n\n> +# __gitdir accepts 0 or 1 arguments (i.e., location)\n> +# returns location of .git repo\n>  __gitdir ()\n>  {\n> -\tif [ -z \"$1\" ]; then\n> +\tif [ $# -eq 0 ] || [ -z \"$1\" ]; then\n\nThis is one of those places where [ -z \"${1-}\" ] is likely easier\nto read then the || usage you have introduced.  We don't care if\nwe got no args, or we got one that is the empty string, either way\nthe $1 cannot be a gitdir and we need to guess it.\n\n> @@ -111,7 +116,7 @@ __git_ps1 ()\n>  \t\t\tfi\n>  \t\tfi\n>\n> -\t\tif [ -n \"$1\" ]; then\n> +\t\tif [ $# -gt 0 ] && [ -n \"$1\" ]; then\n>  \t\t\tprintf \"$1\" \"${b##refs/heads/}$r\"\n\nEh, I'd rather see [ -n \"${1-}\" ] over the && test.\n\n> -complete -o default -o nospace -F _git git\n> -complete -o default -o nospace -F _gitk gitk\n> +complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \\\n> +\t|| complete -o default -o nospace -F _git git\n> +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \\\n> +\t|| complete -o default -o nospace -F _gitk gitk\n\nWhy are we switching to bashdefault?  Is this an unrelated change\nfrom the -u stuff and should go into its own commit, with its own\njustification?\n\n-- \nShawn.\n"},{"id":"100280","messageId":"496CB3B0.7010605@tedpavlic.com","threadId":"17141","inReplyTo":"20090113152047.GO10179@spearce.org","subject":"Re: [PATCH] Simplest update to bash completions to prevent unbounded variable errors","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T15:30:56Z","receivedAt":"2009-01-13T15:30:56Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":">> Another try at fixing bash completions in \"set -u\" environments.\n> I agree with Junio; setting -u in your interactive shell is as bad\n> as export CDPATH.  Its crazy.\n\nThis whole series of patches was inspired by a group of workstations at \na university that set -u by default for all users.\n\nAdditionally, doesn't \"set -u\" make tcsh users feel more at home in \nbash? Certainly other shells have this same behavior in their \ninteractive modes.\n\n>> Additionally added some comments and omitted things like Vim modelines.\n>\n> These are orthogonal to the -u corrections.  They should be in a\n> different patch.  The comments are wecome.  The '#!bash' looks like\n> a good idea.  But a vim specific modeline, I don't like, for the\n> reasons Junio has already stated.\n\nOK. Can do.\n\n>> -\tif [ -z \"$1\" ]; then\n>> +\tif [ $# -eq 0 ] || [ -z \"$1\" ]; then\n>\n> This is one of those places where [ -z \"${1-}\" ] is likely easier\n\nThat was a mistake. I missed that hunk. I meant to use the ${1-}.\n\n>> +\t\tif [ $# -gt 0 ]&&  [ -n \"$1\" ]; then\n>\n> Eh, I'd rather see [ -n \"${1-}\" ] over the&&  test.\n\nAgain, my mistake. It was late and I missed it.\n\n\n>> +complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \\\n>> +\t|| complete -o default -o nospace -F _git git\n>> +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \\\n>> +\t|| complete -o default -o nospace -F _gitk gitk\n>\n> Why are we switching to bashdefault?  Is this an unrelated change\n> from the -u stuff and should go into its own commit, with its own\n> justification?\n\nOk.\n\n From what I understand, normal bash completion is like setting \"-o \nbashdefault -o default\". That is, it tries the bash completions first \nbefore going to the filename completion. This change makes it so that \ngit jumps back to bash completion if nothing git-specific is found. If \nnothing bash-specific is found, it will go back to standard default \nfilename completion.\n\n--Ted\n\n\n\n-- \nTed Pavlic <ted@tedpavlic.com>\n\n   Please visit my ALS association page:\n         http://web.alsa.org/goto/tedpavlic\n   My family appreciates your support in the fight to defeat ALS.\n"},{"id":"100281","messageId":"20090113153308.GP10179@spearce.org","threadId":"17141","inReplyTo":"496CB3B0.7010605@tedpavlic.com","subject":"Re: [PATCH] Simplest update to bash completions to prevent unbounded variable errors","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-13T15:33:08Z","receivedAt":"2009-01-13T15:33:08Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ted Pavlic <ted@tedpavlic.com> wrote:\n>>> Another try at fixing bash completions in \"set -u\" environments.\n>> I agree with Junio; setting -u in your interactive shell is as bad\n>> as export CDPATH.  Its crazy.\n>\n> This whole series of patches was inspired by a group of workstations at  \n> a university that set -u by default for all users.\n\nThe changes look less nasty than I originally thought.  If you can\nsplit the history out and justify the changes in the corresponding\ncommit messages, I think I can ACK the series.\n\n-- \nShawn.\n"}]}