Re: [PATCH] git-status: show short sequencer state
Phil Hord ho...@cisco.com writes: + if (state-substate==WT_SUBSTATE_NOMINAL) status_printf_ln(s, color, _(The current patch is empty.)); This looks weird. First, spaces around == (here and below). Then, the logic is unintuitive. The if suggests everything is allright, and the message below is very specific. This at least deserves a comment. Yes, I agree. It was less clear but more reasonable before I tried to clear it up some. It's driven by the short-token printer. The state is you're in a 'git am' but I do not see any conflicted files. Therefore, your patch must be empty. This was my guess, but I wouldn't have needed to guess if there was a comment in the code ;-). I'll try to make this more explicit. Currently the short-status version will say either am or am \n conflicted when a 'git am' is in progress. The logical path to follow if I re-add 'git-am-empty' state tracker is for this to now show either am \n am-is-empty or am \n conflicted. But I think I should suppress the am-is-empty report in that case. What do you think I don't think you should remove it from the output (no strong opinion). My point was just that the code looked weird. +static void wt_print_token(struct wt_status *s, const char *color, const char *token) +{ + color_fprintf(s-fp, color, %s, token); + fputc(s-null_termination ? '\0' : '\n', s-fp); +} The output format seems to be meant only for machine-consumption. Is there any case when we'd want color? [...] [...]I thought I might be going back there, or that I might combine this with full 'git status' again somehow, and colors seemed appropriate still. [...] So I can remove this color decorator until someone finds a need for it. I'm fine with both options, with a slight preference for removing them. My own use-case involves $PS1. That makes sense (indeed, the implementation of status hints was slightly inspired from what the bash prompt in contrib/completion/git-prompt.sh does). The next step could be to use your porcelain there instead of checking manually file existance. You may want to add a short note about this motivation in the commit message. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] git-status: show short sequencer state
Phil Hord ho...@cisco.com writes: + merge a git-merge is in progress + am a git-am is in progress + rebase a git-rebase is in progress + rebase-interactive a git-rebase--interactive is in progress + cherry-picka git-cherry-pick is in progress + bisect a git-bisect is in progress Avoid using git-foo syntax in documentation, it suggests that this is valid command, which isn't true anymore. `git foo` seems the most common syntax. Also, git-rebase--interactive is not user-visible = `git rebase --interactive`. - if (state-am_empty_patch) + if (state-substate==WT_SUBSTATE_NOMINAL) status_printf_ln(s, color, _(The current patch is empty.)); This looks weird. First, spaces around == (here and below). Then, the logic is unintuitive. The if suggests everything is allright, and the message below is very specific. This at least deserves a comment. if (advice_status_hints) { - if (!state-am_empty_patch) + if (state-substate==WT_SUBSTATE_CONFLICTED) Spaces around ==. +static void wt_print_token(struct wt_status *s, const char *color, const char *token) +{ + color_fprintf(s-fp, color, %s, token); + fputc(s-null_termination ? '\0' : '\n', s-fp); +} The output format seems to be meant only for machine-consumption. Is there any case when we'd want color? I'd say we can disable coloring completely for this format (normally, color=auto does the right thing, but I prefer being 100% sure I'll get no color when writing scripts) +static void wt_shortstatus_print_sequencer(struct wt_status *s) [...] +void wt_sequencer_print(struct wt_status *s) +{ + wt_shortstatus_print_sequencer(s); +} + Why do you need this trivial wrapper? Other than that, I like the idea (although I have no concrete use-case in mind), but I didn't actually test the patch. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] git-status: show short sequencer state
Matthieu Moy wrote: Phil Hord ho...@cisco.com writes: +merge a git-merge is in progress +am a git-am is in progress +rebase a git-rebase is in progress +rebase-interactive a git-rebase--interactive is in progress +cherry-picka git-cherry-pick is in progress +bisect a git-bisect is in progress Avoid using git-foo syntax in documentation, it suggests that this is valid command, which isn't true anymore. `git foo` seems the most common syntax. Also, git-rebase--interactive is not user-visible = `git rebase --interactive`. Thanks. -if (state-am_empty_patch) +if (state-substate==WT_SUBSTATE_NOMINAL) status_printf_ln(s, color, _(The current patch is empty.)); This looks weird. First, spaces around == (here and below). Then, the logic is unintuitive. The if suggests everything is allright, and the message below is very specific. This at least deserves a comment. Yes, I agree. It was less clear but more reasonable before I tried to clear it up some. It's driven by the short-token printer. The state is you're in a 'git am' but I do not see any conflicted files. Therefore, your patch must be empty. I suspect this may be a leaving from the git-am machinery where it did not choose to clean up after itself if the patch was empty, but perhaps it should have. I seldom use git-am, so I do not know if this code reflects a common and useful state. I'll try to make this more explicit. Currently the short-status version will say either am or am \n conflicted when a 'git am' is in progress. The logical path to follow if I re-add 'git-am-empty' state tracker is for this to now show either am \n am-is-empty or am \n conflicted. But I think I should suppress the am-is-empty report in that case. What do you think if (advice_status_hints) { -if (!state-am_empty_patch) +if (state-substate==WT_SUBSTATE_CONFLICTED) Spaces around ==. +static void wt_print_token(struct wt_status *s, const char *color, const char *token) +{ +color_fprintf(s-fp, color, %s, token); +fputc(s-null_termination ? '\0' : '\n', s-fp); +} The output format seems to be meant only for machine-consumption. Is there any case when we'd want color? I'd say we can disable coloring completely for this format (normally, color=auto does the right thing, but I prefer being 100% sure I'll get no color when writing scripts) Originally I had this output nested in the normal 'git status --short' output, like a shortened form of the advice. Then, 'git-status --porcelain' would show the tokens without color, but 'git status --short' would show them with color. I thought I might be going back there, or that I might combine this with full 'git status' again somehow, and colors seemed appropriate still. The --short status report is too confusing when tokens may or may-not appear, and it would likely break some scripts, even though they should be using --porcelain. And the full status already has its long versions of the same text. So I can remove this color decorator until someone finds a need for it. +static void wt_shortstatus_print_sequencer(struct wt_status *s) [...] +void wt_sequencer_print(struct wt_status *s) +{ +wt_shortstatus_print_sequencer(s); +} + Why do you need this trivial wrapper? Another left-over from its previous multiple versions. I'll simplify it. Other than that, I like the idea (although I have no concrete use-case in mind), but I didn't actually test the patch. My own use-case involves $PS1. I keep running into you cannot cherry-pick because you are in the middle of a rebase in submodules in which I have long forgotten about the failed action and have gone on to write many new commits, or switched branches, or worse. I do not know what 'git rebase --abort' will give me in those cases, and I wonder what work I might have lost for having been interrupted in the middle of that action in the past. These tokens will help me decorate my prompt to remind me I left some baggage untended. Thanks for the feedback. Phil -- 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
[PATCH] git-status: show short sequencer state
Recently git-status learned to display the state of the git sequencer in long form to help the user remember an interrupted command. This information is also useful in short form to humans and scripts, but no option is available to boil it down. Teach git-status to report the sequencer state in short form using a new --sequencer (-S) switch. Output zero or more simple state token strings indicating the deduced state of the git sequencer. Introduce a common function to determine the current sequencer state so the regular status function and this short version can share common code. Add a substate to wt_status_state to track more detailed information about a state, such as conflicted or resolved. Move the am_empty_patch flage out of wt_status_state and into this new substate. State token strings which may be emitted and their meanings: merge a git-merge is in progress am a git-am is in progress rebase a git-rebase is in progress rebase-interactive a git-rebase--interactive is in progress cherry-picka git-cherry-pick is in progress bisect a git-bisect is in progress conflicted there are unresolved conflicts resolved conflicts have been resolved editinginteractive rebase stopped to edit a commit edited interactive rebase edit has been edited splitting interactive rebase, commit is being split I also considered adding these tokens, but I decided it was not appropriate since these changes are not sequencer-related. But it is possible I am being too short-sighted or have chosen the switch name poorly. clean index modified untracked --- Documentation/git-status.txt | 18 +++ builtin/commit.c | 12 - wt-status.c | 125 +++ wt-status.h | 14 - 4 files changed, 145 insertions(+), 24 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 67e5f53..31ffabd 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -38,6 +38,24 @@ OPTIONS across git versions and regardless of user configuration. See below for details. +-S:: +--sequence:: + Show the git sequencer status. This shows zero or more tokens + describing the state of several git sequence operations. Each + token is separated by a newline. ++ + merge a merge is in progress + am an am is in progress + rebase a rebase is in progress + rebase-interactive an interactive rebase is in progress + cherry-picka cherry-pick is in progress + bisect a bisect is in progress + conflicted there are unresolved conflicts + resolved conflicts have been resolved + editinginteractive rebase stopped to edit a commit + edited interactive rebase edit has been edited + splitting interactive rebase, commit is being split + -u[mode]:: --untracked-files[=mode]:: Show untracked files. diff --git a/builtin/commit.c b/builtin/commit.c index a17a5df..9706ed9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -114,7 +114,8 @@ static struct strbuf message = STRBUF_INIT; static enum { STATUS_FORMAT_LONG, STATUS_FORMAT_SHORT, - STATUS_FORMAT_PORCELAIN + STATUS_FORMAT_PORCELAIN, + STATUS_FORMAT_SEQUENCER } status_format = STATUS_FORMAT_LONG; static int opt_parse_m(const struct option *opt, const char *arg, int unset) @@ -454,6 +455,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int case STATUS_FORMAT_PORCELAIN: wt_porcelain_print(s); break; + case STATUS_FORMAT_SEQUENCER: + wt_sequencer_print(s); + break; case STATUS_FORMAT_LONG: wt_status_print(s); break; @@ -1156,6 +1160,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) N_(show status concisely), STATUS_FORMAT_SHORT), OPT_BOOLEAN('b', branch, s.show_branch, N_(show branch information)), + OPT_SET_INT('S', sequence, status_format, + N_(show sequencer state), + STATUS_FORMAT_SEQUENCER), OPT_SET_INT(0, porcelain, status_format, N_(machine-readable output), STATUS_FORMAT_PORCELAIN), @@ -1216,6 +1223,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) case STATUS_FORMAT_PORCELAIN: wt_porcelain_print(s); break; + case STATUS_FORMAT_SEQUENCER: + wt_sequencer_print(s); + break;
[PATCH] git-status: show short sequencer state
Recently git-status learned to display the state of the git sequencer in long form to help the user remember an interrupted command. This information is also useful in short form to humans and scripts, but no option is available to boil it down. Teach git-status to report the sequencer state in short form using a new --sequencer (-S) switch. Output zero or more simple state token strings indicating the deduced state of the git sequencer. Introduce a common function to determine the current sequencer state so the regular status function and this short version can share common code. Add a substate to wt_status_state to track more detailed information about a state, such as conflicted or resolved. Move the am_empty_patch flage out of wt_status_state and into this new substate. State token strings which may be emitted and their meanings: merge a git-merge is in progress am a git-am is in progress rebase a git-rebase is in progress rebase-interactive a git-rebase--interactive is in progress cherry-picka git-cherry-pick is in progress bisect a git-bisect is in progress conflicted there are unresolved conflicts resolved conflicts have been resolved editinginteractive rebase stopped to edit a commit edited interactive rebase edit has been edited splitting interactive rebase, commit is being split I also considered adding these tokens, but I decided it was not appropriate since these changes are not sequencer-related. But it is possible I am being too short-sighted or have chosen the switch name poorly. clean index modified untracked --- Documentation/git-status.txt | 18 ++ builtin/commit.c | 12 +++- wt-status.c | 128 +++ wt-status.h | 13 - 4 files changed, 146 insertions(+), 25 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 67e5f53..200a8e2 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -38,6 +38,24 @@ OPTIONS across git versions and regardless of user configuration. See below for details. +-S:: +--sequence:: + Show the git sequencer status. This shows zero or more tokens + describing the state of several git sequence operations. Each + token is separated by a newline. ++ + merge a git-merge is in progress + am a git-am is in progress + rebase a git-rebase is in progress + rebase-interactive a git-rebase--interactive is in progress + cherry-picka git-cherry-pick is in progress + bisect a git-bisect is in progress + conflicted there are unresolved conflicts + resolved conflicts have been resolved + editinginteractive rebase stopped to edit a commit + edited interactive rebase edit has been edited + splitting interactive rebase, commit is being split + -u[mode]:: --untracked-files[=mode]:: Show untracked files. diff --git a/builtin/commit.c b/builtin/commit.c index a17a5df..9706ed9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -114,7 +114,8 @@ static struct strbuf message = STRBUF_INIT; static enum { STATUS_FORMAT_LONG, STATUS_FORMAT_SHORT, - STATUS_FORMAT_PORCELAIN + STATUS_FORMAT_PORCELAIN, + STATUS_FORMAT_SEQUENCER } status_format = STATUS_FORMAT_LONG; static int opt_parse_m(const struct option *opt, const char *arg, int unset) @@ -454,6 +455,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int case STATUS_FORMAT_PORCELAIN: wt_porcelain_print(s); break; + case STATUS_FORMAT_SEQUENCER: + wt_sequencer_print(s); + break; case STATUS_FORMAT_LONG: wt_status_print(s); break; @@ -1156,6 +1160,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) N_(show status concisely), STATUS_FORMAT_SHORT), OPT_BOOLEAN('b', branch, s.show_branch, N_(show branch information)), + OPT_SET_INT('S', sequence, status_format, + N_(show sequencer state), + STATUS_FORMAT_SEQUENCER), OPT_SET_INT(0, porcelain, status_format, N_(machine-readable output), STATUS_FORMAT_PORCELAIN), @@ -1216,6 +1223,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) case STATUS_FORMAT_PORCELAIN: wt_porcelain_print(s); break; + case STATUS_FORMAT_SEQUENCER: + wt_sequencer_print(s); +