[PATCH 1/3] completion: fix alias listings with newlines

Subsystems: the rest

DORMANTno replies

10 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 1/3] completion: fix alias listings with newlines

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

Since commit 518ef8f (completion: Replace config --list with
--get-regexp, 2009-09-11) an alias config value containing a newline
would break the completion of aliases. Instead of setting the IFS to the
newline, use git-config's null termination to separate aliases with the
null character.

An example .gitconfig causing the breakage.

[alias]
	whowhat = "log -1 --pretty='format:%an <%ae>\n%s'"
	wont-complete = ...

Signed-off-by: Stephen Boyd <redacted>
---

This is the best I can come up with. I'm not sure about using the
d option of read though.

 contrib/completion/git-completion.bash |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2c2a0d4..332be99 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -600,10 +600,10 @@ __git_porcelain_commandlist="$(__git_porcelain_commands 2>/dev/null)"
 
 __git_aliases ()
 {
-	local i IFS=$'\n'
-	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "alias\..*" 2>/dev/null); do
-		i="${i#alias.}"
-		echo "${i/ */}"
+	local i
+	git --git-dir="$(__gitdir)" config -z --get-regexp "alias\..*" 2>/dev/null |
+	while IFS= read -rd '' i; do
+		echo ${i#alias.} | cut -d' ' -f1
 	done
 }
 
-- 
1.6.5.rc2.17.gdbc1b

[PATCH 2/3] completion: update am, commit, and log

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

git am learned --scissors, git commit learned --dry-run and git log
learned --decorate=long|short recently.

Signed-off-by: Stephen Boyd <redacted>
---
 contrib/completion/git-completion.bash |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 332be99..2ab8c5e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -668,7 +668,7 @@ _git_am ()
 			--3way --committer-date-is-author-date --ignore-date
 			--ignore-whitespace --ignore-space-change
 			--interactive --keep --no-utf8 --signoff --utf8
-			--whitespace=
+			--whitespace= --scissors
 			"
 		return
 	esac
@@ -894,6 +894,7 @@ _git_commit ()
 		__gitcomp "
 			--all --author= --signoff --verify --no-verify
 			--edit --amend --include --only --interactive
+			--dry-run
 			"
 		return
 	esac
@@ -1179,6 +1180,10 @@ _git_log ()
 		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
 		return
 		;;
+	--decorate=*)
+		__gitcomp "long short" "" "${cur##--decorate=}"
+		return
+		;;
 	--*)
 		__gitcomp "
 			$__git_log_common_options
@@ -1191,7 +1196,7 @@ _git_log ()
 			--pretty= --format= --oneline
 			--cherry-pick
 			--graph
-			--decorate
+			--decorate --decorate=
 			--walk-reflogs
 			--parents --children
 			$merge
-- 
1.6.5.rc2.17.gdbc1b

[PATCH 3/3] completion: add dirstat and friends to diff options

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

Signed-off-by: Stephen Boyd <redacted>
---

I sent this a while back, no response. If no response again I'll drop.

 contrib/completion/git-completion.bash |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2ab8c5e..a5fe1df 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -927,6 +927,8 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
 			--inter-hunk-context=
 			--patience
 			--raw
+			--dirstat --dirstat= --dirstat-by-file
+			--dirstat-by-file= --cumulative
 "
 
 _git_diff ()
-- 
1.6.5.rc2.17.gdbc1b

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:29

Stephen Boyd schrieb:
 __git_aliases ()
 {
-	local i IFS=$'\n'
-	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "alias\..*" 2>/dev/null); do
-		i="${i#alias.}"
-		echo "${i/ */}"
+	local i
+	git --git-dir="$(__gitdir)" config -z --get-regexp "alias\..*" 2>/dev/null |
+	while IFS= read -rd '' i; do
+		echo ${i#alias.} | cut -d' ' -f1
 	done
 }
Is it necessary to change the body of the loop? Your version spawns two
processes on each iteration, while the original spawned no processes.

You can avoid the pipeline (i.e. yet another process) using a "here-string":

	local i aliases=$(git --git-dir="$(__gitdir)" config -z \
				--get-regexp "alias\..*" 2>/dev/null)
	while IFS= read -rd '' i; do
		i="${i#alias.}"
		echo "${i/ */}"	# could be: echo "${i%% *}"
  	done <<< "$aliases"

but I don't know how well bash handles variable values with embedded NULs.

-- Hannes

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

Johannes Sixt wrote:
Is it necessary to change the body of the loop? Your version spawns two
processes on each iteration, while the original spawned no processes.

You can avoid the pipeline (i.e. yet another process) using a "here-string":

	local i aliases=$(git --git-dir="$(__gitdir)" config -z \
				--get-regexp "alias\..*" 2>/dev/null)
	while IFS= read -rd '' i; do
		i="${i#alias.}"
		echo "${i/ */}"	# could be: echo "${i%% *}"
  	done <<< "$aliases"

but I don't know how well bash handles variable values with embedded NULs.
I can't get the above snippet to work, but maybe I'm doing something wrong.

I had a problem with the newline between the key and the value. bash
gives me the whole line in the loop, but when I try to trim it $i is
treated as two values. I couldn't figure out any other way to do it,
besides piping $i to cut or tr.

Maybe a better solution would be to add --keys-only or something to
git-config?

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:47:29

Stephen Boyd [off-list ref] wrote:
Johannes Sixt wrote:
quoted
Is it necessary to change the body of the loop? Your version spawns two
processes on each iteration, while the original spawned no processes.
Yes, we should try to avoid spawning a process here, it fires on
each completion attempt if I recall.
Maybe a better solution would be to add --keys-only or something to
git-config?
Or a format string with a language based escape like for-each-ref
supports?  Might make it easier to use git config in scripts if we
can write things like:

  git config --format='$data{%(key)}=%(value);' --perl

-- 
Shawn.

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

Shawn O. Pearce wrote:
Yes, we should try to avoid spawning a process here, it fires on
each completion attempt if I recall.
Ok. It looks like it fires on every completion with no command (i.e. git
<TAB><TAB>). I don't particularly like my solution anyways, and this has
existed for a long time (56fc25f maybe?) with no one noticing. A fix for
it is probably not too pressing.

I found another problem, [ARGS] and COMMAND show up in git <TAB><TAB>.
Looks like it's due to the wrapping of git's usage string.
Or a format string with a language based escape like for-each-ref
supports?  Might make it easier to use git config in scripts if we
can write things like:

  git config --format='$data{%(key)}=%(value);' --perl
This sounds reasonable, but also like more work ;-)

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:29

Stephen Boyd schrieb:
Shawn O. Pearce wrote:
quoted
Or a format string with a language based escape like for-each-ref
supports?  Might make it easier to use git config in scripts if we
can write things like:

  git config --format='$data{%(key)}=%(value);' --perl
This sounds reasonable, but also like more work ;-)
OTOH, you can simply revert the hunk from 518ef8f (completion: Replace
config --list with --get-regexp, 2009-09-11) that applies here, or even
only reinstate the case statement in the loop body without changing
anything else.

-- Hannes

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:29

Johannes Sixt wrote:
OTOH, you can simply revert the hunk from 518ef8f (completion: Replace
config --list with --get-regexp, 2009-09-11) that applies here, or even
only reinstate the case statement in the loop body without changing
anything else.
I'm not opposed to the idea, but then it wouldn't work with something like

[alias]
    whowhat = "log --format="%an\nalias.broken=foo'"

although that actually happening is probably rare, it can still happen.

Re: [PATCH 1/3] completion: fix alias listings with newlines

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:29

Stephen Boyd schrieb:
Johannes Sixt wrote:
quoted
OTOH, you can simply revert the hunk from 518ef8f (completion: Replace
config --list with --get-regexp, 2009-09-11) that applies here, or even
only reinstate the case statement in the loop body without changing
anything else.
I'm not opposed to the idea, but then it wouldn't work with something like

[alias]
    whowhat = "log --format="%an\nalias.broken=foo'"

although that actually happening is probably rare, it can still happen.
I'd ignore this case, but *you* write the patch, *you* get to draw the line ;)

-- Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help