Re: [PATCH 3/4] status: give more information during rebase -i
Junio C Hamano gits...@pobox.com writes: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] status: factor two rebase-related messages together
Junio C Hamano gits...@pobox.com writes: Hmmm, it obviously does not break anything but it is not obvious why this is a good change. Is it that you wanted to have a single instance of if on a branch, we say 'you are rebasing that branch', otherwise we say 'you are rebasing'? Even then, I am not sure if this code movement was the best way to do so (an obvious alternative is to use a shared helper function and call from the two arms of if/elseif/... chain). I made this change because at first sight, this piece of code was difficult to read for me. There was two long branches very similar and I had to spot the differences, and the actual differences were at the very end of the branches so I had to check back what the condition was about. It seems now much more natural to me: the part in common of both branches is in OR-condition and the differences between branches are gathered with the test on the variable they depend. By the way, I agree that this change is not absolutely necessary. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] status: give more information during rebase -i
Junio C Hamano gits...@pobox.com writes Matthieu Moy matthieu@grenoble-inp.fr writes: +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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] status: fix tests to handle new verbose git status during rebase
Thanks for reviewing, I take everything into account and release a v2 ASAP. Guillaume -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] potential error in parsing git checkout options?
Git version : 2.4.1.652.g8fd8657 When we run git checkout -babar, we would expect an error message like unknown switch 'a' and we get Switched to a new branch 'abar'. We are not sure since we don't entirely understand the syntax -b branch which is shown in the documentation, but as average users, we found it a bit surprising. Guillaume Pagès -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2] create a skeleton for the command git rebase --status
After this discussion I eventually agree that it would be better upgrading git status than creating a new command.When people use git status, it means that they need information to continue their work, so if you don't even know that you are in a rebase, you will very likely need information about the current rebase. During a classic rebase we could have output like: rebase in progress; onto d9d448a You are currently rebasing branch 'branche1' on 'd9d448a'. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) (use git rebase --abort to check out the original branch) (5 commits applied, 3 remainings) Failed to apply: 252c273 commit message Unmerged paths: (use git reset HEAD file... to unstage) (use git add file... to mark resolution) both modified: file1 And during an interactive rebase: rebase in progress; onto 247c883 You are currently editing a commit while rebasing branch 'b1.1' on '247c883'. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) Last commands done (5 commands done) : pick 62609785 commit message1 reword 85ae9001 new commit message2 (See more at .git/rebase-merge/done) Next commands to do (3 remainings commands) : squash 62609785 commit message3 pick 85ae9001 commit message4 (See more at .git/rebase-merge/git-rebase-todo) Changes not staged for commit: (use git add file... to update what will be committed) (use git checkout -- file... to discard changes in working directory) modified: file1 ... Is it a good practice to send the user find information in the .git directory? Thanks Guillaume -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Implementation of git rebase --status
Junio C Hamano gits...@pobox.com writes: Guillaume Pages guillaume.pa...@ensimag.grenoble-inp.fr writes: Do you think it could be useful or do you have any suggestion? All of your examples say things like: You are in the middle of a rebase session. The line that paused this session is: but what if there is no such line? IOW, what does the user see when using this new option during a git rebase (not git rebase -i)? I guess it should display the sha1 of the patch that failed in this case. Other than that, sounds like a neat thing to do. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Implementation of git rebase --status
Hi, I would like to implement a new command git rebase --status to inform the user about the current rebase session. Here is a sample of what I think it could look like in case of merge conflict: git rebase --status You are in the middle of a rebase session. The line that paused this session is: pick 848a16f commit with conflicts CONFLICT (content): Merge conflict in file1 Consult and edit remaining actions with git rebase --edit-todo In case of syntax error: git rebase --status You are in the middle of a rebase session. The line that paused this session is: tick 848a16f syntax error SYNTAX ERROR Consult and edit remaining actions with git rebase --edit-todo In case of error during the execution of a script: git rebase --status You are in the middle of a rebase session. The line that paused this session is: exec exit 3 ERROR IN SCRIPT EXECUTION Consult and edit remaining actions with git rebase --edit-todo Do you think it could be usefull or do you have any suggestion? Thanks, Guillaume Pagès -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html