* [PATCH v3 0/2] submodule: Don't print status output with submodule.<name>.ignore=all @ 2013-09-01 20:06 brian m. carlson 2013-09-01 20:06 ` [PATCH v3 1/2] submodule: fix confusing variable name brian m. carlson 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson 0 siblings, 2 replies; 13+ messages in thread From: brian m. carlson @ 2013-09-01 20:06 UTC (permalink / raw) To: git; +Cc: jrnieder, Jens.Lehmann, judge.packham, gitster There are configuration options for each submodule that specify under what circumstances git status should display output for that submodule. Unfortunately, these settings were not being respected, and as such the tests were marked TODO. This patch series consists of two patches: the first is a fix for a confusing variable name, and the latter actually makes git status not output the submodule information. The tests have been updated accordingly, and additional tests have been included to ensure that git submodule summary is not affected by these changes unless the --for-status option is given. Changes from v2: * Add tests to ensure that git submodule summary is not affected. * Fix bug that caused git submodule summary to be affected. Changes from v1: * Handle moved submodules by not ignoring them. * Use sm_path instead of path. * Only ignore when --for-status is given. brian m. carlson (2): submodule: fix confusing variable name submodule: don't print status output with ignore=all git-submodule.sh | 15 +++++++++++---- t/t7401-submodule-summary.sh | 30 ++++++++++++++++++++++++++++++ t/t7508-status.sh | 4 ++-- 3 files changed, 43 insertions(+), 6 deletions(-) -- 1.8.4.rc3 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/2] submodule: fix confusing variable name 2013-09-01 20:06 [PATCH v3 0/2] submodule: Don't print status output with submodule.<name>.ignore=all brian m. carlson @ 2013-09-01 20:06 ` brian m. carlson 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson 1 sibling, 0 replies; 13+ messages in thread From: brian m. carlson @ 2013-09-01 20:06 UTC (permalink / raw) To: git; +Cc: jrnieder, Jens.Lehmann, judge.packham, gitster cmd_summary reads the output of git diff, but reads in the submodule path into a variable called name. Since this variable does not contain the name of the submodule, but the path, rename it to be clearer what data it actually holds. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- git-submodule.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..38520db 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1032,13 +1032,13 @@ cmd_summary() { # Get modified modules cared by user modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- "$@" | sane_egrep '^:([0-7]* )?160000' | - while read mod_src mod_dst sha1_src sha1_dst status name + while read mod_src mod_dst sha1_src sha1_dst status sm_path do # Always show modules deleted or type-changed (blob<->module) - test $status = D -o $status = T && echo "$name" && continue + test $status = D -o $status = T && echo "$sm_path" && continue # Also show added or modified modules which are checked out - GIT_DIR="$name/.git" git-rev-parse --git-dir >/dev/null 2>&1 && - echo "$name" + GIT_DIR="$sm_path/.git" git-rev-parse --git-dir >/dev/null 2>&1 && + echo "$sm_path" done ) -- 1.8.4.rc3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-01 20:06 [PATCH v3 0/2] submodule: Don't print status output with submodule.<name>.ignore=all brian m. carlson 2013-09-01 20:06 ` [PATCH v3 1/2] submodule: fix confusing variable name brian m. carlson @ 2013-09-01 20:06 ` brian m. carlson 2013-09-03 19:17 ` Jens Lehmann ` (2 more replies) 1 sibling, 3 replies; 13+ messages in thread From: brian m. carlson @ 2013-09-01 20:06 UTC (permalink / raw) To: git; +Cc: jrnieder, Jens.Lehmann, judge.packham, gitster git status prints information for submodules, but it should ignore the status of those which have submodule.<name>.ignore set to all. Fix it so that it does properly ignore those which have that setting either in .git/config or in .gitmodules. Not ignored are submodules that are added, deleted, or moved (which is essentially a combination of the first two) because it is not easily possible to determine the old path once a move has occurred, nor is it easily possible to detect which adds and deletions are moves and which are not. This also preserves the previous behavior of always listing modules which are to be deleted. Tests are included which verify that this change has no effect on git submodule summary without the --for-status option. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- git-submodule.sh | 7 +++++++ t/t7401-submodule-summary.sh | 30 ++++++++++++++++++++++++++++++ t/t7508-status.sh | 4 ++-- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 38520db..004b21c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1036,6 +1036,13 @@ cmd_summary() { do # Always show modules deleted or type-changed (blob<->module) test $status = D -o $status = T && echo "$sm_path" && continue + # Respect the ignore setting for --for-status. + if test -n "$for_status" + then + name=$(module_name "$sm_path") + ignore_config=$(get_submodule_config "$name" ignore none) + test $status != A -a $ignore_config = all && continue + fi # Also show added or modified modules which are checked out GIT_DIR="$sm_path/.git" git-rev-parse --git-dir >/dev/null 2>&1 && echo "$sm_path" diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index ac2434c..ca9441e 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -104,6 +104,36 @@ EOF test_cmp expected actual " +test_expect_success '.gitmodules ignore=all has no effect' " + git config --add -f .gitmodules submodule.sm1.ignore all && + git config --add -f .gitmodules submodule.sm1.path sm1 && + git submodule summary >actual && + cat >expected <<-EOF && +* sm1 $head1...$head2 (1): + > Add foo3 + +EOF + test_cmp expected actual && + git config -f .gitmodules --remove-section submodule.sm1 +" + +test_expect_success '.git/config ignore=all has no effect' " + git config --add -f .gitmodules submodule.sm1.ignore none && + git config --add -f .gitmodules submodule.sm1.path sm1 && + git config --add submodule.sm1.ignore all && + git config --add submodule.sm1.path sm1 && + git submodule summary >actual && + cat >expected <<-EOF && +* sm1 $head1...$head2 (1): + > Add foo3 + +EOF + test_cmp expected actual && + git config --remove-section submodule.sm1 && + git config -f .gitmodules --remove-section submodule.sm1 +" + + commit_file sm1 && head3=$( cd sm1 && diff --git a/t/t7508-status.sh b/t/t7508-status.sh index ac3d0fe..fb89fb9 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1316,7 +1316,7 @@ test_expect_success "--ignore-submodules=all suppresses submodule summary" ' test_i18ncmp expect output ' -test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' +test_expect_success '.gitmodules ignore=all suppresses submodule summary' ' git config --add -f .gitmodules submodule.subname.ignore all && git config --add -f .gitmodules submodule.subname.path sm && git status > output && @@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' git config -f .gitmodules --remove-section submodule.subname ' -test_expect_failure '.git/config ignore=all suppresses submodule summary' ' +test_expect_success '.git/config ignore=all suppresses submodule summary' ' git config --add -f .gitmodules submodule.subname.ignore none && git config --add -f .gitmodules submodule.subname.path sm && git config --add submodule.subname.ignore all && -- 1.8.4.rc3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson @ 2013-09-03 19:17 ` Jens Lehmann 2013-09-03 19:53 ` Junio C Hamano 2013-09-04 6:31 ` Matthieu Moy 2 siblings, 0 replies; 13+ messages in thread From: Jens Lehmann @ 2013-09-03 19:17 UTC (permalink / raw) To: brian m. carlson; +Cc: git, jrnieder, judge.packham, gitster, Matthieu Moy Am 01.09.2013 22:06, schrieb brian m. carlson: > git status prints information for submodules, but it should ignore the status of > those which have submodule.<name>.ignore set to all. Fix it so that it does > properly ignore those which have that setting either in .git/config or in > .gitmodules. > > Not ignored are submodules that are added, deleted, or moved (which is > essentially a combination of the first two) because it is not easily possible to > determine the old path once a move has occurred, nor is it easily possible to > detect which adds and deletions are moves and which are not. This also > preserves the previous behavior of always listing modules which are to be > deleted. > > Tests are included which verify that this change has no effect on git submodule > summary without the --for-status option. Thanks, that fixes the bug Matthieu noticed which is now tested for. I only have some nits concerning the new tests, please see below. > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > git-submodule.sh | 7 +++++++ > t/t7401-submodule-summary.sh | 30 ++++++++++++++++++++++++++++++ > t/t7508-status.sh | 4 ++-- > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/git-submodule.sh b/git-submodule.sh > index 38520db..004b21c 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -1036,6 +1036,13 @@ cmd_summary() { > do > # Always show modules deleted or type-changed (blob<->module) > test $status = D -o $status = T && echo "$sm_path" && continue > + # Respect the ignore setting for --for-status. > + if test -n "$for_status" > + then > + name=$(module_name "$sm_path") > + ignore_config=$(get_submodule_config "$name" ignore none) > + test $status != A -a $ignore_config = all && continue > + fi > # Also show added or modified modules which are checked out > GIT_DIR="$sm_path/.git" git-rev-parse --git-dir >/dev/null 2>&1 && > echo "$sm_path" > diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh > index ac2434c..ca9441e 100755 > --- a/t/t7401-submodule-summary.sh > +++ b/t/t7401-submodule-summary.sh > @@ -104,6 +104,36 @@ EOF > test_cmp expected actual > " > > +test_expect_success '.gitmodules ignore=all has no effect' " > + git config --add -f .gitmodules submodule.sm1.ignore all && > + git config --add -f .gitmodules submodule.sm1.path sm1 && > + git submodule summary >actual && > + cat >expected <<-EOF && > +* sm1 $head1...$head2 (1): > + > Add foo3 > + > +EOF > + test_cmp expected actual && > + git config -f .gitmodules --remove-section submodule.sm1 > +" > + > +test_expect_success '.git/config ignore=all has no effect' " > + git config --add -f .gitmodules submodule.sm1.ignore none && > + git config --add -f .gitmodules submodule.sm1.path sm1 && > + git config --add submodule.sm1.ignore all && > + git config --add submodule.sm1.path sm1 && > + git submodule summary >actual && > + cat >expected <<-EOF && > +* sm1 $head1...$head2 (1): > + > Add foo3 > + > +EOF > + test_cmp expected actual && > + git config --remove-section submodule.sm1 && > + git config -f .gitmodules --remove-section submodule.sm1 > +" > + > + > commit_file sm1 && > head3=$( > cd sm1 && I think we should do both tests in one go: just turn on all ignore options and test against that. Additionally we should also set diff.ignoreSubmodules too, while there is no need to set the path in .git/config. And I believe we can drop the --add option from the config command, so I'd propose something like this: -------------------------8<--------------------------- diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index ac2434c..81ae7c9 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -104,6 +104,24 @@ EOF test_cmp expected actual " +test_expect_success 'no ignore=all setting has any effect' " + git config -f .gitmodules submodule.sm1.path sm1 && + git config -f .gitmodules submodule.sm1.ignore all && + git config submodule.sm1.ignore all && + git config diff.ignoreSubmodules all && + git submodule summary >actual && + cat >expected <<-EOF && +* sm1 $head1...$head2 (1): + > Add foo3 + +EOF + test_cmp expected actual && + git config --unset diff.ignoreSubmodules && + git config --remove-section submodule.sm1 && + git config -f .gitmodules --remove-section submodule.sm1 +" + + commit_file sm1 && head3=$( cd sm1 && -------------------------8<--------------------------- > diff --git a/t/t7508-status.sh b/t/t7508-status.sh > index ac3d0fe..fb89fb9 100755 > --- a/t/t7508-status.sh > +++ b/t/t7508-status.sh > @@ -1316,7 +1316,7 @@ test_expect_success "--ignore-submodules=all suppresses submodule summary" ' > test_i18ncmp expect output > ' > > -test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' > +test_expect_success '.gitmodules ignore=all suppresses submodule summary' ' > git config --add -f .gitmodules submodule.subname.ignore all && > git config --add -f .gitmodules submodule.subname.path sm && > git status > output && > @@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' > git config -f .gitmodules --remove-section submodule.subname > ' > > -test_expect_failure '.git/config ignore=all suppresses submodule summary' ' > +test_expect_success '.git/config ignore=all suppresses submodule summary' ' > git config --add -f .gitmodules submodule.subname.ignore none && > git config --add -f .gitmodules submodule.subname.path sm && > git config --add submodule.subname.ignore all && > ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson 2013-09-03 19:17 ` Jens Lehmann @ 2013-09-03 19:53 ` Junio C Hamano 2013-09-04 20:01 ` Jens Lehmann 2013-09-04 6:31 ` Matthieu Moy 2 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2013-09-03 19:53 UTC (permalink / raw) To: Jens.Lehmann; +Cc: git, jrnieder, judge.packham, brian m. carlson Jens, I see 1/2 is the same as the previous one you already acked. Does this update to 2/2 look good to you? Sorry, but I lost track of the discussion that led to this reroll, hence a ping. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-03 19:53 ` Junio C Hamano @ 2013-09-04 20:01 ` Jens Lehmann 2013-09-04 20:57 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Jens Lehmann @ 2013-09-04 20:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, jrnieder, judge.packham, brian m. carlson Am 03.09.2013 21:53, schrieb Junio C Hamano: > Jens, I see 1/2 is the same as the previous one you already acked. Yep. > Does this update to 2/2 look good to you? Sorry, but I lost track > of the discussion that led to this reroll, hence a ping. v3 fixes the bug Matthieu noticed, I only had some remarks to the new test Brian added. If you could replace his patch to t7401 with the following diff it's an ack from me on this one too. -------------------------8<--------------------------- diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index ac2434c..81ae7c9 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -104,6 +104,24 @@ EOF test_cmp expected actual " +test_expect_success 'no ignore=all setting has any effect' " + git config -f .gitmodules submodule.sm1.path sm1 && + git config -f .gitmodules submodule.sm1.ignore all && + git config submodule.sm1.ignore all && + git config diff.ignoreSubmodules all && + git submodule summary >actual && + cat >expected <<-EOF && +* sm1 $head1...$head2 (1): + > Add foo3 + +EOF + test_cmp expected actual && + git config --unset diff.ignoreSubmodules && + git config --remove-section submodule.sm1 && + git config -f .gitmodules --remove-section submodule.sm1 +" + + commit_file sm1 && head3=$( cd sm1 && ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-04 20:01 ` Jens Lehmann @ 2013-09-04 20:57 ` Junio C Hamano 2013-09-04 21:26 ` Jens Lehmann 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2013-09-04 20:57 UTC (permalink / raw) To: Jens Lehmann; +Cc: git, jrnieder, judge.packham, brian m. carlson Jens Lehmann <Jens.Lehmann@web.de> writes: > Am 03.09.2013 21:53, schrieb Junio C Hamano: >> Jens, I see 1/2 is the same as the previous one you already acked. > > Yep. > >> Does this update to 2/2 look good to you? Sorry, but I lost track >> of the discussion that led to this reroll, hence a ping. > > v3 fixes the bug Matthieu noticed, I only had some remarks to the > new test Brian added. If you could replace his patch to t7401 with > the following diff it's an ack from me on this one too. > > -------------------------8<--------------------------- > diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh > index ac2434c..81ae7c9 100755 > --- a/t/t7401-submodule-summary.sh > +++ b/t/t7401-submodule-summary.sh > @@ -104,6 +104,24 @@ EOF > test_cmp expected actual > " > > +test_expect_success 'no ignore=all setting has any effect' " > + git config -f .gitmodules submodule.sm1.path sm1 && > + git config -f .gitmodules submodule.sm1.ignore all && > + git config submodule.sm1.ignore all && > + git config diff.ignoreSubmodules all && > + git submodule summary >actual && > + cat >expected <<-EOF && > +* sm1 $head1...$head2 (1): > + > Add foo3 > + > +EOF > + test_cmp expected actual && > + git config --unset diff.ignoreSubmodules && > + git config --remove-section submodule.sm1 && > + git config -f .gitmodules --remove-section submodule.sm1 > +" > + > + > commit_file sm1 && > head3=$( > cd sm1 && Thanks. The above patch makes the <<-EOF situation that already exists in this script worse. The only reason we would say -EOF not EOF is because we would want to indent the here-document to align with the rest of the command sequence, so we should either indent with HT, or drop the dash. I suspect the original did it that way fearing that someday the indentation of the submodule difference list might start using HT, but I do not think that is likely to happen, so my vote goes to keeping '-' and indenting. We need a clean-up patch after this series settles. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-04 20:57 ` Junio C Hamano @ 2013-09-04 21:26 ` Jens Lehmann 0 siblings, 0 replies; 13+ messages in thread From: Jens Lehmann @ 2013-09-04 21:26 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, jrnieder, judge.packham, brian m. carlson Am 04.09.2013 22:57, schrieb Junio C Hamano: > Jens Lehmann <Jens.Lehmann@web.de> writes: > >> Am 03.09.2013 21:53, schrieb Junio C Hamano: >>> Does this update to 2/2 look good to you? Sorry, but I lost track >>> of the discussion that led to this reroll, hence a ping. >> >> v3 fixes the bug Matthieu noticed, I only had some remarks to the >> new test Brian added. If you could replace his patch to t7401 with >> the following diff it's an ack from me on this one too. >> >> -------------------------8<--------------------------- >> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh >> index ac2434c..81ae7c9 100755 >> --- a/t/t7401-submodule-summary.sh >> +++ b/t/t7401-submodule-summary.sh >> @@ -104,6 +104,24 @@ EOF >> test_cmp expected actual >> " >> >> +test_expect_success 'no ignore=all setting has any effect' " >> + git config -f .gitmodules submodule.sm1.path sm1 && >> + git config -f .gitmodules submodule.sm1.ignore all && >> + git config submodule.sm1.ignore all && >> + git config diff.ignoreSubmodules all && >> + git submodule summary >actual && >> + cat >expected <<-EOF && >> +* sm1 $head1...$head2 (1): >> + > Add foo3 >> + >> +EOF >> + test_cmp expected actual && >> + git config --unset diff.ignoreSubmodules && >> + git config --remove-section submodule.sm1 && >> + git config -f .gitmodules --remove-section submodule.sm1 >> +" >> + >> + >> commit_file sm1 && >> head3=$( >> cd sm1 && > > Thanks. > > The above patch makes the <<-EOF situation that already exists in > this script worse. The only reason we would say -EOF not EOF is > because we would want to indent the here-document to align with the > rest of the command sequence, so we should either indent with HT, or > drop the dash. I suspect the original did it that way fearing that > someday the indentation of the submodule difference list might start > using HT, but I do not think that is likely to happen, so my vote > goes to keeping '-' and indenting. > > We need a clean-up patch after this series settles. Ok, will do (unless someone else volunteers ;-). ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson 2013-09-03 19:17 ` Jens Lehmann 2013-09-03 19:53 ` Junio C Hamano @ 2013-09-04 6:31 ` Matthieu Moy 2013-09-04 20:40 ` Jens Lehmann 2 siblings, 1 reply; 13+ messages in thread From: Matthieu Moy @ 2013-09-04 6:31 UTC (permalink / raw) To: brian m. carlson; +Cc: git, jrnieder, Jens.Lehmann, judge.packham, gitster "brian m. carlson" <sandals@crustytoothpaste.net> writes: > Tests are included which verify that this change has no effect on git submodule > summary without the --for-status option. I still don't understand why this is needed. Why do we want "git status" and "git submodule summary" to display different information? Wasn't it a nice property that the part of "git status" about submodule is the same as "git submodule summary"? This should at least be explained in the commit message IMHO. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-04 6:31 ` Matthieu Moy @ 2013-09-04 20:40 ` Jens Lehmann 2013-09-05 6:30 ` Matthieu Moy 0 siblings, 1 reply; 13+ messages in thread From: Jens Lehmann @ 2013-09-04 20:40 UTC (permalink / raw) To: Matthieu Moy; +Cc: brian m. carlson, git, jrnieder, judge.packham, gitster Am 04.09.2013 08:31, schrieb Matthieu Moy: > "brian m. carlson" <sandals@crustytoothpaste.net> writes: > >> Tests are included which verify that this change has no effect on git submodule >> summary without the --for-status option. > > I still don't understand why this is needed. To avoid a change in behavior for "git submodule summary", as that never honored the submodule.*.ignore nor the diff.ignoreSubmodules setting (and I don't think it ever should). > Why do we want "git status" > and "git submodule summary" to display different information? Wasn't it > a nice property that the part of "git status" about submodule is the > same as "git submodule summary"? I changed that in 2010 (1.7.3) to make the output of status consistent, meaning that submodules that didn't show up in the regular status output don't appear in the summary part either. And I still believe it is the right thing to do for the target audience of the ignore settings, as they want to hide any changes in submodules that are either expensive to traverse or uninteresting for the developer until they explicitly use the submodule commands, which still behave as they always did (I might be wrong here as I'm not in that group myself, but so far no one spoke up). > This should at least be explained in the commit message IMHO. Fine by me, what would you propose to clarify that? (Though I have the suspicion that the explanation will be three years late ;-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-04 20:40 ` Jens Lehmann @ 2013-09-05 6:30 ` Matthieu Moy 2013-09-05 8:05 ` Matthieu Moy 2013-09-06 0:19 ` brian m. carlson 0 siblings, 2 replies; 13+ messages in thread From: Matthieu Moy @ 2013-09-05 6:30 UTC (permalink / raw) To: Jens Lehmann; +Cc: brian m. carlson, git, jrnieder, judge.packham, gitster Jens Lehmann <Jens.Lehmann@web.de> writes: > Am 04.09.2013 08:31, schrieb Matthieu Moy: >> "brian m. carlson" <sandals@crustytoothpaste.net> writes: >> >>> Tests are included which verify that this change has no effect on git submodule >>> summary without the --for-status option. >> >> I still don't understand why this is needed. > > To avoid a change in behavior for "git submodule summary", as that > never honored the submodule.*.ignore nor the diff.ignoreSubmodules > setting (and I don't think it ever should). I don't get it. If the goal is to keep the old behavior, then "git status" shouldn't be changed either. Fixing bugs needs to change the behavior. IOW, why was it a bug that "git status" showed ignored submodules and not a bug that "git submodule summary" did the same? > Fine by me, what would you propose to clarify that? (Though I have the > suspicion that the explanation will be three years late ;-) I have no idea, as I do not understand the reason myself yet. I'm not a frequent user of submodules and not a user of the ignore option at all, so I can't tell what's best. I'd just like the new behavior to be justified somewhere. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-05 6:30 ` Matthieu Moy @ 2013-09-05 8:05 ` Matthieu Moy 2013-09-06 0:19 ` brian m. carlson 1 sibling, 0 replies; 13+ messages in thread From: Matthieu Moy @ 2013-09-05 8:05 UTC (permalink / raw) To: Jens Lehmann; +Cc: brian m. carlson, git, jrnieder, judge.packham, gitster Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes: > Jens Lehmann <Jens.Lehmann@web.de> writes: > >> Fine by me, what would you propose to clarify that? (Though I have the >> suspicion that the explanation will be three years late ;-) > > I have no idea, as I do not understand the reason myself yet. I'm not a > frequent user of submodules and not a user of the ignore option at all, > so I can't tell what's best. I'd just like the new behavior to be > justified somewhere. I also think that the documentation in Documentation/config.txt could be updated. Perhaps adding something like To view the summary for submodules ignored by 'git status', one can use 'git submodules summary' which shows a similar output but does not honor this option. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] submodule: don't print status output with ignore=all 2013-09-05 6:30 ` Matthieu Moy 2013-09-05 8:05 ` Matthieu Moy @ 2013-09-06 0:19 ` brian m. carlson 1 sibling, 0 replies; 13+ messages in thread From: brian m. carlson @ 2013-09-06 0:19 UTC (permalink / raw) To: Matthieu Moy; +Cc: Jens Lehmann, git, jrnieder, judge.packham, gitster [-- Attachment #1: Type: text/plain, Size: 1625 bytes --] On Thu, Sep 05, 2013 at 08:30:53AM +0200, Matthieu Moy wrote: > Jens Lehmann <Jens.Lehmann@web.de> writes: > > > Am 04.09.2013 08:31, schrieb Matthieu Moy: > >> "brian m. carlson" <sandals@crustytoothpaste.net> writes: > >> > >>> Tests are included which verify that this change has no effect on git submodule > >>> summary without the --for-status option. > >> > >> I still don't understand why this is needed. > > > > To avoid a change in behavior for "git submodule summary", as that > > never honored the submodule.*.ignore nor the diff.ignoreSubmodules > > setting (and I don't think it ever should). > > I don't get it. If the goal is to keep the old behavior, then "git > status" shouldn't be changed either. Fixing bugs needs to change the > behavior. > > IOW, why was it a bug that "git status" showed ignored submodules and > not a bug that "git submodule summary" did the same? I looked at the tests, and the tests expected git status to respect the ignore settings, but there were no tests that expected git submodule summary to work this way. Perhaps this is a failure in the tests. I got feedback that git submodule summary should *not* change, and nothing until now to contradict that. If the decision is that this should change both, then fine, I'll reroll. But it's not reasonable to keep going back and forth: we should make a decision and stick to it. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-09-06 0:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-01 20:06 [PATCH v3 0/2] submodule: Don't print status output with submodule.<name>.ignore=all brian m. carlson 2013-09-01 20:06 ` [PATCH v3 1/2] submodule: fix confusing variable name brian m. carlson 2013-09-01 20:06 ` [PATCH v3 2/2] submodule: don't print status output with ignore=all brian m. carlson 2013-09-03 19:17 ` Jens Lehmann 2013-09-03 19:53 ` Junio C Hamano 2013-09-04 20:01 ` Jens Lehmann 2013-09-04 20:57 ` Junio C Hamano 2013-09-04 21:26 ` Jens Lehmann 2013-09-04 6:31 ` Matthieu Moy 2013-09-04 20:40 ` Jens Lehmann 2013-09-05 6:30 ` Matthieu Moy 2013-09-05 8:05 ` Matthieu Moy 2013-09-06 0:19 ` brian m. carlson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).