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

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

 



"Junio C Hamano" <gitster@xxxxxxxxx> writes:

>Guillaume Pagès <guillaume.pages@xxxxxxxxxxxxxxxxxxxxxxx> writes: 

>> git status gives more information during rebase -i, about the list of 
>> command that are done during the rebase. It displays the two last 
>> commands executed and the two next lines to be executed. It also gives 
>> hints to find the whole files in .git directory. 
>> --- 

>Without 1/4 and 2/4 in the same thread, it is hard to guess what you 
>wanted to do with these two patches. Remember, reviewers review not 
>just your patches but those from many others. 

>I would in the meantime assume these are replacement patches for the 
>ones in 

>http://thread.gmane.org/gmane.comp.version-control.git/270743/focus=270744 

You assumed correctly, I didn't wanted to flood the mailing list and I
assumed it would be linked correctly with the previous since. Looks like
I was wrong.

>> diff --git a/wt-status.c b/wt-status.c 
>> index c83eca5..7f88470 100644 
>> --- a/wt-status.c 
>> +++ b/wt-status.c 
>> @@ -1026,12 +1026,73 @@ static int split_commit_in_progress(struct wt_status *s) 
>> return split_in_progress; 
>> } 
>> 
>> +static void show_rebase_information(struct wt_status *s, 
>> + struct wt_status_state *state, 
>> + const char *color) 
>> +{ 
>> + if (state->rebase_interactive_in_progress) { 
>> + int i, begin; 
>> + int lines_to_show_nr = 2; 

>"lines_to_show = 2" or "nr_lines_to_show = 2" would be easier to read. 

Done

>> + 
>> + struct strbuf buf = STRBUF_INIT; 
>> + struct string_list have_done = STRING_LIST_INIT_DUP; 
>> + struct string_list yet_to_do = STRING_LIST_INIT_DUP; 
>> + 
>> + strbuf_read_file(&buf, git_path("rebase-merge/done"), 0); 
>> + stripspace(&buf, 1); 
>> + have_done.nr = string_list_split(&have_done, buf.buf, '\n', -1); 
>> + string_list_remove_empty_items(&have_done, 1); 
>> + strbuf_release(&buf); 
>> + 
>> + strbuf_read_file(&buf, git_path("rebase-merge/git-rebase-todo"), 0); 
>> + stripspace(&buf, 1); 
>> + string_list_split(&yet_to_do, buf.buf, '\n', -1); 
>> + string_list_remove_empty_items(&yet_to_do, 1); 
>> + strbuf_release(&buf); 
>> + 
>> + if (have_done.nr == 0) 
>> + status_printf_ln(s, color, _("No commands done.")); 

>Do the users even need to be told that, I wonder? 

I guess it removes the ambiguity of being told nothing.

>> + else{ 

>Style: "else {" 

Ok thanks.

>> + status_printf_ln(s, color, 
>> + Q_("Last command done (%d command done):", 
>> + "Last commands done (%d commands done):", 
>> + have_done.nr), 
>> + have_done.nr); 
>> + begin = (have_done.nr > lines_to_show_nr) ? have_done.nr-lines_to_show_nr : 0; 
>> + for (i = begin; i < have_done.nr; i++) { 
>> + status_printf_ln(s, color, " %s", have_done.items[i].string); 
>> + } 

>Hmm, perhaps fold lines like this (and you do not need "begin")? 

>for (i = (lines_to_show_nr < have_done.nr) 
>? have_done.nr - lines_to_show_nr : 0; 
>i < have_done.nr; 
>i++) 
>status_printf_ln(...);



>> + if (have_done.nr > lines_to_show_nr && s->hints) 
>> + status_printf_ln(s, color, 
>> + _(" (see more in file %s)"), git_path("rebase-merge/done")); 

>That's a nice touch ;-) 

>> + } 
>> + if (yet_to_do.nr == 0) 
>> + status_printf_ln(s, color, 
>> + _("No commands remaining.")); 

>This I can see why we may want to say it. 

>> + else{ 
>> + 
>> + status_printf_ln(s, color, 
>> + Q_("Next command to do (%d remaining command):", 
>> + "Next commands to do (%d remaining commands):", 
>> + yet_to_do.nr), 
>> + yet_to_do.nr); 
>> + for (i = 0; i < lines_to_show_nr && i < yet_to_do.nr; i++) { 
>> + status_printf_ln(s, color, " %s", yet_to_do.items[i].string); 
>> + } 
>> + if (s->hints) 
>> + status_printf_ln(s, color, 
>> + _(" (use \"git rebase --edit-todo\" to view and edit)")); 
>> + } 

>Make sure you do not leak memory used by two string lists here... 

Done, thanks again.





--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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]