{"thread":{"id":"17175","subject":"[PATCH 3/3] bash-completion: Added comments to remind about required arguments","startedAt":"2009-01-14T20:02:20Z","lastAt":"2009-01-15T15:47:41Z","messageCount":6,"participants":["ted@tedpavlic.com","Shawn O. Pearce","Ted Pavlic"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"100472","messageId":"1231963342-24461-1-git-send-email-ted@tedpavlic.com","threadId":"17175","inReplyTo":null,"subject":"[PATCH 1/3] bash-completion: Support running when set -u is enabled","fromName":"","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-14T20:02:20Z","receivedAt":"2009-01-14T20:02:20Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"From: Ted Pavlic \n\n Under \"set -u\" semantics, it is an error to access undefined\n variables. Some user environments may enable this setting in the\n interactive shell.\n\n In any context where the completion functions access an undefined\n variable, accessing a default empty string (aka \"${1-}\" instead of\n \"$1\") is a reasonable way to code the function, as it silences\n the undefined variable error while still supplying an empty string.\n\n In this patch, functions that should always take an argument still use\n $1. Functions that have optional arguments use ${1-}.\n\nSigned-off-by: Ted Pavlic \nAcked-by: Shawn O. Pearce \n---\n contrib/completion/git-completion.bash | 18 +++++++++---------\n 1 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7b074d7..5d1515c 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -52,7 +52,7 @@ esac\n \n __gitdir ()\n {\n-\tif [ -z \"$1\" ]; 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@@ -111,7 +111,7 @@ __git_ps1 ()\n \t\t\tfi\n \t\tfi\n \n-\t\tif [ -n \"$1\" ]; then\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@@ -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-- \n1.6.1.87.g15624\n"},{"id":"100471","messageId":"1231963342-24461-2-git-send-email-ted@tedpavlic.com","threadId":"17175","inReplyTo":"1231963342-24461-1-git-send-email-ted@tedpavlic.com","subject":"[PATCH 2/3] bash-completion: Try bash completions before simple filetype","fromName":"","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-14T20:02:21Z","receivedAt":"2009-01-14T20:02:21Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"From: Ted Pavlic \n\n When a git completion is not found, a bash shell should try bash-type\n completions first before going to standard filetype completions. This\n patch /adds/ \"-o bashdefault\" to the completion line. If that option\n is not available, it uses the old method.\n\n This behavior was inspired by Mercurial's bash completion script.\n\nSigned-off-by: Ted Pavlic \nAcked-by: Shawn O. Pearce \n---\n contrib/completion/git-completion.bash | 9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5d1515c..201f9a6 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1766,13 +1766,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":"100470","messageId":"1231963342-24461-3-git-send-email-ted@tedpavlic.com","threadId":"17175","inReplyTo":"1231963342-24461-2-git-send-email-ted@tedpavlic.com","subject":"[PATCH 3/3] bash-completion: Added comments to remind about required arguments","fromName":"","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-14T20:02:22Z","receivedAt":"2009-01-14T20:02:22Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"From: Ted Pavlic \n\n Adds a few simple comments above commands that take arguments. These\n comments are meant to remind developers of potential problems that can\n occur when the script is sourced on systems with \"set -u.\" Any\n function which \"requires\" arguments really ought to be called with\n explicit arguments given.\n\n Also adds a #!bash to the top of bash completions so that editing\n software can always identify that the file is of sh type.\n\nSigned-off-by: Ted Pavlic \nAcked-by: Shawn O. Pearce \n---\n contrib/completion/git-completion.bash | 15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 201f9a6..f8b845a 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,6 +51,8 @@ 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@@ -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@@ -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@@ -150,6 +158,7 @@ __gitcomp ()\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@@ -168,6 +177,7 @@ __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@@ -186,6 +196,7 @@ __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@@ -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-- \n1.6.1.87.g15624\n"},{"id":"100614","messageId":"20090115153402.GE10179@spearce.org","threadId":"17175","inReplyTo":"1231963342-24461-1-git-send-email-ted@tedpavlic.com","subject":"Re: [PATCH 1/3] bash-completion: Support running when set -u is enabled","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-15T15:34:02Z","receivedAt":"2009-01-15T15:34:02Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"ted@tedpavlic.com wrote:\n> From: Ted Pavlic \n> \n> Under \"set -u\" semantics, it is an error to access undefined\n> variables. Some user environments may enable this setting in the\n> interactive shell.\n> \n> In any context where the completion functions access an undefined\n> variable, accessing a default empty string (aka \"${1-}\" instead of\n> \"$1\") is a reasonable way to code the function, as it silences\n> the undefined variable error while still supplying an empty string.\n> \n> In this patch, functions that should always take an argument still use\n> $1. Functions that have optional arguments use ${1-}.\n> \n> Signed-off-by: Ted Pavlic \n> Acked-by: Shawn O. Pearce \n\nYup. This patch isn't as bad as everyone made it out to be.\nI think we should go ahead and include it.\n\nBut usually we don't indent the commit message like you did\nabove; instead the message should be aligned on the left margin at\ncolumn 0. git log will indent the message during display, hence\nany identation you add in the patch email just makes the message\nthat much harder to read from git log.\n\n> ---\n> contrib/completion/git-completion.bash | 18 +++++++++---------\n> 1 files changed, 9 insertions(+), 9 deletions(-)\n\n-- \nShawn.\n"},{"id":"100615","messageId":"20090115153543.GF10179@spearce.org","threadId":"17175","inReplyTo":"1231963342-24461-3-git-send-email-ted@tedpavlic.com","subject":"Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-15T15:35:43Z","receivedAt":"2009-01-15T15:35:43Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"ted@tedpavlic.com wrote:\n> From: Ted Pavlic \n> \n> Adds a few simple comments above commands that take arguments. These\n> comments are meant to remind developers of potential problems that can\n> occur when the script is sourced on systems with \"set -u.\" Any\n> function which \"requires\" arguments really ought to be called with\n> explicit arguments given.\n> \n> Also adds a #!bash to the top of bash completions so that editing\n> software can always identify that the file is of sh type.\n> \n> Signed-off-by: Ted Pavlic \n> Acked-by: Shawn O. Pearce \n\nYup, this and also 2/3 look fine. But again, the message, it\nshouldn't be indented.\n\n> ---\n> contrib/completion/git-completion.bash | 15 +++++++++++++++\n> 1 files changed, 15 insertions(+), 0 deletions(-)\n\n-- \nShawn.\n"},{"id":"100618","messageId":"496F5A9D.8040209@tedpavlic.com","threadId":"17175","inReplyTo":"20090115153543.GF10179@spearce.org","subject":"Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments","fromName":"Ted Pavlic","fromEmail":"ted@tedpavlic.com","sentAt":"2009-01-15T15:47:41Z","receivedAt":"2009-01-15T15:47:41Z","isPatch":true,"sender":{"key":"ted@tedpavlic.com","avatar":"https://gravatar.com/avatar/d085392370ff4c028cf17a0e81e0647744c9682fbcb36b499f31d08ef80ef569?d=mp&s=160"},"body":"> Yup, this and also 2/3 look fine. But again, the message, it\n> shouldn't be indented.\n\nI will re-submit with no indentation.\n\nSorry for all the fuss. I've learned a great deal.\n\nThanks --\nTed\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"}]}