[PATCH 0/2] Print stash info in the v2 porcelain format

[PATCH 0/2] Print stash info in the v2 porcelain format

[Øystein Walle]

When --porcelain=v2 is given --show-stash is silently ignored. But the
format is useful for quick gathering of repo status, for e.g. prompts.
And indeed git's own git-prompt.sh supports printing a sigil when stash
entries exist.

The first patch is just a very small refactoring. It's far from
necessary (although as it stands the second patch depends on it) but it
makes sense to me. The second patch is the one that counts.

Øystein Walle (2):
  status: count stash entries in separate function
  status: print stash info with --porcelain=v2 --show-stash

 Documentation/git-status.txt |  8 ++++++++
 t/t7064-wtstatus-pv2.sh      | 15 +++++++++++++++
 wt-status.c                  | 25 +++++++++++++++++++++++--
 3 files changed, 46 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH 1/2] status: count stash entries in separate function

[Øystein Walle]

Make the counting of stash entries contained in one simple function as
it will be used in the next commit.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---
 wt-status.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index e4f29b2b4c..97230477b2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -948,11 +948,17 @@ static int stash_count_refs(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
+static int count_stash_entries()
+{
+	int n = 0;
+	for_each_reflog_ent("refs/stash", stash_count_refs, &n);
+	return n;
+}
+
 static void wt_longstatus_print_stash_summary(struct wt_status *s)
 {
-	int stash_count = 0;
+	int stash_count = count_stash_entries();
 
-	for_each_reflog_ent("refs/stash", stash_count_refs, &stash_count);
 	if (stash_count > 0)
 		status_printf_ln(s, GIT_COLOR_NORMAL,
 				 Q_("Your stash currently has %d entry",
-- 
2.27.0

[PATCH 2/2] status: print stash info with --porcelain=v2 --show-stash

[Øystein Walle]

The v2 porcelain format is very convenient for obtaining a lot of
information about the current state of the repo, but does not contain
any info about the stash. git status already accepts --show-stash but
it's silently ignored when --porcelain=v2 is given.

Let's add a simple line to print the number of stash entries but in a
format similar in style to the rest of the format.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---
 Documentation/git-status.txt |  8 ++++++++
 t/t7064-wtstatus-pv2.sh      | 15 +++++++++++++++
 wt-status.c                  | 15 +++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 4a2c3e0408..54a4b29b47 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -314,6 +314,14 @@ Line                                     Notes
 ------------------------------------------------------------
 ....
 
+Stash Information
+^^^^^^^^^^^^^^^^^
+
+If `--show-stash` is given, one line is printed showing the number of stash
+entries if non-zero:
+
+    # stash <N>
+
 Changed Tracked Entries
 ^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index eeb0534163..47fc21d962 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -113,6 +113,21 @@ test_expect_success 'after first commit, create unstaged changes' '
 	test_cmp expect actual
 '
 
+test_expect_success 'after first commit, stash existing changes' '
+	cat >expect <<-EOF &&
+	# branch.oid $H0
+	# branch.head initial-branch
+	# stash 2
+	EOF
+
+	test_when_finished "git stash pop && git stash pop" &&
+
+	git stash -- file_x &&
+	git stash &&
+	git status --porcelain=v2 --branch --show-stash --untracked-files=no >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'after first commit but omit untracked files and branch' '
 	cat >expect <<-EOF &&
 	1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
diff --git a/wt-status.c b/wt-status.c
index 97230477b2..88c5481e24 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2182,6 +2182,18 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)
 	}
 }
 
+/*
+ * Print the stash count in a porcelain-friendly format
+ */
+static void wt_porcelain_v2_print_stash(struct wt_status *s)
+{
+	int stash_count = count_stash_entries();
+	char eol = s->null_termination ? '\0' : '\n';
+
+	if (stash_count > 0)
+		fprintf(s->fp, "# stash %d%c", stash_count, eol);
+}
+
 /*
  * Convert various submodule status values into a
  * fixed-length string of characters in the buffer provided.
@@ -2443,6 +2455,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)
 	if (s->show_branch)
 		wt_porcelain_v2_print_tracking(s);
 
+	if (s->show_stash)
+		wt_porcelain_v2_print_stash(s);
+
 	for (i = 0; i < s->change.nr; i++) {
 		it = &(s->change.items[i]);
 		d = it->util;
-- 
2.27.0

Re: [PATCH 1/2] status: count stash entries in separate function

[Junio C Hamano]

Øystein Walle <oystwa@gmail.com> writes:

> Make the counting of stash entries contained in one simple function as
> it will be used in the next commit.
>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
>  wt-status.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index e4f29b2b4c..97230477b2 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -948,11 +948,17 @@ static int stash_count_refs(struct object_id *ooid, struct object_id *noid,
>  	return 0;
>  }
>  
> +static int count_stash_entries()

Probably "static int count_stash_entries(void)" was what was meant
here.

> +{
> +	int n = 0;

Have a blank line between the block of decls at the beginning of the
function and the first statement.

> +	for_each_reflog_ent("refs/stash", stash_count_refs, &n);
> +	return n;
> +}

I briefly wondered if this want to be size_t or some other unsigned
integral type, but this is merely refactoring the existing code, so
it is not just OK but is correct to use the same "int" as before.

>  static void wt_longstatus_print_stash_summary(struct wt_status *s)
>  {
> -	int stash_count = 0;
> +	int stash_count = count_stash_entries();
>  
> -	for_each_reflog_ent("refs/stash", stash_count_refs, &stash_count);
>  	if (stash_count > 0)
>  		status_printf_ln(s, GIT_COLOR_NORMAL,
>  				 Q_("Your stash currently has %d entry",

OK.

Re: [PATCH 2/2] status: print stash info with --porcelain=v2 --show-stash

[Junio C Hamano]

Øystein Walle <oystwa@gmail.com> writes:

> The v2 porcelain format is very convenient for obtaining a lot of
> information about the current state of the repo, but does not contain
> any info about the stash. git status already accepts --show-stash but
> it's silently ignored when --porcelain=v2 is given.
>
> Let's add a simple line to print the number of stash entries but in a
> format similar in style to the rest of the format.

Who is the primary target audience of this feature?  IDEs that read
from "git status --porcelain"?

Whoever it is, if they bothered enough to pass an extra option
(i.e. "--show-stash") to their "git status" invocation, I wonder if
they want to know at least what "git stash list" would give you at
the same time.  After all, the --porcelain output is all about
machine readability and reducing the number of forks would be what
those people would be interested in, wouldn't it?  I dunno.

Thanks.

Re: [PATCH 2/2] status: print stash info with --porcelain=v2 --show-stash

[Eric Sunshine]

On Thu, Oct 21, 2021 at 6:25 PM Øystein Walle <oystwa@gmail.com> wrote:
> The v2 porcelain format is very convenient for obtaining a lot of
> information about the current state of the repo, but does not contain
> any info about the stash. git status already accepts --show-stash but
> it's silently ignored when --porcelain=v2 is given.
>
> Let's add a simple line to print the number of stash entries but in a
> format similar in style to the rest of the format.
>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
> diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
> @@ -113,6 +113,21 @@ test_expect_success 'after first commit, create unstaged changes' '
> +test_expect_success 'after first commit, stash existing changes' '
> +       cat >expect <<-EOF &&
> +       # branch.oid $H0
> +       # branch.head initial-branch
> +       # stash 2
> +       EOF
> +
> +       test_when_finished "git stash pop && git stash pop" &&

If it's indeed important to clean up the stashes when the test
finishes, then the test_when_finished() invocation should probably be
a bit more robust...

> +       git stash -- file_x &&
> +       git stash &&

... since, as it is now, if an error occurs between these two
git-stash invocations or before them, then there will only be zero or
one stashes, so the double stash-pop by test_when_finished() will
itself errout out. Better, perhaps to do this:

    test_when_finished "git stash pop && git stash pop || :" &&

> +       git status --porcelain=v2 --branch --show-stash --untracked-files=no >actual &&
> +       test_cmp expect actual
> +'

Re: [PATCH 2/2] status: print stash info with --porcelain=v2 --show-stash

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       test_when_finished "git stash pop && git stash pop" &&
>
> If it's indeed important to clean up the stashes when the test
> finishes, then the test_when_finished() invocation should probably be
> a bit more robust...
>
>> +       git stash -- file_x &&
>> +       git stash &&
>
> ... since, as it is now, if an error occurs between these two
> git-stash invocations or before them, then there will only be zero or
> one stashes, so the double stash-pop by test_when_finished() will
> itself errout out. Better, perhaps to do this:
>
>     test_when_finished "git stash pop && git stash pop || :" &&
>
>> +       git status --porcelain=v2 --branch --show-stash --untracked-files=no >actual &&
>> +       test_cmp expect actual
>> +'

There is no "I do not care what is in the stash right now, just
clear all"?

Re: [PATCH 2/2] status: print stash info with --porcelain=v2 --show-stash

[Eric Sunshine]

On Thu, Oct 21, 2021 at 8:30 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> >> +       test_when_finished "git stash pop && git stash pop" &&
> >
> > If it's indeed important to clean up the stashes when the test
> > finishes, then the test_when_finished() invocation should probably be
> > a bit more robust...
> >
> >> +       git stash -- file_x &&
> >> +       git stash &&
> >
> > ... since, as it is now, if an error occurs between these two
> > git-stash invocations or before them, then there will only be zero or
> > one stashes, so the double stash-pop by test_when_finished() will
> > itself errout out. Better, perhaps to do this:
> >
> >     test_when_finished "git stash pop && git stash pop || :" &&
>
> There is no "I do not care what is in the stash right now, just
> clear all"?

Indeed, I wondered that too but was too lazy to look and I had
forgotten about `git stash clear`, which makes this much simpler:

    test_when_finished "git stash clear" &&