* [PATCH] rev-parse --show-prefix: add in trailing newline
@ 2012-04-09 13:27 Ross Lagerwall
2012-04-09 19:07 ` Junio C Hamano
2012-04-10 17:36 ` Jonathan Nieder
0 siblings, 2 replies; 9+ messages in thread
From: Ross Lagerwall @ 2012-04-09 13:27 UTC (permalink / raw)
To: git; +Cc: Ross Lagerwall
Print out a trailing newline when --show-prefix is run with cwd
at the top level of the tree which results in an empty prefix.
Behavior is now like --show-cdup.
Fixes an expected failure in t1501.
Signed-off-by: Ross Lagerwall <rosslagerwall@gmail.com>
---
builtin/rev-parse.c | 2 ++
t/t1501-worktree.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 98d1cbe..733f626 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
if (!strcmp(arg, "--show-prefix")) {
if (prefix)
puts(prefix);
+ else
+ putchar('\n');
continue;
}
if (!strcmp(arg, "--show-cdup")) {
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index e661147..8f36aa9 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -68,7 +68,7 @@ test_expect_success 'inside work tree' '
)
'
-test_expect_failure 'empty prefix is actually written out' '
+test_expect_success 'empty prefix is actually written out' '
echo >expected &&
(
cd work &&
--
1.7.10
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-09 13:27 [PATCH] rev-parse --show-prefix: add in trailing newline Ross Lagerwall
@ 2012-04-09 19:07 ` Junio C Hamano
2012-04-10 5:30 ` Ross Lagerwall
` (2 more replies)
2012-04-10 17:36 ` Jonathan Nieder
1 sibling, 3 replies; 9+ messages in thread
From: Junio C Hamano @ 2012-04-09 19:07 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: git, Jonathan Nieder
Ross Lagerwall <rosslagerwall@gmail.com> writes:
> diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
> index e661147..8f36aa9 100755
> --- a/t/t1501-worktree.sh
> +++ b/t/t1501-worktree.sh
> @@ -68,7 +68,7 @@ test_expect_success 'inside work tree' '
> )
> '
>
> -test_expect_failure 'empty prefix is actually written out' '
> +test_expect_success 'empty prefix is actually written out' '
> echo >expected &&
> (
> cd work &&
fc68875 (t1501 (rev-parse): clarify, 2010-07-24) says it is a "possible
bug", but I do not think the expectation of the new test added by that
commit is entirely correct.
Because the assignment will strip the trailing LF away adding an extra
newline at the end would not matter in practice in many scripts, e.g.
a=$(git rev-parse --show-prefix)
if test -z "$a"
then
echo we are at toplevel
fi
or
pfx=$(git rev-parse --show-prefix)
cd_to_toplevel
for arg
do
case "$arg" in
/*)
user refers to "$arg" that is a full path ;;
*)
user refers to "$pfx$arg" ;;
esac
done
But it will break existing scripts if they expect the following to work:
file="$(git rev-parse --show-prefix)/$1"
cd_to_toplevel
test -f "$file"
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-09 19:07 ` Junio C Hamano
@ 2012-04-10 5:30 ` Ross Lagerwall
2012-04-10 14:04 ` Jonathan Nieder
2012-04-10 14:41 ` Andreas Schwab
2 siblings, 0 replies; 9+ messages in thread
From: Ross Lagerwall @ 2012-04-10 5:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
On 2012/04/09 21:07, Junio C Hamano wrote:
> fc68875 (t1501 (rev-parse): clarify, 2010-07-24) says it is a "possible
> bug", but I do not think the expectation of the new test added by that
> commit is entirely correct.
>
> Because the assignment will strip the trailing LF away adding an extra
> newline at the end would not matter in practice in many scripts, e.g.
>
True, and the current behavior does exactly what the man page says:
"When the command is invoked from a subdirectory, show the path of the
current directory relative to the top-level directory."
So then perhaps the test should be removed?
Also, --show-cdup has a similar description for the man page but does
output a newline when invoked from the root dir. Should this behavior be
made consistent?
--
Ross Lagerwall
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-09 19:07 ` Junio C Hamano
2012-04-10 5:30 ` Ross Lagerwall
@ 2012-04-10 14:04 ` Jonathan Nieder
2012-04-10 14:41 ` Andreas Schwab
2 siblings, 0 replies; 9+ messages in thread
From: Jonathan Nieder @ 2012-04-10 14:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ross Lagerwall, git
Junio C Hamano wrote:
> But it will break existing scripts if they expect the following to work:
>
> file="$(git rev-parse --show-prefix)/$1"
> cd_to_toplevel
> test -f "$file"
Huh? I tried this:
$ foo="$(printf '\n')/bar"
$ printf '%s\n' "$foo"
/bar
If that didn't work, it would mean that "git rev-parse --show-prefix"
would never work when the prefix is nonempty, because it is followed
by a newline.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-09 19:07 ` Junio C Hamano
2012-04-10 5:30 ` Ross Lagerwall
2012-04-10 14:04 ` Jonathan Nieder
@ 2012-04-10 14:41 ` Andreas Schwab
2012-04-10 16:14 ` Junio C Hamano
2 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2012-04-10 14:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ross Lagerwall, git, Jonathan Nieder
Junio C Hamano <gitster@pobox.com> writes:
> Because the assignment will strip the trailing LF away
No, it doesn't. Command substitution does.
> But it will break existing scripts if they expect the following to work:
>
> file="$(git rev-parse --show-prefix)/$1"
> cd_to_toplevel
> test -f "$file"
This is already broken because of the extra slash.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-10 14:41 ` Andreas Schwab
@ 2012-04-10 16:14 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2012-04-10 16:14 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Ross Lagerwall, git, Jonathan Nieder
Andreas Schwab <schwab@linux-m68k.org> writes:
> This is already broken because of the extra slash.
Yeah, you are right.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-09 13:27 [PATCH] rev-parse --show-prefix: add in trailing newline Ross Lagerwall
2012-04-09 19:07 ` Junio C Hamano
@ 2012-04-10 17:36 ` Jonathan Nieder
2012-04-10 17:56 ` Junio C Hamano
2012-04-10 22:51 ` Nguyen Thai Ngoc Duy
1 sibling, 2 replies; 9+ messages in thread
From: Jonathan Nieder @ 2012-04-10 17:36 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: git, Nguyễn Thái Ngọc Duy
(cc-ing Duy)
Hi,
Ross Lagerwall wrote:
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> if (!strcmp(arg, "--show-prefix")) {
> if (prefix)
> puts(prefix);
> + else
> + putchar('\n');
> continue;
This makes the output more consistent but it is a little puzzling how
it does that. Why is prefix NULL instead of "" when we are at the
toplevel of the worktree?
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-10 17:36 ` Jonathan Nieder
@ 2012-04-10 17:56 ` Junio C Hamano
2012-04-10 22:51 ` Nguyen Thai Ngoc Duy
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2012-04-10 17:56 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ross Lagerwall, git, Nguyễn Thái Ngọc Duy
Jonathan Nieder <jrnieder@gmail.com> writes:
> (cc-ing Duy)
> Hi,
>
> Ross Lagerwall wrote:
>
>> --- a/builtin/rev-parse.c
>> +++ b/builtin/rev-parse.c
>> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>> if (!strcmp(arg, "--show-prefix")) {
>> if (prefix)
>> puts(prefix);
>> + else
>> + putchar('\n');
>> continue;
>
> This makes the output more consistent but it is a little puzzling how
> it does that. Why is prefix NULL instead of "" when we are at the
> toplevel of the worktree?
Interesting point ;-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rev-parse --show-prefix: add in trailing newline
2012-04-10 17:36 ` Jonathan Nieder
2012-04-10 17:56 ` Junio C Hamano
@ 2012-04-10 22:51 ` Nguyen Thai Ngoc Duy
1 sibling, 0 replies; 9+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2012-04-10 22:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ross Lagerwall, git
2012/4/11 Jonathan Nieder <jrnieder@gmail.com>:
> (cc-ing Duy)
> Hi,
>
> Ross Lagerwall wrote:
>
>> --- a/builtin/rev-parse.c
>> +++ b/builtin/rev-parse.c
>> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>> if (!strcmp(arg, "--show-prefix")) {
>> if (prefix)
>> puts(prefix);
>> + else
>> + putchar('\n');
>> continue;
>
> This makes the output more consistent but it is a little puzzling how
> it does that. Why is prefix NULL instead of "" when we are at the
> toplevel of the worktree?
It dates back to the first version of setup_git_directory() in d288a70
([PATCH] Make "git diff" work inside relative subdirectories -
2005-08-16). If we get "", setup_git_.. converts it to NULL. Although
I think Linus made a mistake in the first version of "rev-parse
--git-dir" doing "puts(prefix);" unconditionally in the same commit.
Junio fixed --git-dir in 4866ccf (Rationalize output selection in
rev-parse. - 2005-08-24) with the "if (prefix)" clause and should have
added the "else putchar('\n');" there too, I think.
--
Duy
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-04-10 22:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-09 13:27 [PATCH] rev-parse --show-prefix: add in trailing newline Ross Lagerwall
2012-04-09 19:07 ` Junio C Hamano
2012-04-10 5:30 ` Ross Lagerwall
2012-04-10 14:04 ` Jonathan Nieder
2012-04-10 14:41 ` Andreas Schwab
2012-04-10 16:14 ` Junio C Hamano
2012-04-10 17:36 ` Jonathan Nieder
2012-04-10 17:56 ` Junio C Hamano
2012-04-10 22:51 ` Nguyen Thai Ngoc Duy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.