threads / rfc / 45391

RFC patchshort status: improve reporting for submodule changes

Subject: [RFC PATCH] short status: improve reporting for submodule changes

## tl;dr

7 messages between Mar 16, 2017 and Mar 16, 2017. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Stefan Beller· Mar 16, 2017, 00:33 UTC · lore

While we already have a porcelain2 layer for git-status, that is accurate for submodules, users still like the way they are are used to of 'status -s'.

As a submodule has more state than a file potentially, we'll look at all cases:

   ------ new submodule commits
 /  ----- modified files
 | /   -- untracked files
 | |  /
 | | |   current / proposed reporting
 0 0 0     "  "     "  "
 0 0 1     " M"     " ?"
 0 1 0     " M"     " m"
 0 1 1     " M"     " m"
 1 0 0     " M"     " M"
 1 0 1     " M"     " M"
 1 1 0     " M"     " M"
 1 1 1     " M"     " M"

So when there is no new commits in the submodule, we'd want to tell what actually happend to the submodule. The lower case 'm' indicates to the user that there is need to give a --recurse-submodules option for e.g. adding or committing these changes. The " ?" also differentiates an untracked file in the submodule from a regular untracked file.

While making these changes, we need to make sure to not break porcelain level 1, which uses the same code as the short printing function.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-status.txt |  8 ++++++++
 wt-status.c                  | 15 +++++++++++++--
 wt-status.h                  |  1 +
 3 files changed, 22 insertions(+), 2 deletions(-)
Show changes to 3 files +22 −2

Documentation/git-status.txt, wt-status.c, wt-status.h

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ba873657cf..26c8d832cd 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -181,6 +181,12 @@ in which case `XY` are `!!`.
     !           !    ignored
     -------------------------------------------------
 
+Submodules have more state to it, such that it reports instead:
+		M    the submodule has new commits
+		m    the submodule has modified content
+		?    the submodule has untracked files
+
+
 If -b is used the short-format status is preceded by a line
 
     ## branchname tracking info
@@ -210,6 +216,8 @@ field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Any submodule changes are reported as modified `M` instead of `m` or single `?`.
+
 Porcelain Format Version 2
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/wt-status.c b/wt-status.c
index a52d342695..080d97f272 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1664,9 +1664,19 @@ static void wt_shortstatus_status(struct string_list_item *it,
 		color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
 	else
 		putchar(' ');
-	if (d->worktree_status)
+	if (d->worktree_status) {
+		if (!s->submodule_porcelain1 &&
+		    (d->dirty_submodule || d->new_submodule_commits)) {
+			/* It is a submodule, and we're not in plumbing mode. */
+			if (d->new_submodule_commits)
+				d->worktree_status = 'M';
+			else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
+				d->worktree_status = 'm';
+			else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+				d->worktree_status = '?';
+		}
 		color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c", d->worktree_status);
-	else
+	} else
 		putchar(' ');
 	putchar(' ');
 	if (s->null_termination) {
@@ -1811,6 +1821,7 @@ static void wt_porcelain_print(struct wt_status *s)
 	s->relative_paths = 0;
 	s->prefix = NULL;
 	s->no_gettext = 1;
+	s->submodule_porcelain1 = 1;
 	wt_shortstatus_print(s);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 54fec77032..620e4d2ae4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -70,6 +70,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
+	int submodule_porcelain1;
 	int show_ignored_files;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
-- 
2.12.0.269.g1a05a5734c.dirty
Junio C Hamano· Mar 16, 2017, 01:31 UTC · re: Stefan Beller · lore

Re: [RFC PATCH] short status: improve reporting for submodule changes

Stefan Beller <sbeller@google.com> writes:
Show 20 quoted lines
> While we already have a porcelain2 layer for git-status, that is accurate
> for submodules, users still like the way they are are used to of
> 'status -s'.
>
> As a submodule has more state than a file potentially, we'll look at all
> cases:
>
>    ------ new submodule commits
>  /  ----- modified files
>  | /   -- untracked files
>  | |  /
>  | | |   current / proposed reporting
>  0 0 0     "  "     "  "
>  0 0 1     " M"     " ?"
>  0 1 0     " M"     " m"
>  0 1 1     " M"     " m"
>  1 0 0     " M"     " M"
>  1 0 1     " M"     " M"
>  1 1 0     " M"     " M"
>  1 1 1     " M"     " M"

You are essentialy saying that there are three levels, 1. with commit level difference, 2. the same commit with local mods, 3. no mods but with crufts, and instead of wasting 8 letters to express all combinations, the highest level is reported, right? That sounds OK to me. I am not sure if "?" is a good letter to use (doesn't it usually mean it is an untracked cruft?), though.

Does the commit level difference really mean "has new commits"? It probably is not new problem but an old mistake inherited from the current code, but I suspect that you're just comparing the commit bound in the index of the superproject and the HEAD commit in the submodule, in which case "newness" does not matter a bit---"is it the same or different?" is the question you are asking, I would think.

Stefan Beller· Mar 16, 2017, 05:16 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] short status: improve reporting for submodule changes

On Wed, Mar 15, 2017 at 6:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 29 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> While we already have a porcelain2 layer for git-status, that is accurate
>> for submodules, users still like the way they are are used to of
>> 'status -s'.
>>
>> As a submodule has more state than a file potentially, we'll look at all
>> cases:
>>
>>    ------ new submodule commits
>>  /  ----- modified files
>>  | /   -- untracked files
>>  | |  /
>>  | | |   current / proposed reporting
>>  0 0 0     "  "     "  "
>>  0 0 1     " M"     " ?"
>>  0 1 0     " M"     " m"
>>  0 1 1     " M"     " m"
>>  1 0 0     " M"     " M"
>>  1 0 1     " M"     " M"
>>  1 1 0     " M"     " M"
>>  1 1 1     " M"     " M"
>
> You are essentialy saying that there are three levels, 1. with
> commit level difference, 2. the same commit with local mods, 3. no
> mods but with crufts, and instead of wasting 8 letters to express
> all combinations, the highest level is reported, right?  That sounds
> OK to me.  I am not sure if "?" is a good letter to use (doesn't it
> usually mean it is an untracked cruft?), though.

ok. it helped me, though, to picture all possibilities to come up with what I consider best for each case. Yes in the end it can be described as 'report highest bit'.

Show 8 quoted lines
>
> Does the commit level difference really mean "has new commits"?  It
> probably is not new problem but an old mistake inherited from the
> current code, but I suspect that you're just comparing the commit
> bound in the index of the superproject and the HEAD commit in the
> submodule, in which case "newness" does not matter a bit---"is it
> the same or different?" is the question you are asking, I would
> think.
yes, I agree. That is the actual question asked.
>
>
Stefan Beller· Mar 16, 2017, 20:46 UTC · re: Stefan Beller · lore

[PATCH v2] short status: improve reporting for submodule changes

While we already have a porcelain2 layer for git-status, that is accurate for submodules, users still like the way they are are used to of 'status -s'.

As a submodule has more state than a file potentially, we need more letters indicating these states, we'll go with lower 'm' and single '?' for now.

When there the recorded commit doesn't differ from the submodule HEAD (which we still want to treat as "M", because that can be dealt with in the superproject), we can have different types of change in the submodule. The lower case 'm' indicates that there are changes to tracked files in the submodule. It signals that the --recurse-submodules option is needed for e.g. adding or committing these changes. The " ?" also differentiates an untracked file in the submodule from a regular untracked file.

While making these changes, we need to make sure to not break porcelain level 1, which uses the same code as the short printing function.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-status.txt |  8 ++++++++
 t/t7506-status-submodule.sh  | 24 ++++++++++++++++++++++++
 wt-status.c                  | 15 +++++++++++++--
 wt-status.h                  |  1 +
 4 files changed, 46 insertions(+), 2 deletions(-)
Show changes to 4 files +46 −2

Documentation/git-status.txt, t/t7506-status-submodule.sh, wt-status.c, wt-status.h

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ba873657cf..b182b16c48 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -181,6 +181,12 @@ in which case `XY` are `!!`.
     !           !    ignored
     -------------------------------------------------
 
+Submodules have more state to it, such that it reports instead:
+		M    the submodule has a different HEAD than recorded
+		m    the submodule has modified content
+		?    the submodule has untracked files
+
+
 If -b is used the short-format status is preceded by a line
 
     ## branchname tracking info
@@ -210,6 +216,8 @@ field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Any submodule changes are reported as modified `M` instead of `m` or single `?`.
+
 Porcelain Format Version 2
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index d31b34da83..98dff01947 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -50,6 +50,15 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with modified file in submodule (short)' '
+	(cd sub && git reset --hard) &&
+	echo "changed" >sub/foo &&
+	git status --short >output &&
+	diff output - <<-\EOF
+	 m sub
+	EOF
+'
+
 test_expect_success 'status with added file in submodule' '
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	git status >output &&
@@ -64,6 +73,14 @@ test_expect_success 'status with added file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with added file in submodule (short)' '
+	(cd sub && git reset --hard && echo >foo && git add foo) &&
+	git status --short >output &&
+	diff output - <<-\EOF
+	 m sub
+	EOF
+'
+
 test_expect_success 'status with untracked file in submodule' '
 	(cd sub && git reset --hard) &&
 	echo "content" >sub/new-file &&
@@ -83,6 +100,13 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with untracked file in submodule (short)' '
+	git status --short >output &&
+	diff output - <<-\EOF
+	 ? sub
+	EOF
+'
+
 test_expect_success 'status with added and untracked file in submodule' '
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	echo "content" >sub/new-file &&
diff --git a/wt-status.c b/wt-status.c
index a52d342695..080d97f272 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1664,9 +1664,19 @@ static void wt_shortstatus_status(struct string_list_item *it,
 		color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
 	else
 		putchar(' ');
-	if (d->worktree_status)
+	if (d->worktree_status) {
+		if (!s->submodule_porcelain1 &&
+		    (d->dirty_submodule || d->new_submodule_commits)) {
+			/* It is a submodule, and we're not in plumbing mode. */
+			if (d->new_submodule_commits)
+				d->worktree_status = 'M';
+			else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
+				d->worktree_status = 'm';
+			else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+				d->worktree_status = '?';
+		}
 		color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c", d->worktree_status);
-	else
+	} else
 		putchar(' ');
 	putchar(' ');
 	if (s->null_termination) {
@@ -1811,6 +1821,7 @@ static void wt_porcelain_print(struct wt_status *s)
 	s->relative_paths = 0;
 	s->prefix = NULL;
 	s->no_gettext = 1;
+	s->submodule_porcelain1 = 1;
 	wt_shortstatus_print(s);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 54fec77032..620e4d2ae4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -70,6 +70,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
+	int submodule_porcelain1;
 	int show_ignored_files;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
-- 
2.12.0.269.g1a05a5734c.dirty
Jonathan Nieder· Mar 16, 2017, 22:19 UTC · re: Stefan Beller · lore

Re: [PATCH v2] short status: improve reporting for submodule changes

Hi,

Yay, I like the change this makes. So I'll nitpick in the hope that that makes the patch more likely to stick.

Stefan Beller wrote:
> While we already have a porcelain2 layer for git-status, that is accurate
> for submodules, users still like the way they are are used to of
> 'status -s'.

I had to read this a few times to understand it. Maybe explaining it from the user's point of view would work:

	If I add an untracked file to a submodule or modify a tracked file,
	currently "git status --short" treats the change in the same way as
	changes to the current HEAD of the submodule:
		$ git clone --quiet --recurse-submodules https://gerrit.googlesource.com/gerrit
		$ echo hello >gerrit/plugins/replication/stray-file
		$ sed -i -e 's/.*//' gerrit/plugins/replication/.mailmap
		$ git -C gerrit status --short
		 M plugins/replication
	This is by analogy with ordinary files, where "M" represents a change
	that has not been added yet to the index.  But this change cannot be
	added to the index without entering the submodule, "git add"-ing it,
	and running "git commit", so the analogy is counterproductive.
Show 11 quoted lines
> As a submodule has more state than a file potentially, we need more letters
> indicating these states, we'll go with lower 'm' and single '?' for now.
>
> When there the recorded commit doesn't differ from the submodule HEAD
> (which we still want to treat as "M", because that can be dealt with
> in the superproject), we can have different types of change in the
> submodule. The lower case 'm' indicates that there are changes to tracked
> files in the submodule. It signals that the --recurse-submodules option
> is needed for e.g. adding or committing these changes. The " ?" also
> differentiates an untracked file in the submodule from a regular
> untracked file.
This could say
	Introduce new status letters " ?" and " m" for this.  These are similar
	to the existing "??" and " M" but mean that the submodule (not the
	parent project) has new untracked files and modified files, respectively.
	The user can use "git add" and "git commit" from within the submodule to
	add them.
	Changes to the submodule's HEAD commit can be recorded in the index with
	a plain "git add -u" and are shown with " M", like today.
	To avoid excessive clutter, show at most one of " ?", " m", and " M" for
	the submodule.  They represent increasing levels of change --- the last
	one that applies is shown (e.g., " m" if there are both modified files
	and untracked files in the submodule, or " M" if the submodule's HEAD
	has been modified and it has untracked files).
> While making these changes, we need to make sure to not break porcelain
> level 1, which uses the same code as the short printing function.
	While making these changes, we need to make sure to not break porcelain
	level 1, which shares code with "status --short".  We only change
	"git status --short".
	Non-short "git status" and "git status --porcelain=2" already handle
	these cases by showing more detail:
		$ git -C gerrit status --porcelain=2
		1 .M S.MU 160000 160000 160000 305c864db28eb0c77c8499bc04c87de3f849cf3c 305c864db28eb0c77c8499bc04c87de3f849cf3c plugins/replication
		$ git -C gerrit status
	[...]
	        modified:   plugins/replication (modified content, untracked content)
	
	Scripts caring about these distinctions should use --porcelain=2.
> Signed-off-by: Stefan Beller <sbeller@google.com>
Having written that, a few questions:

Is it important to avoid clutter by showing the submodule only once? What would you think of showing whatever subset of those three statuses apply to a given submodule as separate lines instead, to match the information that long-form "git status" shows?

How should a new untracked file in a submodule of a submodule be shown?

[...]
Show 7 quoted lines
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -181,6 +181,12 @@ in which case `XY` are `!!`.
>      !           !    ignored
>      -------------------------------------------------
>  
> +Submodules have more state to it, such that it reports instead:
Language nits: s/ to it//; s/, such that it reports instead/ and instead report/
> +		M    the submodule has a different HEAD than recorded
than recorded in the index?
> +		m    the submodule has modified content
> +		?    the submodule has untracked files
[...]
> --- a/t/t7506-status-submodule.sh
> +++ b/t/t7506-status-submodule.sh
\o/
Show 6 quoted lines
> @@ -50,6 +50,15 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
>  	EOF
>  '
>  
> +test_expect_success 'status with modified file in submodule (short)' '
> +	(cd sub && git reset --hard) &&

not about this patch: this could use "git -C sub" and avoid a subshell. But what you have here matches the existing style, so I wouldn't suggest changing it here.

> +	echo "changed" >sub/foo &&
> +	git status --short >output &&
> +	diff output - <<-\EOF

similarly, this could use test_cmp, but that seems out of scope for this change.

[...]
> +test_expect_success 'status with added file in submodule (short)' '
[...]
> +test_expect_success 'status with untracked file in submodule (short)' '
Test wishlist:
* --porcelain output for these cases
* behavior with nested submodules
[...]
Show 9 quoted lines
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1664,9 +1664,19 @@ static void wt_shortstatus_status(struct string_list_item *it,
>  		color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
>  	else
>  		putchar(' ');
> -	if (d->worktree_status)
> +	if (d->worktree_status) {
> +		if (!s->submodule_porcelain1 &&
Isn't this 's->status_format != STATUS_FORMAT_PORCELAIN'?
[...]
Show 9 quoted lines
> +		    (d->dirty_submodule || d->new_submodule_commits)) {
> +			/* It is a submodule, and we're not in plumbing mode. */
> +			if (d->new_submodule_commits)
> +				d->worktree_status = 'M';
> +			else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
> +				d->worktree_status = 'm';
> +			else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
> +				d->worktree_status = '?';
> +		}

Is this the right place to do this? Could wt_status_collect_changed_cb set worktree_status to the right value in the first place based on whether 'status_format == STATUS_FORMAT_SHORT' or would that have undesirable knock-on effects elsewhere?

[...]
Show 7 quoted lines
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -70,6 +70,7 @@ struct wt_status {
>  	int display_comment_prefix;
>  	int relative_paths;
>  	int submodule_summary;
> +	int submodule_porcelain1;

As described above, this shouldn't be needed since status_format already has this information.

Thanks for this --- I like it.

Sincerely, Jonathan

Stefan Beller· Mar 16, 2017, 22:42 UTC · re: Jonathan Nieder · lore

Re: [PATCH v2] short status: improve reporting for submodule changes

Show 5 quoted lines
>
> Is it important to avoid clutter by showing the submodule only once?
> What would you think of showing whatever subset of those three
> statuses apply to a given submodule as separate lines instead, to
> match the information that long-form "git status" shows?
I considered it, but it would break the visual appeal of git status --short ?
>
> How should a new untracked file in a submodule of a submodule be
> shown?

The same way " ?" indicates that (1) there is an untracked file due to the question mark and (2) that you need to recurse because it differs from "??" for regular untracked files.

The problem here is that we do not know about these nested untracked files, because we use --porcelain instead of --short for submodules in submodule.c#is_submodule_modified(). I am rewriting that function anyway for the "git-describe --dirty" bug, so maybe it's time to switch to porcelain=2 internally there, which can surface untracked files in nested subs.

Show 15 quoted lines
>
> [...]
>> --- a/Documentation/git-status.txt
>> +++ b/Documentation/git-status.txt
>> @@ -181,6 +181,12 @@ in which case `XY` are `!!`.
>>      !           !    ignored
>>      -------------------------------------------------
>>
>> +Submodules have more state to it, such that it reports instead:
>
> Language nits: s/ to it//; s/, such that it reports instead/ and instead report/
>
>> +             M    the submodule has a different HEAD than recorded
>
> than recorded in the index?
ok, will fix.
Show 13 quoted lines
>> +test_expect_success 'status with modified file in submodule (short)' '
>> +     (cd sub && git reset --hard) &&
>
> not about this patch: this could use "git -C sub" and avoid a
> subshell.  But what you have here matches the existing style, so I
> wouldn't suggest changing it here.
>
>> +     echo "changed" >sub/foo &&
>> +     git status --short >output &&
>> +     diff output - <<-\EOF
>
> similarly, this could use test_cmp, but that seems out of scope for
> this change.

Yes I considered if I want to redo the whole file to meet 'modern Git test suite style'. I'll think about it again.

Show 8 quoted lines
> [...]
>> +test_expect_success 'status with added file in submodule (short)' '
> [...]
>> +test_expect_success 'status with untracked file in submodule (short)' '
>
> Test wishlist:
> * --porcelain output for these cases
> * behavior with nested submodules

Look at the test file, please. I copied the tests cases (porcelain) and replaced s/porcelain/short/, so they exist.

Show 13 quoted lines
>
> [...]
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -1664,9 +1664,19 @@ static void wt_shortstatus_status(struct string_list_item *it,
>>               color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
>>       else
>>               putchar(' ');
>> -     if (d->worktree_status)
>> +     if (d->worktree_status) {
>> +             if (!s->submodule_porcelain1 &&
>
> Isn't this 's->status_format != STATUS_FORMAT_PORCELAIN'?
yes.
Show 15 quoted lines
>> +                 (d->dirty_submodule || d->new_submodule_commits)) {
>> +                     /* It is a submodule, and we're not in plumbing mode. */
>> +                     if (d->new_submodule_commits)
>> +                             d->worktree_status = 'M';
>> +                     else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
>> +                             d->worktree_status = 'm';
>> +                     else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
>> +                             d->worktree_status = '?';
>> +             }
>
> Is this the right place to do this?  Could wt_status_collect_changed_cb set
> worktree_status to the right value in the first place based on whether
> 'status_format == STATUS_FORMAT_SHORT' or would that have undesirable
> knock-on effects elsewhere?
>
yes that would be better. I totally missed status_format as a variable.
>> +     int submodule_porcelain1;
>
> As described above, this shouldn't be needed since status_format
> already has this information.
good point. will omit this variable.
Jonathan Nieder· Mar 16, 2017, 22:53 UTC · re: Stefan Beller · lore

Re: [PATCH v2] short status: improve reporting for submodule changes

Stefan Beller wrote:
> Jonathan Nieder wrote:
Show 6 quoted lines
>> Is it important to avoid clutter by showing the submodule only once?
>> What would you think of showing whatever subset of those three
>> statuses apply to a given submodule as separate lines instead, to
>> match the information that long-form "git status" shows?
>
> I considered it, but it would break the visual appeal of git status --short ?
I could go either way.  As long as you've thought about it, I'm happy.
Show 12 quoted lines
>> How should a new untracked file in a submodule of a submodule be
>> shown?
>
> The same way " ?" indicates that (1) there is an untracked file due to
> the question mark and (2) that you need to recurse because it differs from
> "??" for regular untracked files.
>
> The problem here is that we do not know about these nested untracked files,
> because we use --porcelain instead of --short for submodules in
> submodule.c#is_submodule_modified(). I am rewriting that function anyway
> for the "git-describe --dirty" bug, so maybe it's time to switch to porcelain=2
> internally there, which can surface untracked files in nested subs.

Punting to a TODO / separate patch sounds reasonable. Tests in this patch describing either the current behavior or the desired behavior would be helpful, though.

Thanks, Jonathan

← back to recent threads