[PATCHv2] git-status: show short sequencer state

[PATCHv2] git-status: show short sequencer state

[Phil Hord]

Updated per Matthieu's comments, adding Sign-off and fixing my prefix to have a little "v2" on the end.

Sorry for the extra noise.

Phil

[PATCHv2] git-status: show short sequencer state

[Phil Hord]

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-pick        a git-cherry-pick is in progress
    bisect             a git-bisect is in progress
    conflicted         there are unresolved conflicts
    resolved           conflicts have been resolved
    editing            interactive 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

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 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-pick        a cherry-pick is in progress
+	bisect             a bisect is in progress
+	conflicted         there are unresolved conflicts
+	resolved           conflicts have been resolved
+	editing            interactive 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;
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
 		s.ignore_submodule_arg = ignore_submodule_arg;
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..81d91e3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -781,7 +781,7 @@ static void show_merge_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	if (has_unmerged(s)) {
+	if (state->substate == WT_SUBSTATE_CONFLICTED) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -802,11 +802,11 @@ static void show_am_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color,
 		_("You are in the middle of an am session."));
-	if (state->am_empty_patch)
+	if (state->substate == WT_SUBSTATE_AM_EMPTY)
 		status_printf_ln(s, color,
 			_("The current patch is empty."));
 	if (advice_status_hints) {
-		if (!state->am_empty_patch)
+		if (state->substate == WT_SUBSTATE_CONFLICTED)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and then run \"git am --resolved\")"));
 		status_printf_ln(s, color,
@@ -867,9 +867,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	struct stat st;
-
-	if (has_unmerged(s)) {
+	if (state->substate == WT_SUBSTATE_CONFLICTED) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints) {
 			status_printf_ln(s, color,
@@ -879,19 +877,19 @@ static void show_rebase_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --abort\" to check out the original branch)"));
 		}
-	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
+	} else if (state->substate == WT_SUBSTATE_RESOLVED) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
-	} else if (split_commit_in_progress(s)) {
+	} else if (state->substate == WT_SUBSTATE_SPLITTING) {
 		status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (Once your working directory is clean, run \"git rebase --continue\")"));
 	} else {
 		status_printf_ln(s, color, _("You are currently editing a commit during a rebase."));
-		if (advice_status_hints && !s->amend) {
+		if (advice_status_hints && state->substate == WT_SUBSTATE_EDITING) {
 			status_printf_ln(s, color,
 				_("  (use \"git commit --amend\" to amend the current commit)"));
 			status_printf_ln(s, color,
@@ -907,7 +905,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color, _("You are currently cherry-picking."));
 	if (advice_status_hints) {
-		if (has_unmerged(s))
+		if (state->substate == WT_SUBSTATE_CONFLICTED)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
 		else
@@ -928,34 +926,68 @@ static void show_bisect_in_progress(struct wt_status *s,
 	wt_status_print_trailer(s);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_get_state(struct wt_status *s , struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
+	// Determine main sequencer activity
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->substate = WT_SUBSTATE_AM_EMPTY;
+			else
+				state->substate = WT_SUBSTATE_CONFLICTED;
 		} else {
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
-		state.bisect_in_progress = 1;
+		state->bisect_in_progress = 1;
+
+	// Check for unmerged files
+	if (state->rebase_in_progress || state->rebase_interactive_in_progress ||
+			state->cherry_pick_in_progress || state->merge_in_progress) {
+		if (has_unmerged(s)) {
+			state->substate = WT_SUBSTATE_CONFLICTED;
+		} else {
+			state->substate = WT_SUBSTATE_RESOLVED;
+		}
+	}
+
+	// Interactive Rebase is more nuanced
+	if (state->rebase_interactive_in_progress && state->substate != WT_SUBSTATE_CONFLICTED) {
+		if (!stat(git_path("MERGE_MSG"), &st)) {
+			state->substate = WT_SUBSTATE_RESOLVED;
+		} else if (split_commit_in_progress(s)) {
+			state->substate = WT_SUBSTATE_SPLITTING;
+		} else {
+			if (s->amend) {
+				state->substate = WT_SUBSTATE_EDITED;
+			} else {
+				state->substate = WT_SUBSTATE_EDITING;
+			}
+		}
+	}
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
@@ -1192,6 +1224,55 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
+static void wt_print_token(struct wt_status *s, const char *token)
+{
+	fprintf(s->fp, "%s", token);
+	fputc(s->null_termination ? '\0' : '\n', s->fp);
+}
+
+void wt_sequencer_print(struct wt_status *s)
+{
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
+
+	if (state.merge_in_progress)
+		wt_print_token(s, "merge");
+	if (state.am_in_progress)
+		wt_print_token(s, "am");
+	if (state.rebase_in_progress)
+		wt_print_token(s, "rebase");
+	if (state.rebase_interactive_in_progress)
+		wt_print_token(s, "rebase-interactive");
+	if (state.cherry_pick_in_progress)
+		wt_print_token(s, "cherry-pick");
+	if (state.bisect_in_progress)
+		wt_print_token(s, "bisect");
+
+	switch (state.substate) {
+	case WT_SUBSTATE_NOMINAL:
+		break;
+	case WT_SUBSTATE_CONFLICTED:
+		wt_print_token(s, "conflicted");
+		break;
+	case WT_SUBSTATE_RESOLVED:
+		wt_print_token(s, "resolved");
+		break;
+	case WT_SUBSTATE_EDITED:
+		wt_print_token(s, "edited");
+		break;
+	case WT_SUBSTATE_EDITING:
+		wt_print_token(s, "editing");
+		break;
+	case WT_SUBSTATE_SPLITTING:
+		wt_print_token(s, "splitting");
+		break;
+	case WT_SUBSTATE_AM_EMPTY:
+		wt_print_token(s, "am-empty");
+		break;
+	}
+}
+
 void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
diff --git a/wt-status.h b/wt-status.h
index 236b41f..900889c 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -59,6 +59,7 @@ struct wt_status {
 	unsigned colopts;
 	int null_termination;
 	int show_branch;
+	int show_sequencer;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
@@ -71,14 +72,24 @@ struct wt_status {
 	struct string_list ignored;
 };
 
+enum wt_status_substate {
+	WT_SUBSTATE_NOMINAL = 0,
+	WT_SUBSTATE_CONFLICTED,
+	WT_SUBSTATE_RESOLVED,
+	WT_SUBSTATE_SPLITTING,
+	WT_SUBSTATE_EDITING,
+	WT_SUBSTATE_EDITED,
+	WT_SUBSTATE_AM_EMPTY,
+};
+
 struct wt_status_state {
 	int merge_in_progress;
 	int am_in_progress;
-	int am_empty_patch;
 	int rebase_in_progress;
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
+	enum wt_status_substate substate;
 };
 
 void wt_status_prepare(struct wt_status *s);
@@ -86,6 +97,7 @@ void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 
 void wt_shortstatus_print(struct wt_status *s);
+void wt_sequencer_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
 
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...)
-- 
1.8.0.2.gc921d59.dirty

Re: [PATCHv2] git-status: show short sequencer state

[Jeff King]

On Tue, Oct 23, 2012 at 04:02:54PM -0400, Phil Hord wrote:

> 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 patch ended up quite long. It might be a little easier to review
if it were broken into refactoring steps (I have not looked at it too
closely yet, but it seems like the three paragraphs above could each be
their own commit).

> 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

A minor nit, but you might want to update this list from the one in the
documentation.

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

Hmm. So the new format is its own distinct output format. I could not
say "I would like to see short status, and by the way, show me the
sequencer state", as you can with "-b". Is it possible to do this (or
even desirable; getting the sequencer state should be way cheaper, so
conflating the two may not be what some callers want).

Not complaining, just wondering about the intended use cases.

Also, does there need to be a --porcelain version of this output? It
seems like we can have multiple words (e.g., in a merge, with conflicted
entries). If there is no arbitrary data, we do not have to worry about
delimiters and quoting.  But I wonder if we would ever want to expand
the information to include arbitrary strings, at which point we would
want NUL delimiters; should we start with that now?

> +	// Determine main sequencer activity

Please avoid C99 comments (there are others in the patch, too).

> +void wt_sequencer_print(struct wt_status *s)
> +{
> +	struct wt_status_state state;
> +
> +	wt_status_get_state(s, &state);
> +
> +	if (state.merge_in_progress)
> +		wt_print_token(s, "merge");
> +	if (state.am_in_progress)
> +		wt_print_token(s, "am");
> +	if (state.rebase_in_progress)
> +		wt_print_token(s, "rebase");
> +	if (state.rebase_interactive_in_progress)
> +		wt_print_token(s, "rebase-interactive");
> +	if (state.cherry_pick_in_progress)
> +		wt_print_token(s, "cherry-pick");
> +	if (state.bisect_in_progress)
> +		wt_print_token(s, "bisect");
> +
> +	switch (state.substate) {
> +	case WT_SUBSTATE_NOMINAL:
> +		break;
> +	case WT_SUBSTATE_CONFLICTED:
> +		wt_print_token(s, "conflicted");
> +		break;
> +	case WT_SUBSTATE_RESOLVED:
> +		wt_print_token(s, "resolved");
> +		break;
> +	case WT_SUBSTATE_EDITED:
> +		wt_print_token(s, "edited");
> +		break;
> +	case WT_SUBSTATE_EDITING:
> +		wt_print_token(s, "editing");
> +		break;
> +	case WT_SUBSTATE_SPLITTING:
> +		wt_print_token(s, "splitting");
> +		break;
> +	case WT_SUBSTATE_AM_EMPTY:
> +		wt_print_token(s, "am-empty");
> +		break;
> +	}
> +}

It is clear from this code that some tokens can happen together, and
some are mutually exclusive. Should the documentation talk about that,
or do we want to literally keep it as a list of tags?

-Peff

Re: [PATCHv2] git-status: show short sequencer state

[Phil Hord]

Jeff King wrote:
> On Tue, Oct 23, 2012 at 04:02:54PM -0400, Phil Hord wrote:
>
>> 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 patch ended up quite long. It might be a little easier to review
> if it were broken into refactoring steps (I have not looked at it too
> closely yet, but it seems like the three paragraphs above could each be
> their own commit).

I can do that.

>> 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
> A minor nit, but you might want to update this list from the one in the
> documentation.

I considered it, but I also considered the audience; then I took the
easier path.  I'll look again.

>> 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;
> Hmm. So the new format is its own distinct output format. I could not
> say "I would like to see short status, and by the way, show me the
> sequencer state", as you can with "-b". Is it possible to do this (or
> even desirable; getting the sequencer state should be way cheaper, so
> conflating the two may not be what some callers want).
>
> Not complaining, just wondering about the intended use cases.

Originally I did place this output in the short-format display.  I
particularly want this info to be available for scripts. But it feels
"right" to include it with the short status, since the long-form is
already available with 'git status --no-short'.

Since there may be more than one line reported, I did not feel there was
a simple way to contain these details in with the short status.  For
branch this is a simple decision as the reported branch will take
exactly one line.

  git status -b -s
  ## master
   M Foo

But for sequencer state, this seemed like it could be too convoluted or
might encourage too much reliance line-counting:

  git status -b -s -S
  ## master
  ## merge
  ## conflicted
   M Foo

Maybe a different line-prefix would help make this clearer:

  git status -b -s -S
  ## master
  #! merge
  #! conflicted
   M Foo

It seemed to me this was someone else's itch and I might not scratch it
properly.  But I am willing to try if you think this is more useful than
distracting.

> Also, does there need to be a --porcelain version of this output? It
> seems like we can have multiple words (e.g., in a merge, with conflicted
> entries). If there is no arbitrary data, we do not have to worry about
> delimiters and quoting.  But I wonder if we would ever want to expand
> the information to include arbitrary strings, at which point we would
> want NUL delimiters; should we start with that now?

This works, though I haven't tested it on v2:

   git status -S -z


>> +	// Determine main sequencer activity
> Please avoid C99 comments (there are others in the patch, too).

Thanks.

>> +void wt_sequencer_print(struct wt_status *s)
>> +{
>> +	struct wt_status_state state;
>> +
>> +	wt_status_get_state(s, &state);
>> +
>> +	if (state.merge_in_progress)
>> +		wt_print_token(s, "merge");
>> +	if (state.am_in_progress)
>> +		wt_print_token(s, "am");
>> +	if (state.rebase_in_progress)
>> +		wt_print_token(s, "rebase");
>> +	if (state.rebase_interactive_in_progress)
>> +		wt_print_token(s, "rebase-interactive");
>> +	if (state.cherry_pick_in_progress)
>> +		wt_print_token(s, "cherry-pick");
>> +	if (state.bisect_in_progress)
>> +		wt_print_token(s, "bisect");
>> +
>> +	switch (state.substate) {
>> +	case WT_SUBSTATE_NOMINAL:
>> +		break;
>> +	case WT_SUBSTATE_CONFLICTED:
>> +		wt_print_token(s, "conflicted");
>> +		break;
>> +	case WT_SUBSTATE_RESOLVED:
>> +		wt_print_token(s, "resolved");
>> +		break;
>> +	case WT_SUBSTATE_EDITED:
>> +		wt_print_token(s, "edited");
>> +		break;
>> +	case WT_SUBSTATE_EDITING:
>> +		wt_print_token(s, "editing");
>> +		break;
>> +	case WT_SUBSTATE_SPLITTING:
>> +		wt_print_token(s, "splitting");
>> +		break;
>> +	case WT_SUBSTATE_AM_EMPTY:
>> +		wt_print_token(s, "am-empty");
>> +		break;
>> +	}
>> +}
> It is clear from this code that some tokens can happen together, and
> some are mutually exclusive. Should the documentation talk about that,
> or do we want to literally keep it as a list of tags?
>
> -Peff

I want to literally keep it as a list of tags.  This code does not
presume any exclusivity in the main state and I I do not think it
should.  I can imagine being in a "rebase cherry-pick conflicted" state,
even though the current wt_status_get_state function does not allow
detecting it.  I think that function is overly restrictive, but it was
not important to me yet.

Perhaps it is useful to indicate that the substate values are mutually
exclusive, but I also feel like this is an implementation detail.  It is
possible to be in "rebase-interactive edited conflicted" state, but it
is not possible for this code to tell you that. But should we document
that fact and thus restrict future improvement in this area?  My first
inclination is to say no, that we should leave this open.  But on the
other hand, I can see how someone could be confused if their expected
token never appeared only because another one took precedence.

Maybe the exclusive states should be explicit in the implementation. 
Maybe I should not have a substate at all but each of these substates
should be a separate boolean (or ternary).  Something like this:

 struct wt_status_state {
        int merge_in_progress;
        int am_in_progress;
+       int am_empty_patch;
-       int rebase_in_progress;
-       int rebase_interactive_in_progress;
+       int rebase_in_progress;       /* 0=no, 1=normal, 2=interactive */
        int cherry_pick_in_progress;
        int bisect_in_progress;
        enum wt_status_substate substate;
+       int conflicts;                    /* 0=none, 1=yes, 2=resolved */
+       int editing;                      /* 0=no, 1=editing, 2=edited */
 };

My primary motivation is to standardize the detection of these various
states so scripts do not need to know, for example, to look for the
presence of '$GIT_DIR/rebase-merge' or that a zero-length
'$GIT_DIR/rebase-apply/patch' is significant.  An immediate need is to
show some tags in my shell prompt when my current state is not clean. 
The current patch meets this latter need well.  If I add the output of
$(git status -S) to my prompt, I see nothing when I am "clean" and I see
_something_ when I am not.

But the former need is not so clear.  A script today which is interested
in an interactive rebase may simply say $(git status -S | grep
"^rebase-interactive$").  But a new flavor of rebase in the future could
muddy the waters.  Maybe there is a need for another reporting state
which provides a definitive answer on all possible tokens.  Consider
that I am in an interactive-rebase which has encountered a conflict. 

1. Simple state, useful for humans and prompts:

  $ git status -S    
  rebase-interactive
  conflicted


2. More details for scripts who may not care to distinguish between
inclusive things like "rebase" and "rebase-interactive":

  $ git status -S=detailed
  rebase
  rebase-interactive
  conflicted


3. Complete disambiguation:

  git status -S=all
  no-merge
  no-am
  rebase
  rebase-interactive
  no-cherry-pick
  no-bisect
  conflicted
  no-resolved
  no-edited
  no-editing
  no-splitting
  no-am-empty

Now a script interested in a condition can check for it explicitly in
two forms: "condition" and "no-condition".  If neither is found, he can
assume that his token is obsolete (possibly it was replaced with
something more explicit).  And if his goal is to detect any kind of
rebase, he can look for the "rebase" token rather than worrying about
its myriad types.

But at this point I feel overwhelmed by the possibilities.  Uncertainty
comes from trying to scratch someone else's itch.  But at the same time,
I do want to make this status-detector appealing as "The One True Way to
check git's state" to avoid more deviating implementations in the future.

And so I am conflicted.

I apologize for being so long-winded on this.  Thanks for playing
along.  I welcome your thoughts and advice.

Phil

Re: [PATCHv2] git-status: show short sequencer state

[Phil Hord]

On Thu, Oct 25, 2012 at 12:05 PM, Phil Hord <hordp@cisco.com> wrote:
>
> Jeff King wrote:
>> On Tue, Oct 23, 2012 at 04:02:54PM -0400, Phil Hord wrote:
>>
>>> 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 patch ended up quite long. It might be a little easier to review
>> if it were broken into refactoring steps (I have not looked at it too
>> closely yet, but it seems like the three paragraphs above could each be
>> their own commit).

I'm currently splitting this out into a series and reconsidering some
of it along the way.  I need some guidance.

I want to support these two modes:

  A.  'git status --short' with sequence tokens added:
       ## conflicted
       ## merge
       ?? untracked-workdir-file
       etc.

  B.  Same as (A) but without workdir status:
       ## conflicted
       ## merge

The user who wants 'A' would initiate it like this:
    git status --sequencer
  or
    git status -S

How do I spell the options for 'B'?  I have come up with these three
possibilities:
    git --sequencer-only   # Another switch
    git --sequencer=only   # An OPTARG parser
    git -S -S           # like git-diff -C -C, an OPT_COUNTUP

The first one is easy but weird, imho.
The second seems silly for just one type of option.
The last one is cheap to implement, but harder to explain in Documentation/

Any opinions?

Phil

Re: [PATCHv2] git-status: show short sequencer state

[Jeff King]

On Mon, Oct 29, 2012 at 02:05:14PM -0400, Phil Hord wrote:

> I'm currently splitting this out into a series and reconsidering some
> of it along the way.  I need some guidance.
> 
> I want to support these two modes:
> 
>   A.  'git status --short' with sequence tokens added:
>        ## conflicted
>        ## merge
>        ?? untracked-workdir-file
>        etc.
> 
>   B.  Same as (A) but without workdir status:
>        ## conflicted
>        ## merge
> 
> The user who wants 'A' would initiate it like this:
>     git status --sequencer
>   or
>     git status -S
> 
> How do I spell the options for 'B'?  I have come up with these three
> possibilities:
>     git --sequencer-only   # Another switch
>     git --sequencer=only   # An OPTARG parser
>     git -S -S           # like git-diff -C -C, an OPT_COUNTUP

Might it be easier to spell 'A' as:

  git status --short -S

and B as:

  git status -S

this is sort of like how "-b" works (except you cannot currently ask for
it separately, but arguably you could). If we have a proliferation of
such options, then we might need config to help turn them on all the
time (I'd guess people are probably already using aliases to do this).

-Peff

Re: [PATCHv2] git-status: show short sequencer state

[Phil Hord]

On Mon, Oct 29, 2012 at 5:41 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 02:05:14PM -0400, Phil Hord wrote:
>
>> I'm currently splitting this out into a series and reconsidering some
>> of it along the way.  I need some guidance.
>>
>> I want to support these two modes:
>>
>>   A.  'git status --short' with sequence tokens added:
>>        ## conflicted
>>        ## merge
>>        ?? untracked-workdir-file
>>        etc.
>>
>>   B.  Same as (A) but without workdir status:
>>        ## conflicted
>>        ## merge
>>
>> The user who wants 'A' would initiate it like this:
>>     git status --sequencer
>>   or
>>     git status -S
>>
>> How do I spell the options for 'B'?  I have come up with these three
>> possibilities:
>>     git --sequencer-only   # Another switch
>>     git --sequencer=only   # An OPTARG parser
>>     git -S -S           # like git-diff -C -C, an OPT_COUNTUP
>
> Might it be easier to spell 'A' as:
>
>   git status --short -S
>
> and B as:
>
>   git status -S
>
> this is sort of like how "-b" works (except you cannot currently ask for
> it separately, but arguably you could). If we have a proliferation of
> such options, then we might need config to help turn them on all the
> time (I'd guess people are probably already using aliases to do this).

I think I like this path.

I expect a common idiom to be 'git status -S --porcelain --null', and
both --porcelain and --null imply --short.  I think I can still do The
Right Thing, but the code is starting to spaghettify.  I'll take a
crack at it.

Thanks.

P

[PATCHv2 0/3] git-status short sequencer state info

[Phil Hord]

V2 of this patch breaks it into a series, cleans up some gaffes,
and adds more flexible display options.  I still need to add some
tests.  Until I do that, maybe this should be considered a WIP.

 [PATCHv2 1/3] Refactor print_state into get_state
 [PATCHv2 2/3] wt-status: More state retrieval abstraction
 [PATCHv2 3/3] git-status: show short sequencer state

[PATCHv2 1/3] Refactor print_state into get_state

[Phil Hord]

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 useful to other callers who do
not want it printed in the same way.

Split the new print_state function into separate get_state and
print_state functions so others can get this state info and
share common code.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 wt-status.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2a9658b..760f52b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -928,34 +928,41 @@ static void show_bisect_in_progress(struct wt_status *s,
 	wt_status_print_trailer(s);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_get_state(struct wt_status *s , struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
+	/* Determine sequencer activity */
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
-		state.bisect_in_progress = 1;
+		state->bisect_in_progress = 1;
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv2 2/3] wt-status: More state retrieval abstraction

[Phil Hord]

wt_status_print_state retrieves some sequencer state
information via wt_status_get_state, but other state
information it deduces on its own.  Replaces these
"local knowledge" deductions with wt_status variables
so we can share more common code in the future.
---
 wt-status.c | 16 +++++++++-------
 wt-status.h |  3 +++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 760f52b..a888120 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -781,7 +781,7 @@ static void show_merge_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -867,9 +867,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	struct stat st;
-
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints) {
 			status_printf_ln(s, color,
@@ -879,12 +877,12 @@ static void show_rebase_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --abort\" to check out the original branch)"));
 		}
-	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
+	} else if (state->rebase_in_progress || state->commit_is_pending) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
-	} else if (split_commit_in_progress(s)) {
+	} else if (state->split_in_progress) {
 		status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -907,7 +905,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color, _("You are currently cherry-picking."));
 	if (advice_status_hints) {
-		if (has_unmerged(s))
+		if (state->has_unmerged)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
 		else
@@ -955,6 +953,10 @@ static void wt_status_get_state(struct wt_status *s , struct wt_status_state *st
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
 		state->bisect_in_progress = 1;
+
+	state->has_unmerged = has_unmerged(s);
+	state->split_in_progress = split_commit_in_progress(s);
+	state->commit_is_pending = !stat(git_path("MERGE_MSG"), &st);
 }
 
 static void wt_status_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 236b41f..0b866a2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,6 +79,9 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
+	int split_in_progress;
+	int has_unmerged;
+	int commit_is_pending;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv2 3/3] git-status: show short sequencer state

[Phil Hord]

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.

Sequencer state info tokens are displayed in "short"
form.  'git status --short -S' will show the normal short
status including the sequencer tokens.  'git status -S'
without --short will show only the new sequencer information
tokens in whatever the requested format is.

State token strings which may be emitted and their meanings:
    merge              a merge is in progress
    am                 an am is in progress
    am-is-empty        the am patch is empty
    rebase             a rebase is in progress
    rebase-interactive an interactive rebase is in progress
    cherry-pick        a cherry-pick is in progress
    bisect             a bisect is in progress
    conflicted         there are unresolved conflicts
    commit-pending     a commit operation is waiting to be completed
    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.
    changed-index  Changes exist in the index
    changed-files  Changes exist in the working directory
    untracked      New files exist in the working directory

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 Documentation/git-status.txt | 16 ++++++++++++++++
 builtin/commit.c             | 14 ++++++++++++++
 wt-status.c                  | 35 +++++++++++++++++++++++++++++++++++
 wt-status.h                  |  7 +++++++
 4 files changed, 72 insertions(+)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 67e5f53..7a699f1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -38,6 +38,22 @@ 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 strings
+	describing the state of the git sequencer.
++
+	merge              a merge is in progress
+	am                 an am is in progress
+	am-is-empty        the am patch is empty
+	rebase             a rebase is in progress
+	rebase-interactive an interactive rebase is in progress
+	cherry-pick        a cherry-pick is in progress
+	bisect             a bisect is in progress
+	conflicted         there are unresolved conflicts
+	commit-pending     a commit operation is waiting to be completed
+	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..bf16cc6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1060,6 +1060,11 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (s->null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s->show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s->show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 	if (status_format != STATUS_FORMAT_LONG)
 		dry_run = 1;
 
@@ -1156,6 +1161,8 @@ 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', "sequencer", &s.show_sequencer,
+				N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"),
 			    STATUS_FORMAT_PORCELAIN),
@@ -1188,6 +1195,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	if (s.null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s.show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s.show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 
 	handle_untracked_files_arg(&s);
 	if (show_ignored_in_status)
@@ -1384,6 +1396,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
 			    STATUS_FORMAT_SHORT),
 		OPT_BOOLEAN(0, "branch", &s.show_branch, N_("show branch information")),
+		OPT_SET_INT(0, "sequencer", &s.show_sequencer,
+			    N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"), STATUS_FORMAT_PORCELAIN),
 		OPT_BOOLEAN('z', "null", &s.null_termination,
diff --git a/wt-status.c b/wt-status.c
index a888120..aee9745 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1201,6 +1201,34 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
+static void wt_print_token(struct wt_status *s, const char * token_color, const char *token, int active)
+{
+	if (active) {
+		color_fprintf(s->fp, token_color, "## %s", token);
+		fputc(s->null_termination ? '\0' : '\n', s->fp);
+	}
+}
+
+static void wt_sequencer_print(struct wt_status *s)
+{
+	struct wt_status_state state;
+	const char *header_color = color(WT_STATUS_HEADER, s);
+
+	wt_status_get_state(s, &state);
+
+	wt_print_token(s, header_color, "merge", state.merge_in_progress);
+	wt_print_token(s, header_color, "am", state.am_in_progress);
+	wt_print_token(s, header_color, "rebase", state.rebase_in_progress);
+	wt_print_token(s, header_color, "rebase-interactive", state.rebase_interactive_in_progress);
+	wt_print_token(s, header_color, "cherry-pick", state.cherry_pick_in_progress);
+	wt_print_token(s, header_color, "bisect", state.bisect_in_progress);
+	wt_print_token(s, header_color, "am-empty", state.am_empty_patch);
+
+	wt_print_token(s, header_color, "conflicted", state.has_unmerged);
+	wt_print_token(s, header_color, "commit-pending", state.commit_is_pending);
+	wt_print_token(s, header_color, "splitting", state.split_in_progress);
+}
+
 void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
@@ -1208,6 +1236,13 @@ void wt_shortstatus_print(struct wt_status *s)
 	if (s->show_branch)
 		wt_shortstatus_print_tracking(s);
 
+	if (s->show_sequencer)
+	{
+		wt_sequencer_print(s);
+		if (s->show_sequencer == SHOW_SEQUENCER_ONLY)
+			return;
+	}
+
 	for (i = 0; i < s->change.nr; i++) {
 		struct wt_status_change_data *d;
 		struct string_list_item *it;
diff --git a/wt-status.h b/wt-status.h
index 0b866a2..df4b36d 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -24,6 +24,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum sequencer_status_type {
+	SHOW_SEQUENCER_NO=0,
+	SHOW_SEQUENCER_YES,
+	SHOW_SEQUENCER_ONLY
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -59,6 +65,7 @@ struct wt_status {
 	unsigned colopts;
 	int null_termination;
 	int show_branch;
+	enum sequencer_status_type show_sequencer;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv3 0/4] git-status short sequencer state info

[Phil Hord]

I think this series is fairly complete at this point. I am still
missing some tests in 4/4, but only because I am not sure how to
induce those conditions.  Maybe I should enlist help from $(git-blame).

 [PATCHv3 1/4] Refactor print_state into get_state
 [PATCHv3 2/4] wt-status: More state retrieval abstraction
 [PATCHv3 3/4] git-status: show short sequencer state
 [PATCHv3 4/4] Add tests for git-status --sequencer

[PATCHv3 1/4] Refactor print_state into get_state

[Phil Hord]

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 useful to other callers who do
not want it printed in the same way.

Split the new print_state function into separate get_state and
print_state functions so others can get this state info and
share common code.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 wt-status.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2a9658b..760f52b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -928,34 +928,41 @@ static void show_bisect_in_progress(struct wt_status *s,
 	wt_status_print_trailer(s);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_get_state(struct wt_status *s , struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
+	/* Determine sequencer activity */
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
-		state.bisect_in_progress = 1;
+		state->bisect_in_progress = 1;
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv3 2/4] wt-status: Teach sequencer advice to use get_state

[Phil Hord]

wt_status_print_state retrieves some sequencer state information via
wt_status_get_state, but other state information it deduces on its
own.  Replace these "local knowledge" deductions with wt_status
variables so we can share more common code in the future.
---
 wt-status.c | 16 +++++++++-------
 wt-status.h |  3 +++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 760f52b..a888120 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -781,7 +781,7 @@ static void show_merge_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -867,9 +867,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	struct stat st;
-
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints) {
 			status_printf_ln(s, color,
@@ -879,12 +877,12 @@ static void show_rebase_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --abort\" to check out the original branch)"));
 		}
-	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
+	} else if (state->rebase_in_progress || state->commit_is_pending) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
-	} else if (split_commit_in_progress(s)) {
+	} else if (state->split_in_progress) {
 		status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -907,7 +905,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color, _("You are currently cherry-picking."));
 	if (advice_status_hints) {
-		if (has_unmerged(s))
+		if (state->has_unmerged)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
 		else
@@ -955,6 +953,10 @@ static void wt_status_get_state(struct wt_status *s , struct wt_status_state *st
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
 		state->bisect_in_progress = 1;
+
+	state->has_unmerged = has_unmerged(s);
+	state->split_in_progress = split_commit_in_progress(s);
+	state->commit_is_pending = !stat(git_path("MERGE_MSG"), &st);
 }
 
 static void wt_status_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 236b41f..0b866a2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,6 +79,9 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
+	int split_in_progress;
+	int has_unmerged;
+	int commit_is_pending;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv3 3/4] git-status: show short sequencer state

[Phil Hord]

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.

Sequencer state info tokens are displayed in "short"
form.  'git status --short -S' will show the normal short
status including the sequencer tokens.  'git status -S'
without --short will show only the new sequencer information
tokens in whatever the requested format is.

State token strings which may be emitted and their meanings:
    merge              a merge is in progress
    am                 an am is in progress
    am-is-empty        the am patch is empty
    rebase             a rebase is in progress
    rebase-interactive an interactive rebase is in progress
    cherry-pick        a cherry-pick is in progress
    bisect             a bisect is in progress
    conflicted         there are unresolved conflicts
    commit-pending     a commit operation is waiting to be completed
    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.
    changed-index  Changes exist in the index
    changed-files  Changes exist in the working directory
    untracked      New files exist in the working directory

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 Documentation/git-status.txt | 20 ++++++++++++++++++++
 builtin/commit.c             | 14 ++++++++++++++
 wt-status.c                  | 34 ++++++++++++++++++++++++++++++++++
 wt-status.h                  |  7 +++++++
 4 files changed, 75 insertions(+)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 67e5f53..bdacf08 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -38,6 +38,26 @@ 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 words
+	describing the state of the git sequencer. If --short is also
+	given, the words are shown at the beginning of the short-format
+	display after the one-line branch information, if present.  If
+	--short is not given, then only the sequencer words are shown
+	with no other status information.
++
+	merge              a merge is in progress
+	am                 an am is in progress
+	am-is-empty        the am patch is empty
+	rebase             a rebase is in progress
+	rebase-interactive an interactive rebase is in progress
+	cherry-pick        a cherry-pick is in progress
+	bisect             a bisect is in progress
+	conflicted         there are unresolved conflicts
+	commit-pending     a commit operation is waiting to be completed
+	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..bf16cc6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1060,6 +1060,11 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (s->null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s->show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s->show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 	if (status_format != STATUS_FORMAT_LONG)
 		dry_run = 1;
 
@@ -1156,6 +1161,8 @@ 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', "sequencer", &s.show_sequencer,
+				N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"),
 			    STATUS_FORMAT_PORCELAIN),
@@ -1188,6 +1195,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	if (s.null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s.show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s.show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 
 	handle_untracked_files_arg(&s);
 	if (show_ignored_in_status)
@@ -1384,6 +1396,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
 			    STATUS_FORMAT_SHORT),
 		OPT_BOOLEAN(0, "branch", &s.show_branch, N_("show branch information")),
+		OPT_SET_INT(0, "sequencer", &s.show_sequencer,
+			    N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"), STATUS_FORMAT_PORCELAIN),
 		OPT_BOOLEAN('z', "null", &s.null_termination,
diff --git a/wt-status.c b/wt-status.c
index a888120..71da1fb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1201,6 +1201,33 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
+static void wt_print_token(struct wt_status *s, const char *token, int active)
+{
+	if (active) {
+		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "## %s", prefix, token);
+		fputc(s->null_termination ? '\0' : '\n', s->fp);
+	}
+}
+
+static void wt_sequencer_print(struct wt_status *s)
+{
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
+
+	wt_print_token(s, "merge", state.merge_in_progress);
+	wt_print_token(s, "am", state.am_in_progress);
+	wt_print_token(s, "rebase", state.rebase_in_progress);
+	wt_print_token(s, "rebase-interactive", state.rebase_interactive_in_progress);
+	wt_print_token(s, "cherry-pick", state.cherry_pick_in_progress);
+	wt_print_token(s, "bisect", state.bisect_in_progress);
+	wt_print_token(s, "am-empty", state.am_empty_patch);
+
+	wt_print_token(s, "conflicted", state.has_unmerged);
+	wt_print_token(s, "commit-pending", state.commit_is_pending);
+	wt_print_token(s, "splitting", state.split_in_progress);
+}
+
 void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
@@ -1208,6 +1235,13 @@ void wt_shortstatus_print(struct wt_status *s)
 	if (s->show_branch)
 		wt_shortstatus_print_tracking(s);
 
+	if (s->show_sequencer)
+	{
+		wt_sequencer_print(s);
+		if (s->show_sequencer == SHOW_SEQUENCER_ONLY)
+			return;
+	}
+
 	for (i = 0; i < s->change.nr; i++) {
 		struct wt_status_change_data *d;
 		struct string_list_item *it;
diff --git a/wt-status.h b/wt-status.h
index 0b866a2..df4b36d 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -24,6 +24,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum sequencer_status_type {
+	SHOW_SEQUENCER_NO=0,
+	SHOW_SEQUENCER_YES,
+	SHOW_SEQUENCER_ONLY
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -59,6 +65,7 @@ struct wt_status {
 	unsigned colopts;
 	int null_termination;
 	int show_branch;
+	enum sequencer_status_type show_sequencer;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.8.0.3.gde9c7d5.dirty

[PATCHv3 4/4] Add tests for git-status --sequencer

[Phil Hord]

Add t7061-status-sequence.sh with tests for the various
'git status --sequence' states.

I did not include tests for these states because I am not
sure how to induce these conditions organically yet:

   conflicted am status
   empty am status
   rebase-interactive editing status
   rebase-interactive splitting status

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 t/t7061-status-sequence.sh | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100755 t/t7061-status-sequence.sh

diff --git a/t/t7061-status-sequence.sh b/t/t7061-status-sequence.sh
new file mode 100755
index 0000000..db99822
--- /dev/null
+++ b/t/t7061-status-sequence.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+test_description='sequencer state tokens'
+
+. ./test-lib.sh
+
+expect_tokens() {
+	for TOKEN in "$@" ; do
+		echo "## $TOKEN"
+	done | sort
+}
+
+status_has_only() {
+	expect_tokens "$@" >expect &&
+	git status -S | sort >actual &&
+	test_cmp expect actual
+}
+	
+test_expect_success setup '
+	test_commit A &&
+	test_commit B oneside added &&
+	git checkout A^0 &&
+	test_commit C oneside created
+'
+
+test_expect_success 'status -S reports conflicted merge' '
+	git checkout B^0 &&
+	test_must_fail git merge C &&
+	status_has_only commit-pending conflicted merge
+'
+
+test_expect_success 'git reset --hard cleans up merge status' '
+	git reset --hard HEAD &&
+	status_has_only
+'
+
+test_expect_success 'status -S reports conflicted rebase' '
+	git reset --hard HEAD &&
+	git checkout B^0 &&
+	test_must_fail git rebase C &&
+	status_has_only conflicted rebase
+'
+
+test_expect_success 'git rebase --abort cleans up rebase status' '
+	git rebase --abort &&
+	status_has_only
+'
+
+test_expect_success 'status -S reports incomplete cherry-pick' '
+	git reset --hard HEAD &&
+	git checkout A &&
+	git cherry-pick --no-commit C &&
+	status_has_only commit-pending
+'
+
+test_expect_success 'completing commit cleans up pending commit status' '
+	git commit -mcompleted &&
+	status_has_only
+'
+
+test_expect_success 'status -S reports failed cherry-pick' '
+	git reset --hard HEAD &&
+	git checkout B &&
+	test_must_fail git cherry-pick C &&
+	status_has_only cherry-pick commit-pending conflicted
+'
+
+test_expect_success 'resolved conflicts clear conflicted status' '
+	git add oneside &&
+	status_has_only cherry-pick commit-pending
+'
+
+test_expect_success 'aborted cherry-pick clears cherry-pick status' '
+	git cherry-pick --abort &&
+	status_has_only
+'
+
+test_expect_success 'conflicted rebase-interactive status' '
+	git reset --hard HEAD &&
+	git checkout B &&
+	test_must_fail git rebase -i C &&
+	status_has_only rebase-interactive conflicted commit-pending
+'
+
+test_expect_success 'bisect status' '
+	git reset --hard HEAD &&
+	git bisect start &&
+	status_has_only bisect
+'
+
+test_expect_success 'bisect-reset clears bisect status' '
+	git bisect reset &&
+	status_has_only
+'
+
+test_done
-- 
1.8.0.3.gde9c7d5.dirty

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Junio C Hamano]

Phil Hord <hordp@cisco.com> writes:

> State token strings which may be emitted and their meanings:
>     merge              a merge is in progress
>     am                 an am is in progress
>     am-is-empty        the am patch is empty
>     rebase             a rebase is in progress
>     rebase-interactive an interactive rebase is in progress
>     cherry-pick        a cherry-pick is in progress
>     bisect             a bisect is in progress
>     conflicted         there are unresolved conflicts
>     commit-pending     a commit operation is waiting to be completed
>     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.
>     changed-index  Changes exist in the index
>     changed-files  Changes exist in the working directory
>     untracked      New files exist in the working directory

I tend to agree; unlike all the normal output from "status -s" that
are per-file, the above are the overall states of the working tree.

It is just that most of the "overall states" look as if they are
dominated by "sequencer states", but that is only because you chose
to call states related to things like "am" and "bisect" that are not
sequencer states as such.

It probably should be called the tree state, working tree state, or
somesuch.

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Phil Hord]

Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> State token strings which may be emitted and their meanings:
>>     merge              a merge is in progress
>>     am                 an am is in progress
>>     am-is-empty        the am patch is empty
>>     rebase             a rebase is in progress
>>     rebase-interactive an interactive rebase is in progress
>>     cherry-pick        a cherry-pick is in progress
>>     bisect             a bisect is in progress
>>     conflicted         there are unresolved conflicts
>>     commit-pending     a commit operation is waiting to be completed
>>     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.
>>     changed-index  Changes exist in the index
>>     changed-files  Changes exist in the working directory
>>     untracked      New files exist in the working directory
> I tend to agree; unlike all the normal output from "status -s" that
> are per-file, the above are the overall states of the working tree.
>
> It is just that most of the "overall states" look as if they are
> dominated by "sequencer states", but that is only because you chose
> to call states related to things like "am" and "bisect" that are not
> sequencer states as such.
>
> It probably should be called the tree state, working tree state, or
> somesuch.

I think you are agreeing that I chose the switch name poorly, right?

Do you think '--tree-state' is an acceptable switch or do you have other
suggestions?

Phil

Re: [PATCHv3 1/4] Refactor print_state into get_state

[Ramkumar Ramachandra]

Phil Hord wrote:
> 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 useful to other callers who do
> not want it printed in the same way.

Nit: Please mention that "Recently" refers to 83c750ac to save future
readers the trouble of running blame.

Thanks.

Ram

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Phil Hord]

Phil Hord wrote:
> Junio C Hamano wrote:
>> Phil Hord <hordp@cisco.com> writes:
>>
>>> State token strings which may be emitted and their meanings:
>>>     merge              a merge is in progress
>>>     am                 an am is in progress
>>>     am-is-empty        the am patch is empty
>>>     rebase             a rebase is in progress
>>>     rebase-interactive an interactive rebase is in progress
>>>     cherry-pick        a cherry-pick is in progress
>>>     bisect             a bisect is in progress
>>>     conflicted         there are unresolved conflicts
>>>     commit-pending     a commit operation is waiting to be completed
>>>     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.
>>>     changed-index  Changes exist in the index
>>>     changed-files  Changes exist in the working directory
>>>     untracked      New files exist in the working directory
>> I tend to agree; unlike all the normal output from "status -s" that
>> are per-file, the above are the overall states of the working tree.
>>
>> It is just that most of the "overall states" look as if they are
>> dominated by "sequencer states", but that is only because you chose
>> to call states related to things like "am" and "bisect" that are not
>> sequencer states as such.
>>
>> It probably should be called the tree state, working tree state, or
>> somesuch.
> I think you are agreeing that I chose the switch name poorly, right?
>
> Do you think '--tree-state' is an acceptable switch or do you have other
> suggestions?
>

I've been calling these 'tokens' myself.  A token is a word-or-phrase I
can parse easily with the default $IFS, for simpler script handling.

I'm happy to make that official and use --tokens and -T, but I suspect a
more appropriate name is available.

Phil

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Junio C Hamano]

Phil Hord <hordp@cisco.com> writes:

>> Do you think '--tree-state' is an acceptable switch or do you have other
>> suggestions?
>
> I've been calling these 'tokens' myself.  A token is a word-or-phrase I
> can parse easily with the default $IFS, for simpler script handling.

That name may be good for variables, but it is good only because you
as the implementor know what purpose the tokens are used for.
Instead of having to call them with a longer name, e.g. "state
tokens", only because you know that these tokens represent tree-wide
(as opposed to per-file) state, you can call them "tokens" in your
implementation (and in your head) without confusing yourself.

To the end users who should not care about the implementation
detail, it is not a good name at all.  The UI should surface the
purpose, i.e. what these tokens are used for, (e.g. to represent
tree-wide state) more than the fact that you happened to represent
them with a single short word (i.e. "token").

So --show-tree-state, --include-tree-state-in-the-output or
something along that line that tells the user what the option is
about is more preferable than --token.  After all, you may want to
use tokens to represent different kind of information in a later
topic that is not about a tree-wide state, and you will regret that
you used --token for this particular feature at that time.

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Phil Hord]

Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>>> Do you think '--tree-state' is an acceptable switch or do you have other
>>> suggestions?
>> I've been calling these 'tokens' myself.  A token is a word-or-phrase I
>> can parse easily with the default $IFS, for simpler script handling.
> That name may be good for variables, but it is good only because you
> as the implementor know what purpose the tokens are used for.
> Instead of having to call them with a longer name, e.g. "state
> tokens", only because you know that these tokens represent tree-wide
> (as opposed to per-file) state, you can call them "tokens" in your
> implementation (and in your head) without confusing yourself.
>
> To the end users who should not care about the implementation
> detail, it is not a good name at all.  The UI should surface the
> purpose, i.e. what these tokens are used for, (e.g. to represent
> tree-wide state) more than the fact that you happened to represent
> them with a single short word (i.e. "token").
>
> So --show-tree-state, --include-tree-state-in-the-output or
> something along that line that tells the user what the option is
> about is more preferable than --token.  After all, you may want to
> use tokens to represent different kind of information in a later
> topic that is not about a tree-wide state, and you will regret that
> you used --token for this particular feature at that time.

I don't think I would regret it at all.  I do not expect to conflate the
word "tokens" with the meaning "show-tree-state".  It only has this
meaning because it is part 'git status'.  If I want to show a different
kind of tokens in the future, I think "--tokens" would still work fine
there.  It would even have precedent.

Consider the usage:

  git status   # show work-tree status
  git status --short  # show short work-tree status
  git status --tokens  # show work-tree status in token form

In the future if someone adds a similar operation to another command,I
do not think it would be confusing if it had a similar result. 

  git log --tokens
  git show-ref --tokens HEAD

But maybe "--tokens" has some better meaning that someone will want to
use in the future.  I'm not married to it.  But "git status" already
means "Show the working tree status".  So "git status --show-tree-state"
sounds redundant or meaningless.

'git status' already recognizes switches --ignored and --branch, for
example, to add extra state information to the output.  I want to add
"tree-state tokens" to the list.  But I don't know a better name for them.

Perhaps "--show-tree-state" is sufficient.  Still sounds redundant to me.

Phil

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Junio C Hamano]

Phil Hord <hordp@cisco.com> writes:

> Consider the usage:
>
>   git status   # show work-tree status
>   git status --short  # show short work-tree status
>   git status --tokens  # show work-tree status in token form

OK, your --tokens is more about *how* things are output, but it is
unclear how it would interact with --short.  I had an impression
that you are basing your output on the short output, whose existing
record include "##" (that shows the branch names and states), and
"MM", "A " and friends (that show the per-file states), by adding
new record types that shows tree-wide states.

> But maybe "--tokens" has some better meaning that someone will want to
> use in the future.  I'm not married to it.  But "git status" already
> means "Show the working tree status".  So "git status --show-tree-state"
> sounds redundant or meaningless.

I didn't mean to say that you have to spell out all these words;
"show" and "state" are redundant.

The important part is that unlike the existing "per-file" state the
"status" command is showing, the option is to add "tree-wide" state
to the output, and my suggestion was to pick a word that makes it
clear, rather than using "output is done using tokens" without
saying "what is being output in tokenized form".

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Phil Hord]

Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> Consider the usage:
>>
>>   git status   # show work-tree status
>>   git status --short  # show short work-tree status
>>   git status --tokens  # show work-tree status in token form
> OK, your --tokens is more about *how* things are output, but it is
> unclear how it would interact with --short.  I had an impression
> that you are basing your output on the short output, whose existing
> record include "##" (that shows the branch names and states), and
> "MM", "A " and friends (that show the per-file states), by adding
> new record types that shows tree-wide states.

I am, but I don't much care for the "##" prefix, especially when
combined with --null, for example.  I'm inclined to remove it when
--short is not provided, specifically to give scripts an easier time of
parsing.  But scripts are likely to need "--porcelain" as well, and
currently that implies "--short".  But I suppose another combination
could be meaningful.

  # tokens only
  $ git status --tree
  changed-files

  # tokens and short-status
  $ git status --tree --short  
  ## changed-files
   M foo.txt

  # short-status only
  $ git status --porcelain
   M foo.txt

  # tokens only?
  $ git status --tree --porcelain
  changed-files

I think this spaghettify's the ui too much.  Maybe this instead:

  # undecorated tokens only
  $ git status --tree=porcelain
  changed-files


>
>> But maybe "--tokens" has some better meaning that someone will want to
>> use in the future.  I'm not married to it.  But "git status" already
>> means "Show the working tree status".  So "git status --show-tree-state"
>> sounds redundant or meaningless.
> I didn't mean to say that you have to spell out all these words;
> "show" and "state" are redundant.
>
> The important part is that unlike the existing "per-file" state the
> "status" command is showing, the option is to add "tree-wide" state
> to the output, and my suggestion was to pick a word that makes it
> clear, rather than using "output is done using tokens" without
> saying "what is being output in tokenized form".

Thanks for clarifying. 

Phil

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Junio C Hamano]

Phil Hord <hordp@cisco.com> writes:

>   # tokens and short-status
>   $ git status --tree --short  
>   ## changed-files
>    M foo.txt

Hrm, how will the existing readers of the output avoid getting
confused by this overloading of "##", which has meant the current
branch information?

>   # tokens only?
>   $ git status --tree --porcelain
>   changed-files

It probably is simpler for parsers of the --porcelain format if you
(1) always used first two bytes as the record type, and (2) avoided
using "##" that is already used for the tree-wide status.

Just a thought.

Re: [PATCHv3 3/4] git-status: show short sequencer state

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Phil Hord <hordp@cisco.com> writes:
>
>>   # tokens and short-status
>>   $ git status --tree --short  
>>   ## changed-files
>>    M foo.txt
>
> Hrm, how will the existing readers of the output avoid getting
> confused by this overloading of "##", which has meant the current
> branch information?

That was a stupid question.

Existing readers do not know to give --tree to the command, so they
are perfectly OK.

It is however not OK for new readers that wants to read these
tree-wide state and also branch information.