git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] status: display the SHA1 of the commit being currently processed
@ 2013-06-17 12:10 Mathieu Lienard--Mayor
  2013-06-17 12:44 ` Thomas Adam
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-17 12:10 UTC (permalink / raw)
  To: git; +Cc: gitster, Mathieu Lienard--Mayor, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 12:10 [PATCH] status: display the SHA1 of the commit being currently processed Mathieu Lienard--Mayor
@ 2013-06-17 12:44 ` Thomas Adam
  2013-06-17 13:10 ` Peter Krefting
  2013-06-17 18:37 ` Junio C Hamano
  2 siblings, 0 replies; 16+ messages in thread
From: Thomas Adam @ 2013-06-17 12:44 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor
  Cc: git list, Junio C Hamano, Jorge Juan Garcia Garcia, Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 12:10 [PATCH] status: display the SHA1 of the commit being currently processed Mathieu Lienard--Mayor
  2013-06-17 12:44 ` Thomas Adam
@ 2013-06-17 13:10 ` Peter Krefting
  2013-06-17 13:33   ` Mathieu Liénard--Mayor
  2013-06-17 18:37 ` Junio C Hamano
  2 siblings, 1 reply; 16+ messages in thread
From: Peter Krefting @ 2013-06-17 13:10 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor
  Cc: git, gitster, Jorge Juan Garcia Garcia, Matthieu Moy

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/

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

* Re: [PATCH] status: display the SHA1 of the commit being currently  processed
  2013-06-17 13:10 ` Peter Krefting
@ 2013-06-17 13:33   ` Mathieu Liénard--Mayor
  2013-06-17 13:54     ` Peter Krefting
  0 siblings, 1 reply; 16+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-17 13:33 UTC (permalink / raw)
  To: Peter Krefting
  Cc: Mathieu Lienard--Mayor, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 13:33   ` Mathieu Liénard--Mayor
@ 2013-06-17 13:54     ` Peter Krefting
  2013-06-17 13:57       ` Mathieu Liénard--Mayor
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Krefting @ 2013-06-17 13:54 UTC (permalink / raw)
  To: Mathieu Liénard--Mayor
  Cc: Mathieu Lienard--Mayor, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

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/

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

* Re: [PATCH] status: display the SHA1 of the commit being currently  processed
  2013-06-17 13:54     ` Peter Krefting
@ 2013-06-17 13:57       ` Mathieu Liénard--Mayor
  2013-06-17 15:10         ` Johannes Sixt
  0 siblings, 1 reply; 16+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-17 13:57 UTC (permalink / raw)
  To: Peter Krefting
  Cc: Mathieu Lienard--Mayor, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 13:57       ` Mathieu Liénard--Mayor
@ 2013-06-17 15:10         ` Johannes Sixt
  2013-06-17 16:33           ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Sixt @ 2013-06-17 15:10 UTC (permalink / raw)
  To: Mathieu Liénard--Mayor
  Cc: Peter Krefting, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 15:10         ` Johannes Sixt
@ 2013-06-17 16:33           ` Junio C Hamano
  2013-06-20  7:56             ` Peter Krefting
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2013-06-17 16:33 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Mathieu Liénard--Mayor, Peter Krefting, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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.

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 12:10 [PATCH] status: display the SHA1 of the commit being currently processed Mathieu Lienard--Mayor
  2013-06-17 12:44 ` Thomas Adam
  2013-06-17 13:10 ` Peter Krefting
@ 2013-06-17 18:37 ` Junio C Hamano
  2013-06-18 10:12   ` Mathieu Liénard--Mayor
  2 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2013-06-17 18:37 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor; +Cc: git, Jorge Juan Garcia Garcia, Matthieu Moy

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.

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

* Re: [PATCH] status: display the SHA1 of the commit being currently  processed
  2013-06-17 18:37 ` Junio C Hamano
@ 2013-06-18 10:12   ` Mathieu Liénard--Mayor
  2013-06-18 16:32     ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-18 10:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Mathieu Lienard--Mayor, git, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently  processed
  2013-06-18 10:12   ` Mathieu Liénard--Mayor
@ 2013-06-18 16:32     ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2013-06-18 16:32 UTC (permalink / raw)
  To: Mathieu Liénard--Mayor
  Cc: Mathieu Lienard--Mayor, git, Jorge Juan Garcia Garcia,
	Matthieu Moy

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.

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-17 16:33           ` Junio C Hamano
@ 2013-06-20  7:56             ` Peter Krefting
  2013-06-20  8:10               ` Johannes Sixt
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Krefting @ 2013-06-20  7:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Mathieu Liénard--Mayor, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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/

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-20  7:56             ` Peter Krefting
@ 2013-06-20  8:10               ` Johannes Sixt
  2013-06-20 18:11                 ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Sixt @ 2013-06-20  8:10 UTC (permalink / raw)
  To: Peter Krefting
  Cc: Junio C Hamano, Mathieu Liénard--Mayor, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-20  8:10               ` Johannes Sixt
@ 2013-06-20 18:11                 ` Junio C Hamano
  2013-06-21  5:34                   ` Johannes Sixt
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2013-06-20 18:11 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Peter Krefting, Mathieu Liénard--Mayor, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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?

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-20 18:11                 ` Junio C Hamano
@ 2013-06-21  5:34                   ` Johannes Sixt
  2013-06-21 14:23                     ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Sixt @ 2013-06-21  5:34 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Peter Krefting, Mathieu Liénard--Mayor, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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

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

* Re: [PATCH] status: display the SHA1 of the commit being currently processed
  2013-06-21  5:34                   ` Johannes Sixt
@ 2013-06-21 14:23                     ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2013-06-21 14:23 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Peter Krefting, Mathieu Liénard--Mayor, git,
	Jorge Juan Garcia Garcia, Matthieu Moy

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.

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

end of thread, other threads:[~2013-06-21 14:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-17 12:10 [PATCH] status: display the SHA1 of the commit being currently processed Mathieu Lienard--Mayor
2013-06-17 12:44 ` Thomas Adam
2013-06-17 13:10 ` Peter Krefting
2013-06-17 13:33   ` Mathieu Liénard--Mayor
2013-06-17 13:54     ` Peter Krefting
2013-06-17 13:57       ` Mathieu Liénard--Mayor
2013-06-17 15:10         ` Johannes Sixt
2013-06-17 16:33           ` Junio C Hamano
2013-06-20  7:56             ` Peter Krefting
2013-06-20  8:10               ` Johannes Sixt
2013-06-20 18:11                 ` Junio C Hamano
2013-06-21  5:34                   ` Johannes Sixt
2013-06-21 14:23                     ` Junio C Hamano
2013-06-17 18:37 ` Junio C Hamano
2013-06-18 10:12   ` Mathieu Liénard--Mayor
2013-06-18 16:32     ` Junio C Hamano

Code repositories for project(s) associated with this public 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).