# [PATCH] status: display the SHA1 of the commit being currently processed 16 messages from 2013-06-17 to 2013-06-21. Participants: Mathieu Lienard--Mayor, Thomas Adam, Peter Krefting, Mathieu Liénard--Mayor, Johannes Sixt, Junio C Hamano. Thread: https://gitlist.dev/t/34175 ## Mathieu Lienard--Mayor, 2013-06-17 12:10 Subject: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <1371471004-9069-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr> URL: https://gitlist.dev/e/1371471004-9069-1-git-send-email-Mathieu.Lienard--Mayor%40ensimag.imag.fr ``` 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 --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 ``` ## Thomas Adam, 2013-06-17 12:44 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: URL: https://gitlist.dev/e/CA%2B39Oz7niTiaNfhaL3pi6%3DsDGpkpJ%2B%2B1W6eKb1zdHxAm6r_hSw%40mail.gmail.com In-Reply-To: <1371471004-9069-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr> ``` 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 ``` ## Peter Krefting, 2013-06-17 13:10 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1306171357440.7683%40perkele.intern.softwolves.pp.se In-Reply-To: <1371471004-9069-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr> ``` 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/ ``` ## Mathieu Liénard--Mayor, 2013-06-17 13:33 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: URL: https://gitlist.dev/e/c878cf98bff28ea8b72fb6d33fd9ca85%40ensibm.imag.fr In-Reply-To: ``` 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 ``` ## Peter Krefting, 2013-06-17 13:54 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1306171452340.7683%40perkele.intern.softwolves.pp.se In-Reply-To: ``` 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/ ``` ## Mathieu Liénard--Mayor, 2013-06-17 13:57 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <0b88af05ea8d4723a870476036992abc@ensibm.imag.fr> URL: https://gitlist.dev/e/0b88af05ea8d4723a870476036992abc%40ensibm.imag.fr In-Reply-To: ``` 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 ``` ## Johannes Sixt, 2013-06-17 15:10 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <51BF26F9.1080900@viscovery.net> URL: https://gitlist.dev/e/51BF26F9.1080900%40viscovery.net In-Reply-To: <0b88af05ea8d4723a870476036992abc@ensibm.imag.fr> ``` 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 ``` ## Junio C Hamano, 2013-06-17 16:33 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <7vvc5cg0i8.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvc5cg0i8.fsf%40alter.siamese.dyndns.org In-Reply-To: <51BF26F9.1080900@viscovery.net> ``` 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. ``` ## Junio C Hamano, 2013-06-17 18:37 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <7vy5a8d1my.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy5a8d1my.fsf%40alter.siamese.dyndns.org In-Reply-To: <1371471004-9069-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr> ``` 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. ``` ## Mathieu Liénard--Mayor, 2013-06-18 10:12 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <2dc2004fbf61d625515c2b6f62cc104e@ensibm.imag.fr> URL: https://gitlist.dev/e/2dc2004fbf61d625515c2b6f62cc104e%40ensibm.imag.fr In-Reply-To: <7vy5a8d1my.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Junio C Hamano, 2013-06-18 16:32 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <7v61xb752k.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v61xb752k.fsf%40alter.siamese.dyndns.org In-Reply-To: <2dc2004fbf61d625515c2b6f62cc104e@ensibm.imag.fr> ``` 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. ``` ## Peter Krefting, 2013-06-20 07:56 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1306200853250.7267%40ds9.cixit.se In-Reply-To: <7vvc5cg0i8.fsf@alter.siamese.dyndns.org> ``` 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/ ``` ## Johannes Sixt, 2013-06-20 08:10 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <51C2B8DA.6040400@viscovery.net> URL: https://gitlist.dev/e/51C2B8DA.6040400%40viscovery.net In-Reply-To: ``` 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 ``` ## Junio C Hamano, 2013-06-20 18:11 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <7vzjukveiz.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzjukveiz.fsf%40alter.siamese.dyndns.org In-Reply-To: <51C2B8DA.6040400@viscovery.net> ``` 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? ``` ## Johannes Sixt, 2013-06-21 05:34 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <51C3E5D7.6040604@viscovery.net> URL: https://gitlist.dev/e/51C3E5D7.6040604%40viscovery.net In-Reply-To: <7vzjukveiz.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Junio C Hamano, 2013-06-21 14:23 Subject: Re: [PATCH] status: display the SHA1 of the commit being currently processed Message-ID: <7vwqpno846.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vwqpno846.fsf%40alter.siamese.dyndns.org In-Reply-To: <51C3E5D7.6040604@viscovery.net> ``` 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. ```