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

From: Guillaume Pages <guillaume.pages@ensimag.grenoble-inp.fr>
Date: Jun 5, 2015, 16:35 UTC
Message-ID: <261867654.193546.1433522151490.JavaMail.zimbra@ensimag.grenoble-inp.fr>
In-Reply-To: <xmqqtwunv1bv.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes

>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:

I felt that was not the right way to do so. What do you think of a function like that:

/*
 * Puts nb_commands commands from filename in lines,
 * returns the total number of commands in the file
 * ignores comments and empty lines
 * lines needs to be at least of size nb_commands
 * part: 0 get last commands
 * 	 1 get first commands
 */

int get_commands(char *filename, int nb_commands, char **lines, int part)

Maybe part is not the best word to choose to take the beginning or the end of the file. I also hesitate about adding a parameter to ignore or not the comments.