From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 01 Sep 2016 08:45:51 GMT
Subject: Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file
Message-ID: <alpine.DEB.2.20.1609011038430.129229@virtualbox>
In-Reply-To: <52d61bef-668b-fdc4-30b3-a34c11b39f81@gmail.com>

Hi Kuba,

On Wed, 31 Aug 2016, Jakub Narębski wrote:

> W dniu 29.08.2016 o 10:05, Johannes Schindelin pisze:
> 
> > The `git-rebase-todo` file contains a list of commands. Most of those
> > commands have the form
> > 
> > 	<verb> <sha1> <oneline>
> > 
> > The <oneline> is displayed primarily for the user's convenience, as
> > rebase -i really interprets only the <verb> <sha1> part. However, there
> > are *some* places in interactive rebase where the <oneline> is used to
> > display messages, e.g. for reporting at which commit we stopped.
> > 
> > So let's just remember it when parsing the todo file; we keep a copy of
> > the entire todo file anyway (to write out the new `done` and
> > `git-rebase-todo` file just before processing each command), so all we
> > need to do is remember the begin and end offsets.
> 
> Actually what we remember is pointer and length, or begin offset and length,
> not offset and offset.

Right. Fixed.

> > diff --git a/sequencer.c b/sequencer.c
> > index 06759d4..3398774 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -709,6 +709,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
> >  struct todo_item {
> >  	enum todo_command command;
> >  	struct commit *commit;
> > +	const char *arg;
> > +	int arg_len;
> 
> Why 'arg', and not 'oneline', or 'subject'?
> I'm not saying it is bad name.

Because we will use it for `exec` commands' args, too. Clarified in the
commit message.

> > @@ -760,6 +762,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
> >  	status = get_sha1(bol, commit_sha1);
> >  	*end_of_object_name = saved;
> >  
> > +	item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
> > +	item->arg_len = (int)(eol - item->arg);
> > +
> 
> Does it work correctly for line without <oneline>, that is
> 
>   	<verb> <sha1>
> 
> I think it does, but I not entirely sure.

It does work correctly: in the example, *end_of_object_name would be '\n',
and strspn(end_of_object_name, " \t") would return 0.

Thanks for the review!
Dscho