git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] completion: escape metacharacters when completing paths
@ 2018-12-26 16:08 Chayoung You
  2018-12-28 23:25 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Chayoung You @ 2018-12-26 16:08 UTC (permalink / raw)
  To: git; +Cc: SZEDER Gábor, Junio C Hamano

The following is the description of -Q flag of zsh compadd [1]:

  This flag instructs the completion code not to quote any
  metacharacters in the words when inserting them into the command line.

Let's say there is a file named 'foo bar.txt' in repository, but it's
not yet added to the repository. Then the following command triggers a
completion:

  git add fo<Tab>
  git add 'fo<Tab>
  git add "fo<Tab>

The completion results in bash:

  git add foo\ bar.txt
  git add 'foo bar.txt'
  git add "foo bar.txt"

While them in zsh:

  git add foo bar.txt
  git add 'foo bar.txt'
  git add "foo bar.txt"

The main cause of this behavior is __gitcomp_file_direct(). The both
implementions of bash and zsh are called with an argument 'foo bar.txt',
but only bash adds a backslash before a space on command line.

[1]: http://zsh.sourceforge.net/Doc/Release/Completion-Widgets.html

Signed-off-by: Chayoung You <yousbe@gmail.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 contrib/completion/git-completion.zsh  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8ec95c3..816ee3280 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2993,7 +2993,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -f -- ${=1} && _ret=0
+		compadd -f -- ${=1} && _ret=0
 	}
 
 	__gitcomp_file ()
@@ -3002,7 +3002,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+		compadd -p "${2-}" -f -- ${=1} && _ret=0
 	}
 
 	_git ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 049d6b80f..886bf95d1 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -99,7 +99,7 @@ __gitcomp_file_direct ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -f -- ${=1} && _ret=0
+	compadd -f -- ${=1} && _ret=0
 }
 
 __gitcomp_file ()
@@ -108,7 +108,7 @@ __gitcomp_file ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+	compadd -p "${2-}" -f -- ${=1} && _ret=0
 }
 
 __git_zsh_bash_func ()
-- 
2.20.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] completion: escape metacharacters when completing paths
  2018-12-26 16:08 [PATCH] completion: escape metacharacters when completing paths Chayoung You
@ 2018-12-28 23:25 ` Junio C Hamano
  2018-12-30  5:31   ` [PATCH v2] zsh: complete unquoted paths with spaces correctly Chayoung You
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2018-12-28 23:25 UTC (permalink / raw)
  To: Chayoung You; +Cc: git, SZEDER Gábor

Chayoung You <yousbe@gmail.com> writes:

> Subject: Re: [PATCH] completion: escape metacharacters when completing paths

I am not a zsh user, but from the patch text I am getting the
impression that this patch is only about zsh.  If so, please help
readers of "git shortlog" by saying so in the title.

	Subject: zsh: complete unquoted paths with spaces correctly

or something like that, perhaps?

> Let's say there is a file named 'foo bar.txt' in repository, but it's
> not yet added to the repository. Then the following command triggers a
> completion:
>
>   git add fo<Tab>
>   git add 'fo<Tab>
>   git add "fo<Tab>
>
> The completion results in bash:
>
>   git add foo\ bar.txt
>   git add 'foo bar.txt'
>   git add "foo bar.txt"
>
> While them in zsh:
>
>   git add foo bar.txt
>   git add 'foo bar.txt'
>   git add "foo bar.txt"

You leave it unsaid what you think is wrong, and what you think
should be the right output, before saying "cause of this is...".

I do not think it is given that "git add fo<TAB>" should be
completed to "git add foo\ bar.txt".  It would also be OK if the
completion produced "git add 'foo bar.txt'", for example.  So what
your ideal output is is rather important, and by doing that you end
up with saying what problem you have with the current output is.

So, say something here, perhaps like...

	The first one, where the pathname is not enclosed in quotes,
	should escape the SP with a backslash, just like bash
	completion does.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] zsh: complete unquoted paths with spaces correctly
  2018-12-28 23:25 ` Junio C Hamano
@ 2018-12-30  5:31   ` Chayoung You
  2019-01-01 14:05     ` [PATCH v3 0/2] completion: handle " Chayoung You
  0 siblings, 1 reply; 6+ messages in thread
From: Chayoung You @ 2018-12-30  5:31 UTC (permalink / raw)
  To: git; +Cc: SZEDER Gábor, Junio C Hamano

The following is the description of -Q flag of zsh compadd [1]:

    This flag instructs the completion code not to quote any
    metacharacters in the words when inserting them into the command
    line.

Let's say there is a file named 'foo bar.txt' in repository, but it's
not yet added to the repository. Then the following command triggers a
completion:

    git add fo<Tab>
    git add 'fo<Tab>
    git add "fo<Tab>

The completion results in bash:

    git add foo\ bar.txt
    git add 'foo bar.txt'
    git add "foo bar.txt"

While them in zsh:

    git add foo bar.txt
    git add 'foo bar.txt'
    git add "foo bar.txt"

The first one, where the pathname is not enclosed in quotes, should
escape the space with a backslash, just like bash completion does.
Otherwise, this leads git to think there are two files; foo, and
bar.txt.

The main cause of this behavior is __gitcomp_file_direct(). The both
implementions of bash and zsh are called with an argument 'foo bar.txt',
but only bash adds a backslash before a space on command line.

[1]: http://zsh.sourceforge.net/Doc/Release/Completion-Widgets.html

Signed-off-by: Chayoung You <yousbe@gmail.com>
Reviewed-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 contrib/completion/git-completion.zsh  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8ec95c3..816ee3280 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2993,7 +2993,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -f -- ${=1} && _ret=0
+		compadd -f -- ${=1} && _ret=0
 	}
 
 	__gitcomp_file ()
@@ -3002,7 +3002,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+		compadd -p "${2-}" -f -- ${=1} && _ret=0
 	}
 
 	_git ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 049d6b80f..886bf95d1 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -99,7 +99,7 @@ __gitcomp_file_direct ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -f -- ${=1} && _ret=0
+	compadd -f -- ${=1} && _ret=0
 }
 
 __gitcomp_file ()
@@ -108,7 +108,7 @@ __gitcomp_file ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+	compadd -p "${2-}" -f -- ${=1} && _ret=0
 }
 
 __git_zsh_bash_func ()
-- 
2.20.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 0/2] completion: handle paths with spaces correctly
  2018-12-30  5:31   ` [PATCH v2] zsh: complete unquoted paths with spaces correctly Chayoung You
@ 2019-01-01 14:05     ` Chayoung You
  2019-01-01 14:05       ` [PATCH v3 1/2] zsh: complete unquoted " Chayoung You
  2019-01-01 14:05       ` [PATCH v3 2/2] completion: treat results of git ls-tree as file paths Chayoung You
  0 siblings, 2 replies; 6+ messages in thread
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, SZEDER Gábor

I found more issues with completion for git show ref:path, so here I add one
more patch.

Chayoung You (2):
  zsh: complete unquoted paths with spaces correctly
  completion: treat results of git ls-tree as file paths

 contrib/completion/git-completion.bash | 35 +++++++++++---------------
 contrib/completion/git-completion.zsh  |  4 +--
 t/t9902-completion.sh                  | 10 ++++----
 3 files changed, 21 insertions(+), 28 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] zsh: complete unquoted paths with spaces correctly
  2019-01-01 14:05     ` [PATCH v3 0/2] completion: handle " Chayoung You
@ 2019-01-01 14:05       ` Chayoung You
  2019-01-01 14:05       ` [PATCH v3 2/2] completion: treat results of git ls-tree as file paths Chayoung You
  1 sibling, 0 replies; 6+ messages in thread
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, SZEDER Gábor

The following is the description of -Q flag of zsh compadd [1]:

    This flag instructs the completion code not to quote any
    metacharacters in the words when inserting them into the command
    line.

Let's say there is a file named 'foo bar.txt' in repository, but it's
not yet added to the repository. Then the following command triggers a
completion:

    git add fo<Tab>
    git add 'fo<Tab>
    git add "fo<Tab>

The completion results in bash:

    git add foo\ bar.txt
    git add 'foo bar.txt'
    git add "foo bar.txt"

While them in zsh:

    git add foo bar.txt
    git add 'foo bar.txt'
    git add "foo bar.txt"

The first one, where the pathname is not enclosed in quotes, should
escape the space with a backslash, just like bash completion does.
Otherwise, this leads git to think there are two files; foo, and
bar.txt.

The main cause of this behavior is __gitcomp_file_direct(). The both
implementions of bash and zsh are called with an argument 'foo bar.txt',
but only bash adds a backslash before a space on command line.

[1]: http://zsh.sourceforge.net/Doc/Release/Completion-Widgets.html

Signed-off-by: Chayoung You <yousbe@gmail.com>
Reviewed-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 contrib/completion/git-completion.zsh  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8ec95c3..816ee3280 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2993,7 +2993,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -f -- ${=1} && _ret=0
+		compadd -f -- ${=1} && _ret=0
 	}
 
 	__gitcomp_file ()
@@ -3002,7 +3002,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+		compadd -p "${2-}" -f -- ${=1} && _ret=0
 	}
 
 	_git ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 049d6b80f..886bf95d1 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -99,7 +99,7 @@ __gitcomp_file_direct ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -f -- ${=1} && _ret=0
+	compadd -f -- ${=1} && _ret=0
 }
 
 __gitcomp_file ()
@@ -108,7 +108,7 @@ __gitcomp_file ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+	compadd -p "${2-}" -f -- ${=1} && _ret=0
 }
 
 __git_zsh_bash_func ()
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] completion: treat results of git ls-tree as file paths
  2019-01-01 14:05     ` [PATCH v3 0/2] completion: handle " Chayoung You
  2019-01-01 14:05       ` [PATCH v3 1/2] zsh: complete unquoted " Chayoung You
@ 2019-01-01 14:05       ` Chayoung You
  1 sibling, 0 replies; 6+ messages in thread
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, SZEDER Gábor

Let's say there are files named 'foo bar.txt', and 'abc def/test.txt' in
repository. When following commands trigger a completion:

    git show HEAD:fo<Tab>
    git show HEAD:ab<Tab>

The completion results in bash/zsh:

    git show HEAD:foo bar.txt
    git show HEAD:abc def/

Where the both of them have an unescaped space in paths, so they'll be
misread by git. All entries of git ls-tree either a filename or a
directory, so __gitcomp_file() is proper rather than __gitcomp_nl().

Note the commit f12785a3, which handles quoted paths properly. Like this
case, we should dequote $cur_ for ?*:* case. For example, let's say
there is untracked directory 'abc deg', then trigger a completion:

    git show HEAD:abc\ de<Tab>
    git show HEAD:'abc de<Tab>
    git show HEAD:"abc de<Tab>

should uniquely complete 'abc def', but bash completes 'abc def' and
'abc deg' instead. In zsh, triggering a completion:

    git show HEAD:abc\ def/<Tab>

should complete 'test.txt', but nothing comes. The both problems will be
resolved by dequoting paths.

__git_complete_revlist_file() passes arguments to __gitcomp_nl() where
the first one is a list something like:

    abc def/Z
    foo bar.txt Z

where Z is the mark of the EOL.

- The trailing space of blob in __git ls-tree | sed.
  It makes the completion results become:

      git show HEAD:foo\ bar.txt\ <CURSOR>

  So git will try to find a file named 'foo bar.txt ' instead.

- The trailing slash of tree in __git ls-tree | sed.
  It makes the completion results on zsh become:

      git show HEAD:abc\ def/ <CURSOR>

  So that the last space on command like should be removed on zsh to
  complete filenames under 'abc def/'.

Signed-off-by: Chayoung You <yousbe@gmail.com>
---
 contrib/completion/git-completion.bash | 31 ++++++++++----------------
 t/t9902-completion.sh                  | 10 ++++-----
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 816ee3280..26ea310fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -855,7 +855,7 @@ __git_compute_merge_strategies ()
 
 __git_complete_revlist_file ()
 {
-	local pfx ls ref cur_="$cur"
+	local dequoted_word pfx ls ref cur_="$cur"
 	case "$cur_" in
 	*..?*:*)
 		return
@@ -863,14 +863,18 @@ __git_complete_revlist_file ()
 	?*:*)
 		ref="${cur_%%:*}"
 		cur_="${cur_#*:}"
-		case "$cur_" in
+
+		__git_dequote "$cur_"
+
+		case "$dequoted_word" in
 		?*/*)
-			pfx="${cur_%/*}"
-			cur_="${cur_##*/}"
+			pfx="${dequoted_word%/*}"
+			cur_="${dequoted_word##*/}"
 			ls="$ref:$pfx"
 			pfx="$pfx/"
 			;;
 		*)
+			cur_="$dequoted_word"
 			ls="$ref"
 			;;
 		esac
@@ -880,21 +884,10 @@ __git_complete_revlist_file ()
 		*)   pfx="$ref:$pfx" ;;
 		esac
 
-		__gitcomp_nl "$(__git ls-tree "$ls" \
-				| sed '/^100... blob /{
-				           s,^.*	,,
-				           s,$, ,
-				       }
-				       /^120000 blob /{
-				           s,^.*	,,
-				           s,$, ,
-				       }
-				       /^040000 tree /{
-				           s,^.*	,,
-				           s,$,/,
-				       }
-				       s/^.*	//')" \
-			"$pfx" "$cur_" ""
+		__gitcomp_file "$(__git ls-tree "$ls" \
+				| sed 's/^.*	//
+				       s/$//')" \
+			"$pfx" "$cur_"
 		;;
 	*...*)
 		pfx="${cur_%...*}..."
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 137fdc9bd..94157e587 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1515,8 +1515,8 @@ test_expect_success 'show completes all refs' '
 
 test_expect_success '<ref>: completes paths' '
 	test_completion "git show mytag:f" <<-\EOF
-	file1 Z
-	file2 Z
+	file1Z
+	file2Z
 	EOF
 '
 
@@ -1525,7 +1525,7 @@ test_expect_success 'complete tree filename with spaces' '
 	git add "name with spaces" &&
 	git commit -m spaces &&
 	test_completion "git show HEAD:nam" <<-\EOF
-	name with spaces Z
+	name with spacesZ
 	EOF
 '
 
@@ -1534,8 +1534,8 @@ test_expect_success 'complete tree filename with metacharacters' '
 	git add "name with \${meta}" &&
 	git commit -m meta &&
 	test_completion "git show HEAD:nam" <<-\EOF
-	name with ${meta} Z
-	name with spaces Z
+	name with ${meta}Z
+	name with spacesZ
 	EOF
 '
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-01 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-26 16:08 [PATCH] completion: escape metacharacters when completing paths Chayoung You
2018-12-28 23:25 ` Junio C Hamano
2018-12-30  5:31   ` [PATCH v2] zsh: complete unquoted paths with spaces correctly Chayoung You
2019-01-01 14:05     ` [PATCH v3 0/2] completion: handle " Chayoung You
2019-01-01 14:05       ` [PATCH v3 1/2] zsh: complete unquoted " Chayoung You
2019-01-01 14:05       ` [PATCH v3 2/2] completion: treat results of git ls-tree as file paths Chayoung You

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).