[PATCH] git status: ignoring untracked files must apply to submodules too

[PATCH] git status: ignoring untracked files must apply to submodules too

[Jens Lehmann]

Since 1.7.0 submodules are considered dirty when they contain untracked
files. But when git status is called with the "-uno" option, the user
asked to ignore untracked files, so they must be ignored in submodules
too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES
is introduced.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

This patch applies on top of current pu.

I'm open to suggestions for a shorter name for the new diff option
IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a
shorter yet still descriptive enough name.


 diff-lib.c                  |    2 +-
 diff.h                      |    1 +
 submodule.c                 |    9 +++++++--
 submodule.h                 |    2 +-
 t/t7506-status-submodule.sh |    5 +++++
 wt-status.c                 |    2 ++
 6 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 4e30b8f..c9f6e05 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	if (S_ISGITLINK(ce->ce_mode)
 	    && !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
 	    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
-		*dirty_submodule = is_submodule_modified(ce->name);
+		*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
 	}
 	return changed;
 }
diff --git a/diff.h b/diff.h
index 95ed7f8..6a71013 100644
--- a/diff.h
+++ b/diff.h
@@ -70,6 +70,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
 #define DIFF_OPT_SUBMODULE_LOG       (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)
+#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)

 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
diff --git a/submodule.c b/submodule.c
index 714ca97..b3b8bc1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -130,7 +130,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

-unsigned is_submodule_modified(const char *path)
+unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	int i;
 	ssize_t len;
@@ -139,6 +139,7 @@ unsigned is_submodule_modified(const char *path)
 		"status",
 		"--porcelain",
 		NULL,
+		NULL,
 	};
 	const char *env[LOCAL_REPO_ENV_SIZE + 3];
 	struct strbuf buf = STRBUF_INIT;
@@ -163,6 +164,9 @@ unsigned is_submodule_modified(const char *path)
 	env[i++] = strbuf_detach(&buf, NULL);
 	env[i] = NULL;

+	if (ignore_untracked)
+		argv[2] = "-uno";
+
 	memset(&cp, 0, sizeof(cp));
 	cp.argv = argv;
 	cp.env = env;
@@ -181,7 +185,8 @@ unsigned is_submodule_modified(const char *path)
 				break;
 		} else {
 			dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
-			if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+			if (ignore_untracked ||
+			    (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED))
 				break;
 		}
 		next_line = strchr(line, '\n');
diff --git a/submodule.h b/submodule.h
index 267881c..dbda270 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,6 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
-unsigned is_submodule_modified(const char *path);
+unsigned is_submodule_modified(const char *path, int ignore_untracked);

 #endif
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index dc9150a..aeec1f6 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -67,6 +67,11 @@ test_expect_success 'status with untracked file in submodule' '
 	grep "modified:   sub (untracked content)" output
 '

+test_expect_success 'status -uno with untracked file in submodule' '
+	git status -uno >output &&
+	grep "^nothing to commit" output
+'
+
 test_expect_success 'status with untracked file in submodule (porcelain)' '
 	git status --porcelain >output &&
 	diff output - <<-\EOF
diff --git a/wt-status.c b/wt-status.c
index cf90430..8ca59a2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -304,6 +304,8 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	if (!s->show_untracked_files)
+		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
 	rev.prune_data = s->pathspec;
-- 
1.7.0.2.399.g53a88

Re: [PATCH] git status: ignoring untracked files must apply to submodules too

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Since 1.7.0 submodules are considered dirty when they contain untracked
> files. But when git status is called with the "-uno" option, the user
> asked to ignore untracked files, so they must be ignored in submodules
> too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES
> is introduced.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> This patch applies on top of current pu.
>
> I'm open to suggestions for a shorter name for the new diff option
> IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a
> shorter yet still descriptive enough name.

Why do you even need that flag?  Isn't it the matter of deciding to ignore
or pay attention to the DIRTY_SUBMODULE_UNTRACKED bit in the return value
of is_submodule_modified(), depending on whether the toplevel wt_status
was called with -uno?

Re: [PATCH] git status: ignoring untracked files must apply to submodules too

[Jens Lehmann]

Am 13.03.2010 23:24, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Since 1.7.0 submodules are considered dirty when they contain untracked
>> files. But when git status is called with the "-uno" option, the user
>> asked to ignore untracked files, so they must be ignored in submodules
>> too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES
>> is introduced.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>>
>> This patch applies on top of current pu.
>>
>> I'm open to suggestions for a shorter name for the new diff option
>> IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a
>> shorter yet still descriptive enough name.
> 
> Why do you even need that flag?  Isn't it the matter of deciding to ignore
> or pay attention to the DIRTY_SUBMODULE_UNTRACKED bit in the return value
> of is_submodule_modified(), depending on whether the toplevel wt_status
> was called with -uno?

First: When called from "git status" run_diff_files() calls
wt_status_collect_changed_cb() for every file it considers changed, so
when the "-uno" option is given for a submodule with only untracked files
i thought it cleaner to let is_submodule_modified() return 0 so this
callback is not called at all and the submodule won't show up in the
"change" list of wt_status (Yes, this could be done by not adding such a
submodule to that list in the wt_status_collect_changed_cb() too).

Second: One of my next patches will be about adding an option to the git
diff family to not show submodules with only untracked files in their
work tree as modified, which calls for such a diff option AFAICS.

Third: We give is_submodule_modified() the possibility to stop parsing the
output of the "git status" run in the submodule early, because as soon as
it sees a modified file it will stop to parse the output further when
untracked files shall be ignored.

Re: [PATCH] git status: ignoring untracked files must apply to submodules too

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> First: When called from "git status" run_diff_files() calls
> wt_status_collect_changed_cb() for every file it considers changed, so

This reminds me of another thing, perhaps independent, perhaps related.
Why aren't we collecting the submodule status in wt_status_collect() to
begin with?  It examines the submodule status in wt_status_print(), but
that feels quite against the way how the whole "struct wt_status" was
designed to be used in the first place, I think.  Would restuctuing the
code that way make this easier to handle?

Re: [PATCH] git status: ignoring untracked files must apply to submodules too

[Jens Lehmann]

Am 14.03.2010 01:07, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> First: When called from "git status" run_diff_files() calls
>> wt_status_collect_changed_cb() for every file it considers changed, so
> 
> This reminds me of another thing, perhaps independent, perhaps related.
> Why aren't we collecting the submodule status in wt_status_collect() to
> begin with?  It examines the submodule status in wt_status_print(), but
> that feels quite against the way how the whole "struct wt_status" was
> designed to be used in the first place, I think.  Would restuctuing the
> code that way make this easier to handle?

I'm not sure I understand that, but AFAICS in wt_status_print_changed()
only the fact that at least one submodule is dirty is examined to be
able to print the extra hint line. The status of each submodule is
collected in wt_status_collect_changed_cb(), no?

And while not having being active when "struct wt_status" has been
designed, i think adding submodules to the "change" list when they
are dirty makes kind a sense ... but i might be wrong ;-)