Re: [PATCH 3/4] status: give more information during rebase -i

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 4, 2015, 17:19 UTC
Message-ID: <xmqqtwunv1bv.fsf@gitster.dls.corp.google.com>
In-Reply-To: <vpqfv67zylq.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Show 9 quoted lines

>> +void get_two_last_lines(char *filename, int *numlines, char **lines)
>> +{
>> +...
>> +}
>> +
>> +void get_two_first_lines(char *filename, int *numlines, char **lines)
>> +{
>> +...
>> +}

I had a handful of comments on these:

 - Do we need two separate and overly specific functions like these,
   i.e. "two" and "first/last"?

 - Wouldn't people want to be able to configure the number of lines?

 - Do we really want get_two_{first,last}_LINES() functions?

   I am wondering if insn sheets these functions read include
   comments, in which case get_n_{first,last}_commands() may be a
   more correct name.

 - Wouldn't it be necessary for these functions to report the total
   number of commands, instead of giving "void" back?  Otherwise how
   would the caller produce summary like this:

       An interactive rebase of 14 commits in progress.  You have
       replayed 4 commits so far, the last few of which were:

          da66b27 remote.c: provide per-branch pushremote name
          f052154 remote.c: hoist branch.*.remote lookup out of

       and 10 more commits to go, the next few of which are:

          a9f9f8c remote.c: introduce branch_get_upstream helper
          8770e6f remote.c: hoist read_config into remote_get_1
       
   Note that I am not suggesting the phrasing or presentation.  The
   information content is what I am interested in.