{"thread":{"id":"17121","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","startedAt":"2009-01-12T19:58:28Z","lastAt":"2009-01-12T21:51:44Z","messageCount":14,"participants":["Boyd Stephen Smith Jr.","Adeodato Simó","Ted Pavlic","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"100993","messageId":"496BA0E4.2040607@tedpavlic.com","threadId":"17121","inReplyTo":null,"subject":"[PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-12T19:58:28Z","receivedAt":"2009-01-12T19:58:28Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"In bash, \"set -u\" gives an error when a variable is unbound. In this\ncase, the bash completion script included in the git/contrib directory\nproduces several errors.\n\nThe attached patch replaces things like\n\n if [ -z \"$1\" ]\n\nwith\n\n if [ -z \"${1-}\" ]\n\nso that the unbound variable returns an empty value. Hence, the\ncompletion script will now work even \"set -u\" set.\n\nSigned-off-by: Ted Pavlic \n---\n contrib/completion/git-completion.bash | 68 \n++++++++++++++++----------------\n 1 files changed, 34 insertions(+), 34 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash \nb/contrib/completion/git-completion.bash\nindex 7b074d7..50e345f 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -52,25 +52,25 @@ esac\n\n __gitdir ()\n {\n-\tif [ -z \"$1\" ]; then\n-\t\tif [ -n \"$__git_dir\" ]; then\n+\tif [ -z \"${1-}\" ]; then\n+\t\tif [ -n \"${__git_dir-}\" ]; then\n \t\t\techo \"$__git_dir\"\n \t\telif [ -d .git ]; then\n \t\t\techo .git\n \t\telse\n \t\t\tgit rev-parse --git-dir 2>/dev/null\n \t\tfi\n-\telif [ -d \"$1/.git\" ]; then\n-\t\techo \"$1/.git\"\n+\telif [ -d \"${1-}/.git\" ]; then\n+\t\techo \"${1-}/.git\"\n \telse\n-\t\techo \"$1\"\n+\t\techo \"${1-}\"\n \tfi\n }\n\n __git_ps1 ()\n {\n \tlocal g=\"$(git rev-parse --git-dir 2>/dev/null)\"\n-\tif [ -n \"$g\" ]; then\n+\tif [ -n \"${g-}\" ]; then\n \t\tlocal r\n \t\tlocal b\n \t\tif [ -d \"$g/rebase-apply\" ]\n@@ -111,8 +111,8 @@ __git_ps1 ()\n \t\t\tfi\n \t\tfi\n\n-\t\tif [ -n \"$1\" ]; then\n-\t\t\tprintf \"$1\" \"${b##refs/heads/}$r\"\n+\t\tif [ -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 \t\tfi\n@@ -122,11 +122,11 @@ __git_ps1 ()\n __gitcomp_1 ()\n {\n \tlocal c IFS=' '$'\\t'$'\\n'\n-\tfor c in $1; do\n-\t\tcase \"$c$2\" in\n-\t\t--*=*) printf %s$'\\n' \"$c$2\" ;;\n-\t\t*.) printf %s$'\\n' \"$c$2\" ;;\n-\t\t*) printf %s$'\\n' \"$c$2 \" ;;\n+\tfor c in ${1-}; do\n+\t\tcase \"$c${2-}\" in\n+\t\t--*=*) printf %s$'\\n' \"$c${2-}\" ;;\n+\t\t*.) printf %s$'\\n' \"$c${2-}\" ;;\n+\t\t*) printf %s$'\\n' \"$c${2-} \" ;;\n \t\tesac\n \tdone\n }\n@@ -135,7 +135,7 @@ __gitcomp ()\n {\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tif [ $# -gt 2 ]; then\n-\t\tcur=\"$3\"\n+\t\tcur=\"${3-}\"\n \tfi\n \tcase \"$cur\" in\n \t--*=)\n@@ -143,8 +143,8 @@ __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@@ -152,13 +152,13 @@ __gitcomp ()\n\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@@ -170,13 +170,13 @@ __git_heads ()\n\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@@ -188,7 +188,7 @@ __git_tags ()\n\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@@ -221,7 +221,7 @@ __git_refs ()\n __git_refs2 ()\n {\n \tlocal i\n-\tfor i in $(__git_refs \"$1\"); do\n+\tfor i in $(__git_refs \"${1-}\"); do\n \t\techo \"$i:$i\"\n \tdone\n }\n@@ -229,11 +229,11 @@ __git_refs2 ()\n __git_refs_remotes ()\n {\n \tlocal cmd i is_hash=y\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\tn,refs/heads/*)\n \t\t\tis_hash=y\n-\t\t\techo \"$i:refs/remotes/$1/${i#refs/heads/}\"\n+\t\t\techo \"$i:refs/remotes/${1-}/${i#refs/heads/}\"\n \t\t\t;;\n \t\ty,*) is_hash=n ;;\n \t\tn,*^{}) is_hash=y ;;\n@@ -264,7 +264,7 @@ __git_remotes ()\n\n __git_merge_strategies ()\n {\n-\tif [ -n \"$__git_merge_strategylist\" ]; then\n+\tif [ -n \"${__git_merge_strategylist-}\" ]; then\n \t\techo \"$__git_merge_strategylist\"\n \t\treturn\n \tfi\n@@ -350,7 +350,7 @@ __git_complete_revlist ()\n\n __git_all_commands ()\n {\n-\tif [ -n \"$__git_all_commandlist\" ]; then\n+\tif [ -n \"${__git_all_commandlist-}\" ]; then\n \t\techo \"$__git_all_commandlist\"\n \t\treturn\n \tfi\n@@ -368,7 +368,7 @@ __git_all_commandlist=\"$(__git_all_commands \n2>/dev/null)\"\n\n __git_porcelain_commands ()\n {\n-\tif [ -n \"$__git_porcelain_commandlist\" ]; then\n+\tif [ -n \"${__git_porcelain_commandlist-}\" ]; then\n \t\techo \"$__git_porcelain_commandlist\"\n \t\treturn\n \tfi\n@@ -473,7 +473,7 @@ __git_aliases ()\n __git_aliased_command ()\n {\n \tlocal word cmdline=$(git --git-dir=\"$(__gitdir)\" \\\n-\t\tconfig --get \"alias.$1\")\n+\t\tconfig --get \"alias.${1-}\")\n \tfor word in $cmdline; do\n \t\tif [ \"${word##-*}\" ]; then\n \t\t\techo $word\n@@ -488,7 +488,7 @@ __git_find_subcommand ()\n\n \twhile [ $c -lt $COMP_CWORD ]; do\n \t\tword=\"${COMP_WORDS[c]}\"\n-\t\tfor subcommand in $1; do\n+\t\tfor subcommand in ${1-}; do\n \t\t\tif [ \"$subcommand\" = \"$word\" ]; then\n \t\t\t\techo \"$subcommand\"\n \t\t\t\treturn\n@@ -599,7 +599,7 @@ _git_bisect ()\n\n \tlocal subcommands=\"start bad good skip reset visualize replay log run\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n-\tif [ -z \"$subcommand\" ]; then\n+\tif [ -z \"${subcommand-}\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \t\treturn\n \tfi\n@@ -1371,7 +1371,7 @@ _git_remote ()\n {\n \tlocal subcommands=\"add rm show prune update\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n-\tif [ -z \"$subcommand\" ]; then\n+\tif [ -z \"${subcommand-}\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \t\treturn\n \tfi\n@@ -1500,7 +1500,7 @@ _git_stash ()\n {\n \tlocal subcommands='save list show apply clear drop pop create branch'\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n-\tif [ -z \"$subcommand\" ]; then\n+\tif [ -z \"${subcommand-}\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \telse\n \t\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n@@ -1552,7 +1552,7 @@ _git_svn ()\n \t\tproplist show-ignore show-externals\n \t\t\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n-\tif [ -z \"$subcommand\" ]; then\n+\tif [ -z \"${subcommand-}\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \telse\n \t\tlocal remote_opts=\"--username= --config-dir= --no-auth-cache\"\n@@ -1672,7 +1672,7 @@ _git ()\n \t\tc=$((++c))\n \tdone\n\n-\tif [ -z \"$command\" ]; then\n+\tif [ -z \"${command-}\" ]; then\n \t\tcase \"${COMP_WORDS[COMP_CWORD]}\" in\n \t\t--*=*) COMPREPLY=() ;;\n \t\t--*) __gitcomp \"\n-- \n1.6.1.87.g15624\n"},{"id":"100156","messageId":"200901121435.35547.bss@iguanasuicide.net","threadId":"17121","inReplyTo":"496BA0E4.2040607@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-12T20:35:35Z","receivedAt":"2009-01-12T20:35:35Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Monday 2009 January 12 13:58:28 you wrote:\n>In bash, \"set -u\" gives an error when a variable is unbound. In this\n>case, the bash completion script included in the git/contrib directory\n>produces several errors.\n>\n>The attached patch replaces things like\n>\n> if [ -z \"$1\" ]\n>\n>with\n>\n> if [ -z \"${1-}\" ]\n\nThat looks ugly to me. Any reason we shouldn't just \"set +u\" at the top of \nthe script?\n-- \nBoyd Stephen Smith Jr. ,= ,-_-. =. \nbss@iguanasuicide.net ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' \nhttp://iguanasuicide.net/ \\_/ \n"},{"id":"100157","messageId":"20090112204030.GA23327@chistera.yi.org","threadId":"17121","inReplyTo":"200901121435.35547.bss@iguanasuicide.net","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-12T20:40:30Z","receivedAt":"2009-01-12T20:40:30Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* Boyd Stephen Smith Jr. [Mon, 12 Jan 2009 14:35:35 -0600]:\n\n> >The attached patch replaces things like\n\n> > if [ -z \"$1\" ]\n\n> >with\n\n> > if [ -z \"${1-}\" ]\n\n> That looks ugly to me. Any reason we shouldn't just \"set +u\" at the top of \n> the script?\n\n`set +u` affects the shell globally, not just to the sourced file. If\nyou do that, you must be aware that you'll be preventing people from\nrunning their shell in `set -u` mode. (Merely stating a fact here, not\ngiving any opinion.)\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \nThe problem I have with making an intelligent statement is that some\npeople then think that it's not an isolated occurrance.\n -- Simon Travaglia\n"},{"id":"100160","messageId":"496BB204.2040109@tedpavlic.com","threadId":"17121","inReplyTo":"200901121435.35547.bss@iguanasuicide.net","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-12T21:11:32Z","receivedAt":"2009-01-12T21:11:32Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":">> if [ -z \"${1-}\" ]\n>\n> That looks ugly to me. Any reason we shouldn't just \"set +u\" at the top of\n> the script?\n\nAs already discussed, because the script must be sourced, then the \"set \n+u\" has global scope.\n\nI suppose that the option could be tested and then reset as appropriate \nat the end of the script.\n\n(note: for some reason Mercurial's bash completion script does not have \nthis problem; they use $1 directly without bash complaining)\n\n-- \nTed Pavlic \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":"100162","messageId":"496BB442.90107@tedpavlic.com","threadId":"17121","inReplyTo":"496BB204.2040109@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-12T21:21:06Z","receivedAt":"2009-01-12T21:21:06Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"> (note: for some reason Mercurial's bash completion script does not have\n> this problem; they use $1 directly without bash complaining)\n\nIt appears like they use\n\n\tcomplete -o bashdefault\n\nwhereas Git's uses\n\n\tcomplete -o default\n\nI think that's the difference.\n\n\n-- \nTed Pavlic \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":"100163","messageId":"20090112212544.GA24941@chistera.yi.org","threadId":"17121","inReplyTo":"496BB204.2040109@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-12T21:25:44Z","receivedAt":"2009-01-12T21:25:44Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* Ted Pavlic [Mon, 12 Jan 2009 16:11:32 -0500]:\n\n>> That looks ugly to me. Any reason we shouldn't just \"set +u\" at the top of\n>> the script?\n\n> As already discussed, because the script must be sourced, then the \"set \n> +u\" has global scope.\n\n> I suppose that the option could be tested and then reset as appropriate \n> at the end of the script.\n\nThat does not help, because appart from being global, it of course takes\neffect at run time. In other words, it doesn't matter if set -u is\nactive or not at function definition time, but at function invoation\ntime.\n\n> (note: for some reason Mercurial's bash completion script does not have \n> this problem; they use $1 directly without bash complaining)\n\nBecause (from a quick look) their completion script never expands a\nvariable which is not known to be set.\n\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \nA hacker does for love what other would not do for money.\n"},{"id":"100164","messageId":"200901121527.21818.bss@iguanasuicide.net","threadId":"17121","inReplyTo":"20090112204030.GA23327@chistera.yi.org","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-12T21:27:16Z","receivedAt":"2009-01-12T21:27:16Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Monday 2009 January 12 14:40:30 Adeodato Simó wrote:\n>* Boyd Stephen Smith Jr. [Mon, 12 Jan 2009 14:35:35 -0600]:\n>> >The attached patch replaces things like\n>> >\n>> > if [ -z \"$1\" ]\n>> >\n>> >with\n>> >\n>> > if [ -z \"${1-}\" ]\n>>\n>> That looks ugly to me. Any reason we shouldn't just \"set +u\" at the top\n>> of the script?\n>\n>`set +u` affects the shell globally, not just to the sourced file. If\n>you do that, you must be aware that you'll be preventing people from\n>running their shell in `set -u` mode. (Merely stating a fact here, not\n>giving any opinion.)\n\nI'm not familiar with bash completion exception as a user, I didn't realize \nall these functions had to be sourced into the current shell.\n\nWell, if the user want to run in \"set -u\" mode preventing it is bogus, IMO. \nWe could use subshells and unset at the top of _git and _gitk functions, that \nwould be only a +6/-4 patch. It would also not be something future \ncontributors have to think (much) about.\n-- \nBoyd Stephen Smith Jr. ,= ,-_-. =. \nbss@iguanasuicide.net ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' \nhttp://iguanasuicide.net/ \\_/ \n"},{"id":"100166","messageId":"20090112213149.GL10179@spearce.org","threadId":"17121","inReplyTo":"200901121527.21818.bss@iguanasuicide.net","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-12T21:31:49Z","receivedAt":"2009-01-12T21:31:49Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Boyd Stephen Smith Jr.\" wrote:\n> \n> Well, if the user want to run in \"set -u\" mode preventing it is bogus, IMO. \n> We could use subshells and unset at the top of _git and _gitk functions, that \n> would be only a +6/-4 patch. It would also not be something future \n> contributors have to think (much) about.\n\nRunning in subshells is a bad idea. We'd likely lose access\nto the completion word array, and it would take a lot longer per\ncompletion because you need to spin-up and tear-down that subshell.\n\nWe have spent some time to make the current completion code use as\nfew forks as possible to get the results we want, because it runs\nfaster that way.\n\n-- \nShawn.\n"},{"id":"100167","messageId":"20090112213213.GM10179@spearce.org","threadId":"17121","inReplyTo":"496BB442.90107@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-12T21:32:13Z","receivedAt":"2009-01-12T21:32:13Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ted Pavlic wrote:\n>> (note: for some reason Mercurial's bash completion script does not have\n>> this problem; they use $1 directly without bash complaining)\n>\n> It appears like they use\n>\n> \tcomplete -o bashdefault\n>\n> whereas Git's uses\n>\n> \tcomplete -o default\n>\n> I think that's the difference.\n\nIf that's all we need to do, that's a simple 1 line change... which\nI like.\n\n-- \nShawn.\n"},{"id":"100168","messageId":"496BB810.30503@tedpavlic.com","threadId":"17121","inReplyTo":"20090112212544.GA24941@chistera.yi.org","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-12T21:37:20Z","receivedAt":"2009-01-12T21:37:20Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"> Because (from a quick look) their completion script never expands a\n> variable which is not known to be set.\n\nThey use $1, $2, etc. In fact, they use $1, $2, and $3 in their _hg, \nwhich is their main completion function. Why would those be defined there?\n\nIn fact, it's $1, $2, $3, and $4 that are causing the problemw ith the \ngit completions.\n\n--Ted\n\n-- \nTed Pavlic \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":"100169","messageId":"200901121538.43400.bss@iguanasuicide.net","threadId":"17121","inReplyTo":"20090112213149.GL10179@spearce.org","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-12T21:38:39Z","receivedAt":"2009-01-12T21:38:39Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Monday 2009 January 12 15:31:49 Shawn O. Pearce wrote:\n>\"Boyd Stephen Smith Jr.\" wrote:\n>> Well, if the user want to run in \"set -u\" mode preventing it is bogus,\n>> IMO. We could use subshells and unset at the top of _git and _gitk\n>> functions, that would be only a +6/-4 patch. It would also not be\n>> something future contributors have to think (much) about.\n>\n>Running in subshells is a bad idea.\n\nYeah, not only for all the reasons you mention, but because it would require \nrefactoring to use -C instead of -F (so we a longer and uglier patch); our \nchanges to COMPREPLY in the subshell wouldn't be seen by bash.\n\nHaving tripped over my lack of experience twice in two messages in this \nthread, I'm going to bow out of the rest of it. My ascetic opinion still \nstands, but I'll take working code, warts and all, over broken code.\n-- \nBoyd Stephen Smith Jr. ,= ,-_-. =. \nbss@iguanasuicide.net ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' \nhttp://iguanasuicide.net/ \\_/ \n"},{"id":"100171","messageId":"20090112214052.GB24941@chistera.yi.org","threadId":"17121","inReplyTo":"496BA0E4.2040607@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-12T21:40:52Z","receivedAt":"2009-01-12T21:40:52Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* Ted Pavlic [Mon, 12 Jan 2009 14:58:28 -0500]:\n\nI don't know if this patch will go forward or not, but there are several\ninstances of spurious ${x-}, eg.:\n\n> __gitdir ()\n> {\n> -\tif [ -z \"$1\" ]; then\n> +\tif [ -z \"${1-}\" ]; then\n\nGiven the above...\n\n> -\telif [ -d \"$1/.git\" ]; then\n> -\t\techo \"$1/.git\"\n> +\telif [ -d \"${1-}/.git\" ]; then\n> +\t\techo \"${1-}/.git\"\n> \telse\n> -\t\techo \"$1\"\n> +\t\techo \"${1-}\"\n\n... this other hunk is redundant, because if [ -z \"${1-}\" ] fails, then\n$1 is surely set.\n\n> __git_ps1 ()\n> {\n> \tlocal g=\"$(git rev-parse --git-dir 2>/dev/null)\"\n> -\tif [ -n \"$g\" ]; then\n> +\tif [ -n \"${g-}\" ]; then\n\nSpurious, $g is always set here.\n\n> @@ -111,8 +111,8 @@ __git_ps1 ()\n> -\t\tif [ -n \"$1\" ]; then\n> +\t\tif [ -n \"${1-}\" ]; then\n\nThis one is okay...\n\n> -\t\t\tprintf \"$1\" \"${b##refs/heads/}$r\"\n> +\t\t\tprintf \"${1-}\" \"${b##refs/heads/}$r\"\n\nBut this one is unnecessary, if [ -n \"${1-}\" ] succeeds, then $1 is set.\n\nAnd so on.\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \nThe true teacher defends his pupils against his own personal influence.\n -- Amos Bronson Alcott\n"},{"id":"100172","messageId":"20090112214729.GC24941@chistera.yi.org","threadId":"17121","inReplyTo":"496BB810.30503@tedpavlic.com","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-12T21:47:29Z","receivedAt":"2009-01-12T21:47:29Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* Ted Pavlic [Mon, 12 Jan 2009 16:37:20 -0500]:\n\n>> Because (from a quick look) their completion script never expands a\n>> variable which is not known to be set.\n\n> They use $1, $2, etc. In fact, they use $1, $2, and $3 in their _hg, \n> which is their main completion function. Why would those be defined \n> there?\n\nFrom http://www.gnu.org/software/bash/manual/bashref.html#Programmable-Completion:\n\n When the function or command is invoked, the first argument is the name\n of the command whose arguments are being completed, the second argument\n is the word being completed, and the third argument is the word\n preceding the word being completed on the current command line.\n\n> In fact, it's $1, $2, $3, and $4 that are causing the problemw ith the \n> git completions.\n\nThey are causing problems in the functions that are called sometimes\nwith arguments, sometimes without, like __gitdir. If you know that\nyou'll always be calling a function with $1, you need not use ${1-};\nthat's what happens in the mercurial completion script AFAICS.\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \nThe surest way to corrupt a youth is to instruct him to hold in higher\nesteem those who think alike than those who think differently.\n -- F. Nietzsche\n"},{"id":"100174","messageId":"496BBB70.5090803@tedpavlic.com","threadId":"17121","inReplyTo":"20090112213213.GM10179@spearce.org","subject":"Re: [PATCH] Update bash completions to prevent unbound variable errors.","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-12T21:51:44Z","receivedAt":"2009-01-12T21:51:44Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":">> It appears like they use\n>>\n>> \tcomplete -o bashdefault\n>>\n>> whereas Git's uses\n>>\n>> \tcomplete -o default\n>>\n>> I think that's the difference.\n>\n> If that's all we need to do, that's a simple 1 line change... which\n> I like.\n\nThat was a red herring. The problem is more fundamental than that.\n\nThe script needs to be more careful about its use. I'll submit a better \npatch momentarily.\n\n--Ted\n\n\n-- \nTed Pavlic \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"}]}