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

Matthieu Moy [off-list ref] writes:

quoted

+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.

Junio C Hamano [off-list ref] writes

Matthieu Moy [off-list ref] writes: 

quoted

quoted

+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.