threads / discuss / 55150

Short status ignores --show-stash option

Subject: Short status ignores --show-stash option

## tl;dr

3 messages between Feb 15, 2021 and Feb 16, 2021.

replies: 2people: 3as markdown or json

janek· Feb 15, 2021, 20:02 UTC · lore

What did you do before the bug happened? (Steps to reproduce your issue) git status --short --show-stash --branch

What did you expect to happen? (Expected behavior) The status shows info about the stash, e.g. next to the branch infos

What happened instead? (Actual behavior) --show-stash is ignored when using short format

[System Info] git version: git version 2.30.1 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 5.10.15-arch1-1 #1 SMP PREEMPT Wed, 10 Feb 2021 18:32:40 +0000 x86_64 compiler info: gnuc: 10.2 libc info: glibc: 2.33 $SHELL (typically, interactive shell): /bin/zsh

[Enabled Hooks] post-commit post-checkout post-merge pre-push

Regards, Janek

Taylor Blau· Feb 16, 2021, 18:25 UTC · re: janek · lore

Re: Short status ignores --show-stash option

On Mon, Feb 15, 2021 at 08:02:24PM +0000, janek wrote:
Show 8 quoted lines
> What did you do before the bug happened? (Steps to reproduce your issue)
> git status --short --show-stash --branch
>
> What did you expect to happen? (Expected behavior)
> The status shows info about the stash, e.g. next to the branch infos
>
> What happened instead? (Actual behavior)
> --show-stash is ignored when using short format
Hmm. It's certainly possible to do something like:
diff --git a/wt-status.c b/wt-status.c
index 0c8287a023..397d36544d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2013,6 +2013,9 @@ static void wt_shortstatus_print(struct wt_status *s)

        for_each_string_list_item(it, &s->ignored)
                wt_shortstatus_other(it, s, "!!");
+
+       if (s->show_stash)
+               wt_longstatus_print_stash_summary(s);
 }

 static void wt_porcelain_print(struct wt_status *s)

and that would cause 'git status' to do what you expect:

    $ git.compile status --short --branch --show-stash
    ## tb/empty-trailer-continuation
     M wt-status.c
     Your stash currently has 16 entries

But it may not be the right thing to do, since that explicitly breaks
the --porcelain format. We may want something like this in addition to
the above:

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7..ef855896a2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1414,6 +1414,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
            s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
                die(_("Unsupported combination of ignored and untracked-files arguments"));

+       if (status_format == STATUS_FORMAT_PORCELAIN ||
+           status_format == STATUS_FORMAT_PORCELAIN_V2) {
+               if (s.show_stash)
+                       die(_("--porcelain is incompatible with --show-stash"));
+       }
+
        parse_pathspec(&s.pathspec, 0,
                       PATHSPEC_PREFER_FULL,
                       prefix, argv);

Thanks,
Taylor
Jeff King· Feb 16, 2021, 20:23 UTC · re: Taylor Blau · lore

Re: Short status ignores --show-stash option

On Tue, Feb 16, 2021 at 01:25:32PM -0500, Taylor Blau wrote:
Show 25 quoted lines
> and that would cause 'git status' to do what you expect:
> 
>     $ git.compile status --short --branch --show-stash
>     ## tb/empty-trailer-continuation
>      M wt-status.c
>      Your stash currently has 16 entries
> 
> But it may not be the right thing to do, since that explicitly breaks
> the --porcelain format. We may want something like this in addition to
> the above:
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 739110c5a7..ef855896a2 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1414,6 +1414,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>             s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
>                 die(_("Unsupported combination of ignored and untracked-files arguments"));
> 
> +       if (status_format == STATUS_FORMAT_PORCELAIN ||
> +           status_format == STATUS_FORMAT_PORCELAIN_V2) {
> +               if (s.show_stash)
> +                       die(_("--porcelain is incompatible with --show-stash"));
> +       }
> +

I think ideally we'd define a porcelain format for it, since v2 has explicit support for extending with new headers. Something like:

  # stash.number 16

would probably work. I don't think it's _strictly_ necessary, in that the die() above does not paint us into a corner. We can always loosen it later. But it is easy enough that it probably makes sense to just define it while we are here.

And in particular, I think we may want to mimic it for --short, rather than just showing the "long" output, which IMHO is kind of dissonant in terms of the formatting (though again, we reserve the right to change --short, so I don't think we've painted ourselves into a corner).

-Peff

← back to recent threads