[RFC PATCH 0/1] status: Allow for short-form output by default

[RFC PATCH 0/1] status: Allow for short-form output by default

[Thomas Adam]

Hi,

It was asked recently whether git status could output the short-form instead
of the long output (via its "-sb" options).  To that end, I've created a
rough POC on how this might look.  It's deliberately lacking documentation;
I was curious to know whether:

status.shortwithbranch = true

Was going to cut it.  Likewise, I was unsure if instead there should  be two
separate options, one for just short (i.e. '-s') also?

What do others think?

ISTR reading on this list recently the addition of a "--long" option to git
status?  I'm wondering how this would relate to that, if at all?

Kindly,

Thomas Adam (1):
  status: Allow for short-form via config option

 builtin/commit.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
1.7.11.4

[RFC PATCH 1/1] status: Allow for short-form via config option

[Thomas Adam]

It is currently not possible to use the short-form output of git status
without declaring an alias to do so.

This isn't always desirable therfore, define a git config option which can
be set to display the short-form:  status.shortwithbranch

Signed-off-by: Thomas Adam <thomas@xteddy.org>
---
 builtin/commit.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..552a9f1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1142,6 +1142,18 @@ static int git_status_config(const char *k, const char *v, void *cb)
 			return error(_("Invalid untracked files mode '%s'"), v);
 		return 0;
 	}
+
+	if (!strcmp(k, "status.shortwithbranch")) {
+		if (git_config_bool(k, v)) {
+			if (!v)
+				return config_error_nonbool(k);
+			else if(!strcmp(v, "true")) {
+				status_format = STATUS_FORMAT_SHORT;
+				s->show_branch = 1;
+			}
+			return 0;
+		}
+	}
 	return git_diff_ui_config(k, v, NULL);
 }
 
-- 
1.7.11.4

Re: [RFC PATCH 1/1] status: Allow for short-form via config option

[Jeff King]

On Sun, Nov 11, 2012 at 10:53:03PM +0000, Thomas Adam wrote:

> It is currently not possible to use the short-form output of git status
> without declaring an alias to do so.
> 
> This isn't always desirable therfore, define a git config option which can
> be set to display the short-form:  status.shortwithbranch

We usually try to avoid booleans for selection among options, even if
there is currently only one useful option. That way, it makes it easier
to extend later when another option presents itself.  So having
"status.format" that you could set to "short", "long", or "porcelain"
would make more sense (although "short" is likely to be the only useful
one currently).

And then we can have a separate boolean like "status.branch" to show the
branch by default when showing short format. That would would naturally
extend to more booleans as other options are added (Phil Hord's recent
"show tree state" patches come to mind).

We also need to consider the impact of options on scripts. I think
status.format should be OK, since any script calling "git status" would
have to explicitly pass "--porcelain" already, which would override this
option. But we would want to make sure that "status.branch" would not
affect the porcelain.

> ---
>  builtin/commit.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

It would need documentation, and tests would be good both to make sure
the feature works, but also to demonstrate that it does not break
--porcelain.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index a17a5df..552a9f1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1142,6 +1142,18 @@ static int git_status_config(const char *k, const char *v, void *cb)
>  			return error(_("Invalid untracked files mode '%s'"), v);
>  		return 0;
>  	}
> +
> +	if (!strcmp(k, "status.shortwithbranch")) {
> +		if (git_config_bool(k, v)) {
> +			if (!v)
> +				return config_error_nonbool(k);
> +			else if(!strcmp(v, "true")) {
> +				status_format = STATUS_FORMAT_SHORT;
> +				s->show_branch = 1;
> +			}
> +			return 0;
> +		}
> +	}

I'm not sure what is going on with the extra nonbool and "true" check.
Shouldn't it just be:

  if (git_config_bool(k, v)) {
          status_format = STATUS_FORMAT_SHORT;
          s->show_branch = 1;
  }
  else {
          /* what do we do when it is false? */
  }
  return 0;

If we follow my suggestions above, then it would be more like:

  if (!strcmp(k, "status.format")) {
          if (!v)
                  return config_error_nonbool(k);
          return parse_status_format(v, &status_format);
  }
  if (!strcmp(k, "status.branch")) {
          s->show_branch = git_config_bool(k, v);
          return 0;
  }

but that would still have to resolve the setting of show_branch when
--porcelain is in use. I think you would need to store the
config-derived value separately, and then only fill it into
s->show_branch if no value was given on the command-line _and_ we are
not showing the porcelain format.

-Peff