# sequencer "edit" command always issues a warning 4 messages from 2017-03-16 to 2017-03-17. Participants: Jeff King, Junio C Hamano, Johannes Schindelin. Thread: https://gitlist.dev/t/45395 ## Jeff King, 2017-03-16 13:15 Subject: sequencer "edit" command always issues a warning Message-ID: <20170316131534.4vpbub3vmde7maua@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170316131534.4vpbub3vmde7maua%40sigill.intra.peff.net ``` I was rebasing with the new built-in sequencer code today, and I was surprised to see the use of warning() here: $ git rebase -i [set one commit to 'edit'] warning: stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name You can amend the commit now, with [...more instructions...] It alarmed me for a minute until I realized that no, this is nothing to be alarmed about, but just git doing exactly what I told it to do. The original just wrote: Stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name It would be easy to switch back: diff --git a/sequencer.c b/sequencer.c index 1f729b053..8183a83c1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1997,7 +1997,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) if (item->command == TODO_EDIT) { struct commit *commit = item->commit; if (!res) - warning(_("stopped at %s... %.*s"), + fprintf(stderr, + _("Stopped at %s... %.*s"), short_commit_name(commit), item->arg_len, item->arg); return error_with_patch(commit, and that would match most of the other messages that the command issues, which use a bare fprintf() and start with a capital letter. But I'm not sure if there was some reason to treat this one differently. -Peff ``` ## Junio C Hamano, 2017-03-16 22:29 Subject: Re: sequencer "edit" command always issues a warning Message-ID: URL: https://gitlist.dev/e/xmqqy3w4ama4.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20170316131534.4vpbub3vmde7maua@sigill.intra.peff.net> ``` Jeff King writes: > I was rebasing with the new built-in sequencer code today, and I was > surprised to see the use of warning() here: > > $ git rebase -i > [set one commit to 'edit'] > warning: stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name > You can amend the commit now, with > [...more instructions...] > > It alarmed me for a minute until I realized that no, this is nothing to > be alarmed about, but just git doing exactly what I told it to do. > > The original just wrote: > > Stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name > > It would be easy to switch back: > > diff --git a/sequencer.c b/sequencer.c > index 1f729b053..8183a83c1 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1997,7 +1997,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) > if (item->command == TODO_EDIT) { > struct commit *commit = item->commit; > if (!res) > - warning(_("stopped at %s... %.*s"), > + fprintf(stderr, > + _("Stopped at %s... %.*s"), > short_commit_name(commit), > item->arg_len, item->arg); > return error_with_patch(commit, > > and that would match most of the other messages that the command issues, > which use a bare fprintf() and start with a capital letter. But I'm not > sure if there was some reason to treat this one differently. I doubt there was. At least, I didn't think I read any rationale for switching in logs or in-code comments. If we stop due to conflicting changes during a "rebase -i" or a range "cherry-pick/revert" session, warning() might make sense, but I agree with you that stopping at "edit" is an expected thing. If we had info("stopped at...") that may be appropriate, but writing it out to stderr is just fine, I would think. Thanks for spotting. ``` ## Johannes Schindelin, 2017-03-16 22:59 Subject: Re: sequencer "edit" command always issues a warning Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.20.1703162358520.3767%40virtualbox In-Reply-To: <20170316131534.4vpbub3vmde7maua@sigill.intra.peff.net> ``` Hi Peff, On Thu, 16 Mar 2017, Jeff King wrote: > I was rebasing with the new built-in sequencer code today, and I was > surprised to see the use of warning() here: > > $ git rebase -i > [set one commit to 'edit'] > warning: stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name > You can amend the commit now, with > [...more instructions...] > > It alarmed me for a minute until I realized that no, this is nothing to > be alarmed about, but just git doing exactly what I told it to do. > > The original just wrote: > > Stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name > > It would be easy to switch back: > > diff --git a/sequencer.c b/sequencer.c > index 1f729b053..8183a83c1 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1997,7 +1997,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) > if (item->command == TODO_EDIT) { > struct commit *commit = item->commit; > if (!res) > - warning(_("stopped at %s... %.*s"), > + fprintf(stderr, > + _("Stopped at %s... %.*s"), > short_commit_name(commit), > item->arg_len, item->arg); > return error_with_patch(commit, > > and that would match most of the other messages that the command issues, > which use a bare fprintf() and start with a capital letter. But I'm not > sure if there was some reason to treat this one differently. I do not recall why I chose warning(); probably just an oversight on my part. Your patch looks good. Ciao, Dscho ``` ## Jeff King, 2017-03-17 00:19 Subject: Re: sequencer "edit" command always issues a warning Message-ID: <20170317001942.2lndsi47kdyzhane@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170317001942.2lndsi47kdyzhane%40sigill.intra.peff.net In-Reply-To: ``` On Thu, Mar 16, 2017 at 11:59:35PM +0100, Johannes Schindelin wrote: > I do not recall why I chose warning(); probably just an oversight on my > part. Your patch looks good. Thanks. Here it is with a commit message. I agree with Junio that if we had "info()" it would perhaps be the right thing to use. We can go down that road later if we choose to. -- >8 -- Subject: [PATCH] sequencer: drop "warning:" when stopping for edit Since the conversion from shell to C in 56dc3ab04 (sequencer (rebase -i): implement the 'edit' command, 2017-01-02), stopping at an "edit" instruction went from: $ git rebase -i Stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name You can amend the commit now, with [...more instructions...] to: $ git rebase -i warning: stopped at 6ce6b914a... odb_pack_keep(): stop generating keepfile name You can amend the commit now, with [...more instructions...] The "warning" implies that it's something unexpected, but it's not. Let's switch back to the original message. Signed-off-by: Jeff King --- sequencer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 1f729b053..8183a83c1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1997,7 +1997,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) if (item->command == TODO_EDIT) { struct commit *commit = item->commit; if (!res) - warning(_("stopped at %s... %.*s"), + fprintf(stderr, + _("Stopped at %s... %.*s"), short_commit_name(commit), item->arg_len, item->arg); return error_with_patch(commit, -- 2.12.0.623.g86ec6c963 ```