[PATCH] status: display the SHA1 of the commit being currently processed

[PATCH] status: display the SHA1 of the commit being currently processed

[Mathieu Lienard--Mayor]

When in the middle of a rebase, it can be annoying to go in .git
in order to find the SHA1 of the commit where the rebase stopped.

git-status now includes this information in its default output.
With this new information, the message is now shorter, to avoid
too long lines.

The new message looks like:
$ git status
 HEAD detached from 33e516f
 Editing c346c87 while rebasing branch 'rebase_i_edit' on 'f90e540'.

Signed-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>
Signed-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---

 -changes in the tests to match the new status output
 -read file rebase-merge/stopped_sha to include the SHA in status output

 t/t7512-status-help.sh |   36 ++++++++++++++++++++++++------------
 wt-status.c            |   25 +++++++++++++++++++++----
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bf08d4e..dc93d77 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -189,10 +189,11 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
 	TGT=$(git rev-parse --short two_rebase_i) &&
+	SHA=$(git rev-parse --short three_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $TGT
-	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -217,9 +218,10 @@ test_expect_success 'status when splitting a commit' '
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
 	TGT=$(git rev-parse --short HEAD) &&
+	SHA=$(git rev-parse --short three_split) &&
 	cat >expected <<-EOF &&
 	# HEAD detached at $TGT
-	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
+	# Splitting $SHA while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
 	# Changes not staged for commit:
@@ -247,11 +249,12 @@ test_expect_success 'status after editing the last commit with --amend during a
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	TGT=$(git rev-parse --short three_amend) &&
+	SHA=$(git rev-parse --short four_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $TGT
-	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -277,11 +280,12 @@ test_expect_success 'status: (continue first edit) second edit' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -298,12 +302,13 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Splitting $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
 	# Changes not staged for commit:
@@ -325,12 +330,13 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -347,12 +353,13 @@ test_expect_success 'status: (amend first edit) second edit' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -369,13 +376,14 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "b" &&
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Splitting $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
 	# Changes not staged for commit:
@@ -397,13 +405,14 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "c" &&
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -420,6 +429,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
 	git add main.txt &&
@@ -427,7 +437,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git rebase --continue &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
@@ -444,6 +454,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
 	git add main.txt &&
@@ -452,7 +463,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Splitting $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
 	# Changes not staged for commit:
@@ -474,6 +485,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	SHA=$(git rev-parse --short three_edits) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
 	git add main.txt &&
@@ -482,7 +494,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
 	# HEAD detached from $ONTO
-	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
+	# Editing $SHA while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
 	#
diff --git a/wt-status.c b/wt-status.c
index bf84a86..5f5cddf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -885,8 +885,19 @@ static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
+	char *stopped_sha = read_line_from_git_path("rebase-merge/stopped-sha");
+	int must_free_stopped_sha = 1;
 	struct stat st;
 
+	/*
+	 * If the file stopped-sha does not exist
+	 * we go back to the old output saying "a commit"
+	 * instead of providing the commit's SHA1.
+	 */
+	if (!stopped_sha) {
+		stopped_sha = "a commit";
+		must_free_stopped_sha = 0;
+	}
 	if (has_unmerged(s)) {
 		if (state->branch)
 			status_printf_ln(s, color,
@@ -919,24 +930,28 @@ static void show_rebase_in_progress(struct wt_status *s,
 	} else if (split_commit_in_progress(s)) {
 		if (state->branch)
 			status_printf_ln(s, color,
-					 _("You are currently splitting a commit while rebasing branch '%s' on '%s'."),
+					 _("Splitting %s while rebasing branch '%s' on '%s'."),
+					 stopped_sha,
 					 state->branch,
 					 state->onto);
 		else
 			status_printf_ln(s, color,
-					 _("You are currently splitting a commit during a rebase."));
+					 _("Splitting %s during a rebase."),
+					 stopped_sha);
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (Once your working directory is clean, run \"git rebase --continue\")"));
 	} else {
 		if (state->branch)
 			status_printf_ln(s, color,
-					 _("You are currently editing a commit while rebasing branch '%s' on '%s'."),
+					 _("Editing %s while rebasing branch '%s' on '%s'."),
+					 stopped_sha,
 					 state->branch,
 					 state->onto);
 		else
 			status_printf_ln(s, color,
-					 _("You are currently editing a commit during a rebase."));
+					 _("Editing %s during a rebase."),
+					 stopped_sha);
 		if (advice_status_hints && !s->amend) {
 			status_printf_ln(s, color,
 				_("  (use \"git commit --amend\" to amend the current commit)"));
@@ -945,6 +960,8 @@ static void show_rebase_in_progress(struct wt_status *s,
 		}
 	}
 	wt_status_print_trailer(s);
+	if (must_free_stopped_sha)
+		free(stopped_sha);
 }
 
 static void show_cherry_pick_in_progress(struct wt_status *s,
-- 
1.7.8

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Thomas Adam]

Hi,

[I rarely do reviews on this list, so feel free to ignore this.]

On 17 June 2013 13:10, Mathieu Lienard--Mayor
<Mathieu.Lienard--Mayor@ensimag.imag.fr> wrote:
> diff --git a/wt-status.c b/wt-status.c
> index bf84a86..5f5cddf 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -885,8 +885,19 @@ static void show_rebase_in_progress(struct wt_status *s,
>                                 struct wt_status_state *state,
>                                 const char *color)
>  {
> +       char *stopped_sha = read_line_from_git_path("rebase-merge/stopped-sha");
> +       int must_free_stopped_sha = 1;
>         struct stat st;
>
> +       /*
> +        * If the file stopped-sha does not exist
> +        * we go back to the old output saying "a commit"
> +        * instead of providing the commit's SHA1.
> +        */
> +       if (!stopped_sha) {
> +               stopped_sha = "a commit";
> +               must_free_stopped_sha = 0;
> +       }

Rather than having to assign a toggle of deciding when to free
stopped_sha, how much overhead would be introduced by just doing:

    if (!stopped_sha)
        stopped_sha = strdup("a commit");

And then at the end just do:

free (stopped_sha);

Just a thought.

-- Thomas Adam

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Peter Krefting]

Mathieu Lienard--Mayor:

> +	/*
> +	 * If the file stopped-sha does not exist
> +	 * we go back to the old output saying "a commit"
> +	 * instead of providing the commit's SHA1.
> +	 */
> +	if (!stopped_sha) {
> +		stopped_sha = "a commit";
> +		must_free_stopped_sha = 0;
> +	}

This is missing gettext markers, and besides that, it very difficult 
to handle for translators. Please consider changing the code to use 
different strings based on what you want to insert, i.e.:

> 		if (state->branch)
> 			status_printf_ln(s, color,
> -					 _("You are currently splitting a commit while rebasing branch '%s' on '%s'."),
> +					 ("Splitting %s while rebasing branch '%s' on '%s'."),

    stopped_sha ? _("Splitting %s while rebasing branch '%s' on '%s'.")
                : _("Splitting a commit while rebasing branch '%2$s' on '%3$s'.")

or something similar.

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Mathieu Liénard--Mayor]

Le 2013-06-17 15:10, Peter Krefting a écrit :
> Mathieu Lienard--Mayor:
>
>> +	/*
>> +	 * If the file stopped-sha does not exist
>> +	 * we go back to the old output saying "a commit"
>> +	 * instead of providing the commit's SHA1.
>> +	 */
>> +	if (!stopped_sha) {
>> +		stopped_sha = "a commit";
>> +		must_free_stopped_sha = 0;
>> +	}
>
> This is missing gettext markers, and besides that, it very difficult
> to handle for translators. Please consider changing the code to use
> different strings based on what you want to insert, i.e.:
>
>> 		if (state->branch)
>> 			status_printf_ln(s, color,
>> -					 _("You are currently splitting a commit while rebasing branch 
>> '%s' on '%s'."),
>> +					 _("Splitting %s while rebasing branch '%s' on '%s'."),
>
>    stopped_sha ? _("Splitting %s while rebasing branch '%s' on 
> '%s'.")
>                : _("Splitting a commit while rebasing branch '%2$s' 
> on '%3$s'.")
>
> or something similar.
Actually, at first I dealt with it this way:

status_printf_ln(s, color,
                  _("Splitting %s while rebasing branch '%s' on '%s'."),
		 stopped_sha ? stopped_sha : _("a commit"),
		 ....);

Would this be more suitable for translators ?

-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Peter Krefting]

Mathieu Liénard--Mayor:

> Actually, at first I dealt with it this way:
>
> status_printf_ln(s, color,
>                 _("Splitting %s while rebasing branch '%s' on '%s'."),
> 		 stopped_sha ? stopped_sha : _("a commit"),
> 		 ....);
>
> Would this be more suitable for translators ?

Not really, the text surrounding "a commit" might need to be inflected 
differently depending on whether it is a SHA-1 or the "a commit" 
string. Word order might also be different.

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Mathieu Liénard--Mayor]

Le 2013-06-17 15:54, Peter Krefting a écrit :
> Mathieu Liénard--Mayor:
>
>> Actually, at first I dealt with it this way:
>>
>> status_printf_ln(s, color,
>>                 _("Splitting %s while rebasing branch '%s' on 
>> '%s'."),
>> 		 stopped_sha ? stopped_sha : _("a commit"),
>> 		 ....);
>>
>> Would this be more suitable for translators ?
>
> Not really, the text surrounding "a commit" might need to be
> inflected differently depending on whether it is a SHA-1 or the "a
> commit" string. Word order might also be different.
Okay, I'll use what you suggested then.
-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Johannes Sixt]

Am 6/17/2013 15:57, schrieb Mathieu Liénard--Mayor:
> Le 2013-06-17 15:54, Peter Krefting a écrit :
>> Mathieu Liénard--Mayor:
>>
>>> Actually, at first I dealt with it this way:
>>>
>>> status_printf_ln(s, color,
>>>                 _("Splitting %s while rebasing branch '%s' on '%s'."),
>>>          stopped_sha ? stopped_sha : _("a commit"),
>>>          ....);
>>>
>>> Would this be more suitable for translators ?
>>
>> Not really, the text surrounding "a commit" might need to be
>> inflected differently depending on whether it is a SHA-1 or the "a
>> commit" string. Word order might also be different.
> Okay, I'll use what you suggested then.

That's not a good idea. Do we already use "%1$" style formats elsewhere?
I'm afraid that they are not supported everywhere.

-- Hannes

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 6/17/2013 15:57, schrieb Mathieu Liénard--Mayor:
>> Le 2013-06-17 15:54, Peter Krefting a écrit :
>>> Mathieu Liénard--Mayor:
>>>
>>>> Actually, at first I dealt with it this way:
>>>>
>>>> status_printf_ln(s, color,
>>>>                 _("Splitting %s while rebasing branch '%s' on '%s'."),
>>>>          stopped_sha ? stopped_sha : _("a commit"),
>>>>          ....);
>>>>
>>>> Would this be more suitable for translators ?
>>>
>>> Not really, the text surrounding "a commit" might need to be
>>> inflected differently depending on whether it is a SHA-1 or the "a
>>> commit" string. Word order might also be different.
>> Okay, I'll use what you suggested then.
>
> That's not a good idea. Do we already use "%1$" style formats elsewhere?

In the template, we obviously don't.

But my understanding is that the reordering using printf() is the
mechanism we suggest l10n folks to use when the order of parameters
given to printf does not match the preferred word order in the
message in their language.

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Peter Krefting]

Junio C Hamano:

> But my understanding is that the reordering using printf() is the 
> mechanism we suggest l10n folks to use when the order of parameters 
> given to printf does not match the preferred word order in the 
> message in their language.

It's documented in the gettext manual, and seems to be used in the 
zh_CN.po to change the word order in quite a few places.

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Johannes Sixt]

Am 6/20/2013 9:56, schrieb Peter Krefting:
> Junio C Hamano:
> 
>> But my understanding is that the reordering using printf() is the
>> mechanism we suggest l10n folks to use when the order of parameters
>> given to printf does not match the preferred word order in the message
>> in their language.
> 
> It's documented in the gettext manual, and seems to be used in the
> zh_CN.po to change the word order in quite a few places.

It is fine to use %n$ in translated strings as long as gettext is enabled
only on systems that have a sufficiently capable printf and these formats
are not used in the source code.

But you can't have this string:

  "Splitting a commit while rebasing branch '%2$s' on '%3$s'."

neither in the template nor in the translation, because the numbers must
begin at 1 (and must be used without gaps).

-- Hannes

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 6/20/2013 9:56, schrieb Peter Krefting:
>> Junio C Hamano:
>> 
>>> But my understanding is that the reordering using printf() is the
>>> mechanism we suggest l10n folks to use when the order of parameters
>>> given to printf does not match the preferred word order in the message
>>> in their language.
>> 
>> It's documented in the gettext manual, and seems to be used in the
>> zh_CN.po to change the word order in quite a few places.
>
> It is fine to use %n$ in translated strings as long as gettext is enabled
> only on systems that have a sufficiently capable printf and these formats
> are not used in the source code.
>
> But you can't have this string:
>
>   "Splitting a commit while rebasing branch '%2$s' on '%3$s'."
>
> neither in the template nor in the translation, because the numbers must
> begin at 1 (and must be used without gaps).

Did any message we saw in the patch (and the discussion to possibly
improve it) need to have such a format string, or are you pointing
out a common gotcha we may want to warn translators about in
po/README?

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Johannes Sixt]

Am 6/20/2013 20:11, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>> But you can't have this string:
>>
>>   "Splitting a commit while rebasing branch '%2$s' on '%3$s'."
>>
>> neither in the template nor in the translation, because the numbers must
>> begin at 1 (and must be used without gaps).
> 
> Did any message we saw in the patch (and the discussion to possibly
> improve it) need to have such a format string, or are you pointing
> out a common gotcha we may want to warn translators about in
> po/README?

I took the example from Peter's message earlier in this thread:

http://thread.gmane.org/gmane.comp.version-control.git/228062/focus=228064

-- Hannes

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 6/20/2013 20:11, schrieb Junio C Hamano:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>>> But you can't have this string:
>>>
>>>   "Splitting a commit while rebasing branch '%2$s' on '%3$s'."
>>>
>>> neither in the template nor in the translation, because the numbers must
>>> begin at 1 (and must be used without gaps).
>> 
>> Did any message we saw in the patch (and the discussion to possibly
>> improve it) need to have such a format string, or are you pointing
>> out a common gotcha we may want to warn translators about in
>> po/README?
>
> I took the example from Peter's message earlier in this thread:
>
> http://thread.gmane.org/gmane.comp.version-control.git/228062/focus=228064

Yeah, that one would not work.

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Junio C Hamano]

Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>
writes:

> When in the middle of a rebase, it can be annoying to go in .git
> in order to find the SHA1 of the commit where the rebase stopped.
>
> git-status now includes this information in its default output.
> With this new information, the message is now shorter, to avoid
> too long lines.
>
> The new message looks like:
> $ git status
>  HEAD detached from 33e516f
>  Editing c346c87 while rebasing branch 'rebase_i_edit' on 'f90e540'.

Hmph.  It only looks into rebase-merge and not rebase-apply; is this
patch complete, or just to show a Work-In-Progress?

I do not think you need to introduce a new stopped-sha file (if you
need it, call that with "sha-1").  "git rebase [-i/-m]" knows where
it stopped and what the next step is without having to have such an
extra file.  Why should you need one?

It seems that wt_status_get_state() tries to read in-progress state
for various operations, and I think the logic to _detect_ what to
show (i.e. what is the next commit to be replayed?  how many more
remains to be replayed?, etc.) would mix well with that function.
Extend wt_status_state structure to hold the necessary info, query
the state from the filesystem in that function, and display the info
(but not collect info) in show_rebase_in_progress(), to keep the
clean division of labor between these two places.

Also, please pay closer attention to topics that are under
discussion in other threads.  I think Ram's "Fix 'checkout -' after
'rebase' finishes" topic cf.

  http://thread.gmane.org/gmane.comp.version-control.git/227994/focus=228092

makes the output reasonably better and consistent (please check what
I'll be pushing out on 'pu' later today after fixing some of them
up).  I suspect that this patch will conflict with it, so either you
would need to wait, or work together with that branch (i.e. rebase
on top of it as necessary), or something.

In the longer term to address issues discussed in this thread cf.

  http://thread.gmane.org/gmane.comp.version-control.git/227432/focus=227471

I think the right direction is *NOT* to keep the first "HEAD
detached at" line and to add more cruft to the status output as
additional lines, when various sequencer-like operations that
tentatively take you to detached HEAD state to give control back to
you in the middle.  "git status" knows what operation is in
progress, and I think we should start our output _without_ that
"HEAD detached at" line.

Thanks.

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Mathieu Liénard--Mayor]

Le 2013-06-17 20:37, Junio C Hamano a écrit :
> Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>
> writes:
>
>> When in the middle of a rebase, it can be annoying to go in .git
>> in order to find the SHA1 of the commit where the rebase stopped.
>>
>> git-status now includes this information in its default output.
>> With this new information, the message is now shorter, to avoid
>> too long lines.
>>
>> The new message looks like:
>> $ git status
>>  HEAD detached from 33e516f
>>  Editing c346c87 while rebasing branch 'rebase_i_edit' on 'f90e540'.
>
> Hmph.  It only looks into rebase-merge and not rebase-apply; is this
> patch complete, or just to show a Work-In-Progress?
It's a complete patch, at least we considered it as one.
We didn't want to change the output too much, so when the old message 
was
too vague (ie. saying "...ing a commit") we replaced "a commit" by the 
SHA1.
>
> I do not think you need to introduce a new stopped-sha file (if you
> need it, call that with "sha-1").  "git rebase [-i/-m]" knows where
> it stopped and what the next step is without having to have such an
> extra file.  Why should you need one?
I'm not following. At what point are we introducing a new file ?
What we meant to do was:
- if the user removed the file .git/.../stopped_sha for some reason,
   go back to the old "too vague" output
- otherwise, use the content of the file to display the SHA1 in the 
output
>
> It seems that wt_status_get_state() tries to read in-progress state
> for various operations, and I think the logic to _detect_ what to
> show (i.e. what is the next commit to be replayed?  how many more
> remains to be replayed?, etc.) would mix well with that function.
This patch is meant to be a first-step.
The only modification it's supposed to bring is the SHA1 where we 
stopped.
Display the list of what's left to be done isn't the purpose of this 
particular patch.
> Extend wt_status_state structure to hold the necessary info, query
> the state from the filesystem in that function, and display the info
> (but not collect info) in show_rebase_in_progress(), to keep the
> clean division of labor between these two places.
Do you mean that we should include the stopped_SHA in wt_status_state ?
>
> Also, please pay closer attention to topics that are under
> discussion in other threads.  I think Ram's "Fix 'checkout -' after
Will do.
> 'rebase' finishes" topic cf.
>
>   
> http://thread.gmane.org/gmane.comp.version-control.git/227994/focus=228092
>
> makes the output reasonably better and consistent (please check what
> I'll be pushing out on 'pu' later today after fixing some of them
> up).  I suspect that this patch will conflict with it, so either you
> would need to wait, or work together with that branch (i.e. rebase
> on top of it as necessary), or something.
We have several modifications to make, so in the end we'll rebase on 
top of it.
>
> In the longer term to address issues discussed in this thread cf.
>
>   
> http://thread.gmane.org/gmane.comp.version-control.git/227432/focus=227471
>
> I think the right direction is *NOT* to keep the first "HEAD
> detached at" line and to add more cruft to the status output as
> additional lines, when various sequencer-like operations that
> tentatively take you to detached HEAD state to give control back to
> you in the middle.  "git status" knows what operation is in
> progress, and I think we should start our output _without_ that
> "HEAD detached at" line.
>
> Thanks.

-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

Re: [PATCH] status: display the SHA1 of the commit being currently processed

[Junio C Hamano]

Mathieu Liénard--Mayor <mathieu.lienard--mayor@ensimag.fr> writes:

> Le 2013-06-17 20:37, Junio C Hamano a écrit :
> ...
>> Extend wt_status_state structure to hold the necessary info, query
>> the state from the filesystem in that function, and display the info
>> (but not collect info) in show_rebase_in_progress(), to keep the
>> clean division of labor between these two places.
>
> Do you mean that we should include the stopped_SHA in wt_status_state ?

That would be a good first step.  In the longer term, we would want
other external interfaces that read from wt-status-state, pick bits
necessary and summarize them for particular use cases.  You can for
example easily imagine the prompt support that collects different
pieces of breadcrumbs from .git/ can instead ask wt-status-state for
necessary pieces information.

Separating the logic to collect state information and the code to
show the result of collection is necessary for that kind of usage.