Re: [PATCH] status: display the SHA1 of the commit being currently processed
Johannes Sixt writes: > Am 6/20/2013 20:11, schrieb Junio C Hamano: >> Johannes Sixt 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Am 6/20/2013 20:11, schrieb Junio C Hamano: > Johannes Sixt 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Johannes Sixt 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Mathieu Liénard--Mayor 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Le 2013-06-17 20:37, Junio C Hamano a écrit : Mathieu Lienard--Mayor 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Mathieu Lienard--Mayor 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Johannes Sixt 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Hi,
[I rarely do reviews on this list, so feel free to ignore this.]
On 17 June 2013 13:10, Mathieu Lienard--Mayor
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] status: display the SHA1 of the commit being currently processed
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 Signed-off-by: Jorge Juan Garcia Garcia Signed-off-by: Matthieu Moy --- -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 --
