{"thread":{"id":"17137","subject":"[PATCH] Simple update to bash completions to prevent unbound variable errors.","startedAt":"2009-01-13T02:44:19Z","lastAt":"2009-01-13T08:01:08Z","messageCount":9,"participants":["Ted Pavlic","Junio C Hamano","Boyd Stephen Smith Jr.","Teemu Likonen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"100224","messageId":"496C0003.7040909@tedpavlic.com","threadId":"17137","inReplyTo":null,"subject":"[PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T02:44:19Z","receivedAt":"2009-01-13T02:44:19Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"Below is a cleaner patch that prevents problems with unbound arguments\nwhen bash is being operated under \"set -u.\"\n\nSome comments were added to remind callers about the requirements of\neach __git* utility function.\n\nAdditionally, the completion call to bash has been modernized a bit.\n\nA vim modeline has also been added for consistency.\n\nSigned-off-by: Ted Pavlic <ted@tedpavlic.com>\n---\n  contrib/completion/git-completion.bash |   70 \n+++++++++++++++++++++++++-------\n  1 files changed, 55 insertions(+), 15 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash \nb/contrib/completion/git-completion.bash\nindex 7b074d7..619e886 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -50,9 +50,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 +69,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 +115,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 +123,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,11 +136,22 @@ __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-\tif [ $# -gt 2 ]; then\n-\t\tcur=\"$3\"\n+\tlocal one two cur=\"${COMP_WORDS[COMP_CWORD]}\" four\n+\tif [ $# -gt 0 ]; then\n+\t\tone=\"$1\"\n+\t\tif [ $# -gt 1 ]; then\n+\t\t\ttwo=\"$2\"\n+\t\t\tif [ $# -gt 2 ]; then\n+\t\t\t\tcur=\"$3\"\n+\t\t\t\tif [ $# -gt 3 ]; then\n+\t\t\t\t\tfour=\"$4\"\n+\t\t\t\tfi\n+\t\t\tfi\n+\t\tfi\n  \tfi\n  \tcase \"$cur\" in\n  \t--*=)\n@@ -143,22 +159,27 @@ __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 \"$two\" \\\n+\t\t\t-W \"$(__gitcomp_1 \"$one\" \"$four\")\" \\\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 one\n+\tif [ $# -gt 0 ]; then\n+\t\tone=\"$1\"\n+\tfi\n+\tlocal cmd i is_hash=y dir=\"$(__gitdir \"$one\")\"\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 \"$one\" 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 +189,20 @@ __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 one\n+\tif [ $# -gt 0 ]; then\n+\t\tone=\"$1\"\n+\tfi\n+\tlocal cmd i is_hash=y dir=\"$(__gitdir \"$one\")\"\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 \"$one\" 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 +212,14 @@ __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 one\n+\tif [ $# -gt 0 ]; then\n+\t\tone=\"$1\"\n+\tfi\n+\tlocal i is_hash=y dir=\"$(__gitdir \"$one\")\"\n  \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\" format refs\n  \tif [ -d \"$dir\" ]; then\n  \t\tcase \"$cur\" in\n@@ -218,6 +249,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 +258,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 +503,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 +516,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 +1801,18 @@ _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+\n+# vim:ft=sh:fdm=marker:ff=unix:noet:ts=4:sw=4\n-- \n1.6.1.87.g15624\n"},{"id":"100226","messageId":"7vy6xfew2n.fsf@gitster.siamese.dyndns.org","threadId":"17137","inReplyTo":"496C0003.7040909@tedpavlic.com","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-13T03:14:40Z","receivedAt":"2009-01-13T03:14:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Pavlic <ted@tedpavlic.com> writes:\n\n> A vim modeline has also been added for consistency.\n\nYuck.\n\n> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>\n> ---\n>  contrib/completion/git-completion.bash |   70\n> +++++++++++++++++++++++++-------\n>  1 files changed, 55 insertions(+), 15 deletions(-)\n\nDouble yuck.\n\n> diff --git a/contrib/completion/git-completion.bash\n> b/contrib/completion/git-completion.bash\n> index 7b074d7..619e886 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -50,9 +50,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 +69,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\nGood.  Would be better if you described what $1 and $2 mean.\n\n> @@ -111,7 +115,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\nI found the previous round's [ -n \"${1-}\" ] much easier to read, if we were to\ndo this.  If -n \"${1-}\", then \"$1\" is definitely set so nothing need to\nchange in the then ... else part.\n\n> +# __gitcomp_1 requires 2 arguments\n\n... and $1 and $2 mean?\n\n>  __gitcomp_1 ()\n>  {\n>  \tlocal c IFS=' '$'\\t'$'\\n'\n> @@ -131,11 +136,22 @@ __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> -\tif [ $# -gt 2 ]; then\n> -\t\tcur=\"$3\"\n> +\tlocal one two cur=\"${COMP_WORDS[COMP_CWORD]}\" four\n> +\tif [ $# -gt 0 ]; then\n> +\t\tone=\"$1\"\n> +\t\tif [ $# -gt 1 ]; then\n> +\t\t\ttwo=\"$2\"\n> +\t\t\tif [ $# -gt 2 ]; then\n> +\t\t\t\tcur=\"$3\"\n> +\t\t\t\tif [ $# -gt 3 ]; then\n> +\t\t\t\t\tfour=\"$4\"\n> +\t\t\t\tfi\n> +\t\t\tfi\n> +\t\tfi\n>  \tfi\n\nYuck.  If you are taking advantage of the fact that \"local one\"\nwill bind one to emptiness anyway, can't you do something like:\n\n\tlocal one=${1-} two=${2-} cur=${3-} four=${4-}\n\nto avoid this ugliness?\n"},{"id":"100229","messageId":"200901122157.03658.bss@iguanasuicide.net","threadId":"17137","inReplyTo":"7vy6xfew2n.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-13T03:56:59Z","receivedAt":"2009-01-13T03:56:59Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Monday 2009 January 12 21:14:40 Junio C Hamano wrote:\n>Ted Pavlic <ted@tedpavlic.com> writes:\n>> A vim modeline has also been added for consistency.\n>\n>Yuck.\n\nWhile I dislike emacs and use vim pretty much exclusively, I don't really see \nthe need for a vim modeline.  On top of that, fdm=marker is a bit silly since \nthere aren't any markers.  ff=unix and ft=sh are redundant, as any vim should \ndetect these properly, given the filename.\n\nSo, I'm slightly negative on the modeline hunk.\n\n>> @@ -111,7 +115,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>\n>I found the previous round's [ -n \"${1-}\" ] much easier to read, if we were\n> to do this.  If -n \"${1-}\", then \"$1\" is definitely set so nothing need to\n> change in the then ... else part.\n\nI found \"${1-}\" ugly, and this a bit better, but I'll defer to Junio.\n\n>> @@ -131,11 +136,22 @@ __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>> -\tif [ $# -gt 2 ]; then\n>> -\t\tcur=\"$3\"\n>> +\tlocal one two cur=\"${COMP_WORDS[COMP_CWORD]}\" four\n>> +\tif [ $# -gt 0 ]; then\n>> +\t\tone=\"$1\"\n>> +\t\tif [ $# -gt 1 ]; then\n>> +\t\t\ttwo=\"$2\"\n>> +\t\t\tif [ $# -gt 2 ]; then\n>> +\t\t\t\tcur=\"$3\"\n>> +\t\t\t\tif [ $# -gt 3 ]; then\n>> +\t\t\t\t\tfour=\"$4\"\n>> +\t\t\t\tfi\n>> +\t\t\tfi\n>> +\t\tfi\n>>  \tfi\n>\n>Yuck.\n\nDefinitely agreeing with Junio here.  This is far less ascetic than the old \npatch.  Truth be told, this whole thread would probably have been more \nproductive without me chiming in.  Sorry about that.\n\n>If you are taking advantage of the fact that \"local one\" \n>will bind one to emptiness anyway, can't you do something like:\n>\n>\tlocal one=${1-} two=${2-} cur=${3-} four=${4-}\n\nEven better to use variable names that match the usage, if possible. \n-- \nBoyd Stephen Smith Jr.                     ,= ,-_-. =. \nbss@iguanasuicide.net                     ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' \nhttp://iguanasuicide.net/                      \\_/     \n"},{"id":"100230","messageId":"496C18D8.9070707@tedpavlic.com","threadId":"17137","inReplyTo":"7vy6xfew2n.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T04:30:16Z","receivedAt":"2009-01-13T04:30:16Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":">> A vim modeline has also been added for consistency.\n>\n> Yuck.\n\nBetter that than have a mixture of spaces and tabs.\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>\n> Good.  Would be better if you described what $1 and $2 mean.\n\nIn that case, there's only $1 (the format string).\n\nNote that in most cases, I didn't know what $1 and $2 were. I was just \ntrying to fix it so that it would work on my system.\n\n>> -\t\tif [ -n \"$1\" ]; then\n>> +\t\tif [ $# -gt 0 ]&&  [ -n \"$1\" ]; then\n>\n> I found the previous round's [ -n \"${1-}\" ] much easier to read, if we were to\n> do this.  If -n \"${1-}\", then \"$1\" is definitely set so nothing need to\n> change in the then ... else part.\n\nHey -- I agree, but no one else liked ${1-}. And hg's bash completion \nseems far superior because they avoid ever having to worry about it. \nThey actually thought about the arguments ahead of time.\n\n>> +# __gitcomp_1 requires 2 arguments\n>\n> ... and $1 and $2 mean?\n\nNo clue. Patches are welcome.\n\n> Yuck.  If you are taking advantage of the fact that \"local one\"\n> will bind one to emptiness anyway, can't you do something like:\n>\n> \tlocal one=${1-} two=${2-} cur=${3-} four=${4-}\n\nWhy even use one, two three, and four then?\n\nI had ${#-} throughout, but I was told that that was ugly. So the best I \ncould do was come up with the above mess.\n\n--Ted\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":"100231","messageId":"496C19E0.6060308@tedpavlic.com","threadId":"17137","inReplyTo":"200901122157.03658.bss@iguanasuicide.net","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T04:34:40Z","receivedAt":"2009-01-13T04:34:40Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"> While I dislike emacs and use vim pretty much exclusively, I don't really see\n> the need for a vim modeline.  On top of that, fdm=marker is a bit silly since\n> there aren't any markers.  ff=unix and ft=sh are redundant, as any vim should\n> detect these properly, given the filename.\n\nWithout the modeline, the vim I was using didn't set ft=sh. It seemed \nlike the original authors were careful to use tabs everywhere, which was \nthe major reason I used the modeline. I added the fdm=marker just in \ncase someone would want to clean up the code someday and do some folding.\n\n> So, I'm slightly negative on the modeline hunk.\n\nIt was just a suggestion.\n\n>> \tlocal one=${1-} two=${2-} cur=${3-} four=${4-}\n>\n> Even better to use variable names that match the usage, if possible.\n\nI don't know much about bash completion. It would be nice if the \noriginal authors could add some semantics, as I was just trying to come \nup with scripts that would actually work with set -u.\n\n--Ted\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":"100232","messageId":"496C1D90.1020609@tedpavlic.com","threadId":"17137","inReplyTo":"496C19E0.6060308@tedpavlic.com","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-13T04:50:24Z","receivedAt":"2009-01-13T04:50:24Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"> Without the modeline, the vim I was using didn't set ft=sh. It seemed\n> like the original authors were careful to use tabs everywhere, which was\n> the major reason I used the modeline. I added the fdm=marker just in\n> case someone would want to clean up the code someday and do some folding.\n\nNOTE: On my system, I save git-completion.bash to .git_bash_completion. \nBecause of that, Vim can't ftdetect off of the file name. The modeline \nallows ft=sh even when you don't end in .sh or .bash.\n\nAn alternative (to a Vim modeline) is to put\n\n#!bash\n\nat the top of the script. That would do the same thing as the modeline \n(even though it would never actually get used by the sourced \"script\").\n\n--Ted\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":"100238","messageId":"7vr637epha.fsf@gitster.siamese.dyndns.org","threadId":"17137","inReplyTo":"496C18D8.9070707@tedpavlic.com","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-13T05:37:05Z","receivedAt":"2009-01-13T05:37:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Pavlic <ted@tedpavlic.com> writes:\n\n>>> +# __gitcomp_1 requires 2 arguments\n>>\n>> ... and $1 and $2 mean?\n>\n> No clue. Patches are welcome.\n\nTo be absolutely honest, I think people with \"set -u\" in their interactive\nenvironment are sick.  Bourne shells have been substituting unset\nvariables to empty string for eons, and this is not _my_ itch to scratch.\n\nIt seems to be yours, though, and I was merely trying to help.\n"},{"id":"100240","messageId":"87hc43emr7.fsf@iki.fi","threadId":"17137","inReplyTo":"496C1D90.1020609@tedpavlic.com","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-01-13T06:35:56Z","receivedAt":"2009-01-13T06:35:56Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Ted Pavlic (2009-01-12 23:50 -0500) wrote:\n\n> NOTE: On my system, I save git-completion.bash to .git_bash_completion.\n> Because of that, Vim can't ftdetect off of the file name. The modeline\n> allows ft=sh even when you don't end in .sh or .bash.\n>\n> An alternative (to a Vim modeline) is to put\n>\n> #!bash\n>\n> at the top of the script. That would do the same thing as the modeline\n> (even though it would never actually get used by the sourced \"script\").\n\nAnother way is to set filetype detection locally. This way the project\nfiles don't get filled with editor-specific stuff. You may want add\nsomething like the following to your ~/.vim/filetype.vim file:\n\n    augroup filetypedetect\n            autocmd BufNewFile,BufRead .git_bash_completion  setl ft=sh\n    augroup END\n"},{"id":"100247","messageId":"7vab9veit7.fsf@gitster.siamese.dyndns.org","threadId":"17137","inReplyTo":"496C1D90.1020609@tedpavlic.com","subject":"Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-13T08:01:08Z","receivedAt":"2009-01-13T08:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Pavlic <ted@tedpavlic.com> writes:\n\n> An alternative (to a Vim modeline) is to put\n>\n> #!bash\n\nI actually like this much better than \"# vim:\" thing.\n\nIt talks about WHAT the file is (\"It is a bash scriptlet\"), as opposed to\n\"# vim:\" that tells HOW one user wants to edit the file (\"with this\nsettings I use vim to edit this file\").  In case somebody misunderstood\nme, my \"Yuck\" was not about Emacs vs vi holy war. [*1*].\n\nAs people noted, it may not have any meaning to the shell, but it serves\nas an important documentation to humans.  As the maintainer, I try to be\nreasonably strict about keeping our shell scripts free of bash-ism to help\npeople on platforms without bash, but this file is about bash (and bash\nonly).  We are free to use all bash-isms we usually try to stay away from\nin our scripts marked with #!/bin/sh (namely, \"local\", \"let\", shell\narrays, and substitutions outside POSIX such as ${parameter:offset:length}\nsubstring, ${#parameter} length, and ${parameter/pattern/string} regexp\nreplacement), as long as they are compatible across recent bash versions.\n\nIf tools like vim notice the same hint that says \"this is bash\" and\nadjusts its behaviour accordingly, that is great.\n\nEven though I've grown to know Shawn well enough to be able to tell that\ncertain kinds of changes would get his Ack and often apply patches myself\nwithout waiting him to give his Ack, the completion script is ultimately\nhis code and you do not necessarily have to make me like it.\n\nYour updated patch still had [ $# -eq 0 -o -z \"$1\" ] even though the\ncommit log message (which is not the place to describe changes between v1\nand v2 submissions, by the way) claimed that you replaced them with\n\"${1-}\".  I am guessing it would need at least one more iteration to clean\nup, but I think this patch is improving _provided if_ supporting \"set -u\"\nin the user's interactive environment is a good idea to begin with.\n\nI just hope that next person who complains about bash completion scripts\nwould not say that he has \"set -e\" in his interactive environment ;-) I\npersonally consider that \"set -u\" is equally silly, but it probably is\njust me.\n\n[Footnote]\n\n*1* For the same reason, Local Variables section to please Emacs is\nequally offending; it tends to be much bigger which is even worse.\n"}]}