{"thread":{"id":"23010","subject":"[PATCH] git status: ignoring untracked files must apply to submodules too","startedAt":"2010-03-13T22:00:27Z","lastAt":"2010-03-14T00:55:35Z","messageCount":5,"participants":["Jens Lehmann","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"136735","messageId":"4B9C0AFB.1050306@web.de","threadId":"23010","inReplyTo":null,"subject":"[PATCH] git status: ignoring untracked files must apply to submodules too","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-03-13T22:00:27Z","receivedAt":"2010-03-13T22:00:27Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 1.7.0 submodules are considered dirty when they contain untracked\nfiles. But when git status is called with the \"-uno\" option, the user\nasked to ignore untracked files, so they must be ignored in submodules\ntoo. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES\nis introduced.\n\nSigned-off-by: Jens Lehmann \n---\n\nThis patch applies on top of current pu.\n\nI'm open to suggestions for a shorter name for the new diff option\nIGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a\nshorter yet still descriptive enough name.\n\n\n diff-lib.c | 2 +-\n diff.h | 1 +\n submodule.c | 9 +++++++--\n submodule.h | 2 +-\n t/t7506-status-submodule.sh | 5 +++++\n wt-status.c | 2 ++\n 6 files changed, 17 insertions(+), 4 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 4e30b8f..c9f6e05 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,\n \tif (S_ISGITLINK(ce->ce_mode)\n \t && !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)\n \t && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {\n-\t\t*dirty_submodule = is_submodule_modified(ce->name);\n+\t\t*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));\n \t}\n \treturn changed;\n }\ndiff --git a/diff.h b/diff.h\nindex 95ed7f8..6a71013 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -70,6 +70,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n #define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)\n #define DIFF_OPT_SUBMODULE_LOG (1 << 23)\n #define DIFF_OPT_DIRTY_SUBMODULES (1 << 24)\n+#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)\n\n #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)\ndiff --git a/submodule.c b/submodule.c\nindex 714ca97..b3b8bc1 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -130,7 +130,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \tstrbuf_release(&sb);\n }\n\n-unsigned is_submodule_modified(const char *path)\n+unsigned is_submodule_modified(const char *path, int ignore_untracked)\n {\n \tint i;\n \tssize_t len;\n@@ -139,6 +139,7 @@ unsigned is_submodule_modified(const char *path)\n \t\t\"status\",\n \t\t\"--porcelain\",\n \t\tNULL,\n+\t\tNULL,\n \t};\n \tconst char *env[LOCAL_REPO_ENV_SIZE + 3];\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -163,6 +164,9 @@ unsigned is_submodule_modified(const char *path)\n \tenv[i++] = strbuf_detach(&buf, NULL);\n \tenv[i] = NULL;\n\n+\tif (ignore_untracked)\n+\t\targv[2] = \"-uno\";\n+\n \tmemset(&cp, 0, sizeof(cp));\n \tcp.argv = argv;\n \tcp.env = env;\n@@ -181,7 +185,8 @@ unsigned is_submodule_modified(const char *path)\n \t\t\t\tbreak;\n \t\t} else {\n \t\t\tdirty_submodule |= DIRTY_SUBMODULE_MODIFIED;\n-\t\t\tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n+\t\t\tif (ignore_untracked ||\n+\t\t\t (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED))\n \t\t\t\tbreak;\n \t\t}\n \t\tnext_line = strchr(line, '\\n');\ndiff --git a/submodule.h b/submodule.h\nindex 267881c..dbda270 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -5,6 +5,6 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule,\n \t\tconst char *del, const char *add, const char *reset);\n-unsigned is_submodule_modified(const char *path);\n+unsigned is_submodule_modified(const char *path, int ignore_untracked);\n\n #endif\ndiff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh\nindex dc9150a..aeec1f6 100755\n--- a/t/t7506-status-submodule.sh\n+++ b/t/t7506-status-submodule.sh\n@@ -67,6 +67,11 @@ test_expect_success 'status with untracked file in submodule' '\n \tgrep \"modified: sub (untracked content)\" output\n '\n\n+test_expect_success 'status -uno with untracked file in submodule' '\n+\tgit status -uno >output &&\n+\tgrep \"^nothing to commit\" output\n+'\n+\n test_expect_success 'status with untracked file in submodule (porcelain)' '\n \tgit status --porcelain >output &&\n \tdiff output - <<-\\EOF\ndiff --git a/wt-status.c b/wt-status.c\nindex cf90430..8ca59a2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -304,6 +304,8 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, NULL);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \tDIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);\n+\tif (!s->show_untracked_files)\n+\t\tDIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \trev.diffopt.format_callback = wt_status_collect_changed_cb;\n \trev.diffopt.format_callback_data = s;\n \trev.prune_data = s->pathspec;\n-- \n1.7.0.2.399.g53a88\n"},{"id":"136738","messageId":"7vhboj6et9.fsf@alter.siamese.dyndns.org","threadId":"23010","inReplyTo":"4B9C0AFB.1050306@web.de","subject":"Re: [PATCH] git status: ignoring untracked files must apply to submodules too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-13T22:24:02Z","receivedAt":"2010-03-13T22:24:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Since 1.7.0 submodules are considered dirty when they contain untracked\n> files. But when git status is called with the \"-uno\" option, the user\n> asked to ignore untracked files, so they must be ignored in submodules\n> too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES\n> is introduced.\n>\n> Signed-off-by: Jens Lehmann \n> ---\n>\n> This patch applies on top of current pu.\n>\n> I'm open to suggestions for a shorter name for the new diff option\n> IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a\n> shorter yet still descriptive enough name.\n\nWhy do you even need that flag? Isn't it the matter of deciding to ignore\nor pay attention to the DIRTY_SUBMODULE_UNTRACKED bit in the return value\nof is_submodule_modified(), depending on whether the toplevel wt_status\nwas called with -uno?\n"},{"id":"136743","messageId":"4B9C1AEC.2070605@web.de","threadId":"23010","inReplyTo":"7vhboj6et9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git status: ignoring untracked files must apply to submodules too","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-03-13T23:08:28Z","receivedAt":"2010-03-13T23:08:28Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 13.03.2010 23:24, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> Since 1.7.0 submodules are considered dirty when they contain untracked\n>> files. But when git status is called with the \"-uno\" option, the user\n>> asked to ignore untracked files, so they must be ignored in submodules\n>> too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES\n>> is introduced.\n>>\n>> Signed-off-by: Jens Lehmann \n>> ---\n>>\n>> This patch applies on top of current pu.\n>>\n>> I'm open to suggestions for a shorter name for the new diff option\n>> IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a\n>> shorter yet still descriptive enough name.\n> \n> Why do you even need that flag? Isn't it the matter of deciding to ignore\n> or pay attention to the DIRTY_SUBMODULE_UNTRACKED bit in the return value\n> of is_submodule_modified(), depending on whether the toplevel wt_status\n> was called with -uno?\n\nFirst: When called from \"git status\" run_diff_files() calls\nwt_status_collect_changed_cb() for every file it considers changed, so\nwhen the \"-uno\" option is given for a submodule with only untracked files\ni thought it cleaner to let is_submodule_modified() return 0 so this\ncallback is not called at all and the submodule won't show up in the\n\"change\" list of wt_status (Yes, this could be done by not adding such a\nsubmodule to that list in the wt_status_collect_changed_cb() too).\n\nSecond: One of my next patches will be about adding an option to the git\ndiff family to not show submodules with only untracked files in their\nwork tree as modified, which calls for such a diff option AFAICS.\n\nThird: We give is_submodule_modified() the possibility to stop parsing the\noutput of the \"git status\" run in the submodule early, because as soon as\nit sees a modified file it will stop to parse the output further when\nuntracked files shall be ignored.\n"},{"id":"136746","messageId":"7vaaubzrxq.fsf@alter.siamese.dyndns.org","threadId":"23010","inReplyTo":"4B9C1AEC.2070605@web.de","subject":"Re: [PATCH] git status: ignoring untracked files must apply to submodules too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-14T00:07:45Z","receivedAt":"2010-03-14T00:07:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> First: When called from \"git status\" run_diff_files() calls\n> wt_status_collect_changed_cb() for every file it considers changed, so\n\nThis reminds me of another thing, perhaps independent, perhaps related.\nWhy aren't we collecting the submodule status in wt_status_collect() to\nbegin with? It examines the submodule status in wt_status_print(), but\nthat feels quite against the way how the whole \"struct wt_status\" was\ndesigned to be used in the first place, I think. Would restuctuing the\ncode that way make this easier to handle?\n"},{"id":"136747","messageId":"4B9C3407.9090502@web.de","threadId":"23010","inReplyTo":"7vaaubzrxq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git status: ignoring untracked files must apply to submodules too","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-03-14T00:55:35Z","receivedAt":"2010-03-14T00:55:35Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 14.03.2010 01:07, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> First: When called from \"git status\" run_diff_files() calls\n>> wt_status_collect_changed_cb() for every file it considers changed, so\n> \n> This reminds me of another thing, perhaps independent, perhaps related.\n> Why aren't we collecting the submodule status in wt_status_collect() to\n> begin with? It examines the submodule status in wt_status_print(), but\n> that feels quite against the way how the whole \"struct wt_status\" was\n> designed to be used in the first place, I think. Would restuctuing the\n> code that way make this easier to handle?\n\nI'm not sure I understand that, but AFAICS in wt_status_print_changed()\nonly the fact that at least one submodule is dirty is examined to be\nable to print the extra hint line. The status of each submodule is\ncollected in wt_status_collect_changed_cb(), no?\n\nAnd while not having being active when \"struct wt_status\" has been\ndesigned, i think adding submodules to the \"change\" list when they\nare dirty makes kind a sense ... but i might be wrong ;-)\n"}]}