Re: [PATCH v3 07/25] sequencer: completely revamp the "todo" script parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Tue, 11 Oct 2016, Johannes Schindelin wrote:

> On Tue, 11 Oct 2016, Johannes Schindelin wrote:
> 
> > -- snipsnap --
> > @@ -906,11 +904,13 @@ static int walk_revs_populate_todo(struct todo_list
> > *todo_list,
> >                 item->command = command;
> >                 item->commit = commit;
> >                 item->offset_in_buf = todo_list->buf.len;
> > +               strbuf_addstr(&todo_list->buf, command);
> 
> This would be command_string instead of command, of course.
> 
> > +               strbuf_addch(&todo_list->buf, ' ');
> > +               strbuf_add_unique_abbrev(&todo_list->buf,
> > +                                        commit->object.oid.hash,
> > +                                        DEFAULT_ABBREV);
> >                 subject_len = find_commit_subject(commit_buffer, &subject);
> > -               strbuf_addf(&todo_list->buf, "%s %s %.*s\n",
> >                 command_string,
> > -                       find_unique_abbrev(commit->object.oid.hash,
> > -                               DEFAULT_ABBREV),
> > -                       subject_len, subject);
> > +               strbuf_add(&todo_list->buf, subject, subject_len);
> >                 unuse_commit_buffer(commit, commit_buffer);
> >         }
> >         return 0;

In the end, I decided to actually *not* use strbuf_add_unique_abbrev()
here because it really makes the code very much too ugly after the
introduction of short_commit_name():

-- snip --
@@ -1093,10 +1093,16 @@ static int walk_revs_populate_todo(struct
todo_list *todo_list,
                item->arg = NULL;
                item->arg_len = 0;
                item->offset_in_buf = todo_list->buf.len;
+               strbuf_addstr(&todo_list->buf, command_string);
+               strbuf_addch(&todo_list->buf, ' ');
+               strbuf_add_unique_abbrev(&todo_list->buf,
+                                        commit->object.oid.hash,
+                                        DEFAULT_ABBREV);
+               strbuf_addch(&todo_list->buf, ' ');
                subject_len = find_commit_subject(commit_buffer, &subject);
-               strbuf_addf(&todo_list->buf, "%s %s %.*s\n",
                command_string,
-                       short_commit_name(commit), subject_len, subject);
+               strbuf_add(&todo_list->buf, subject, subject_len);
                unuse_commit_buffer(commit, commit_buffer);
+               strbuf_addch(&todo_list->buf, '\n');
        }
        return 0;
 }
-- snap --

I hope you will forgive me disagreeing with you here.

To make it easier to accept, I reordered the short_commit_name() patch so
it comes before revamping the todo parsing.

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]