* Don't print status output with submodule.<name>.ignore=all @ 2013-08-03 17:14 brian m. carlson 2013-08-03 17:14 ` [PATCH 1/2] submodule: fix confusing variable name brian m. carlson 2013-08-03 17:14 ` [PATCH 2/2] submodule: don't print status output with ignore=all brian m. carlson 0 siblings, 2 replies; 17+ messages in thread From: brian m. carlson @ 2013-08-03 17:14 UTC (permalink / raw) To: git; +Cc: judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, 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. git-submodule.sh | 10 ++++++---- t/t7508-status.sh | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] submodule: fix confusing variable name 2013-08-03 17:14 Don't print status output with submodule.<name>.ignore=all brian m. carlson @ 2013-08-03 17:14 ` brian m. carlson 2013-08-03 18:14 ` Jonathan Nieder 2013-08-03 17:14 ` [PATCH 2/2] submodule: don't print status output with ignore=all brian m. carlson 1 sibling, 1 reply; 17+ messages in thread From: brian m. carlson @ 2013-08-03 17:14 UTC (permalink / raw) To: git; +Cc: judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, 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..30b7fc1 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 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 "$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="$path/.git" git-rev-parse --git-dir >/dev/null 2>&1 && + echo "$path" done ) -- 1.8.4.rc1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-03 17:14 ` [PATCH 1/2] submodule: fix confusing variable name brian m. carlson @ 2013-08-03 18:14 ` Jonathan Nieder 2013-08-04 17:34 ` Jens Lehmann 0 siblings, 1 reply; 17+ messages in thread From: Jonathan Nieder @ 2013-08-03 18:14 UTC (permalink / raw) To: brian m. carlson Cc: git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster brian m. carlson wrote: > 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. Nice. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-03 18:14 ` Jonathan Nieder @ 2013-08-04 17:34 ` Jens Lehmann 2013-08-04 21:29 ` Fredrik Gustafsson 0 siblings, 1 reply; 17+ messages in thread From: Jens Lehmann @ 2013-08-04 17:34 UTC (permalink / raw) To: Jonathan Nieder Cc: brian m. carlson, git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Ramsay Jones Am 03.08.2013 20:14, schrieb Jonathan Nieder: > brian m. carlson wrote: > >> 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. > > Nice. I totally agree. Paths and names of submodules are often confused (especially as they are identical for a submodule that was never moved), so cleaning up our sources so they use the correct term is The Right Thing. But we'll have to use sm_path here (like everywhere else in the submodule script), because we'll run into problems under Windows otherwise (see 64394e3ae9 for details). Apart from that the patch is fine. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-04 17:34 ` Jens Lehmann @ 2013-08-04 21:29 ` Fredrik Gustafsson 2013-08-06 17:33 ` Jens Lehmann 2013-08-08 17:44 ` Ramsay Jones 0 siblings, 2 replies; 17+ messages in thread From: Fredrik Gustafsson @ 2013-08-04 21:29 UTC (permalink / raw) To: Jens Lehmann Cc: Jonathan Nieder, brian m. carlson, git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Ramsay Jones On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote: > But we'll have to use sm_path here (like everywhere else in the > submodule script), because we'll run into problems under Windows > otherwise (see 64394e3ae9 for details). Apart from that the patch > is fine. We're still using path= in the foreach-script. Or rather, we're setting it. From what I can see and from the commit message 64394e3ae9 it could possible be a problem. Not sure how to solve it though... Just a simple correction would break all script depending on that. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iveqy@iveqy.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-04 21:29 ` Fredrik Gustafsson @ 2013-08-06 17:33 ` Jens Lehmann 2013-08-08 17:44 ` Ramsay Jones 1 sibling, 0 replies; 17+ messages in thread From: Jens Lehmann @ 2013-08-06 17:33 UTC (permalink / raw) To: Fredrik Gustafsson Cc: Jonathan Nieder, brian m. carlson, git, judge.packham, Jorge-Juan.Garcia-Garcia, gitster, Ramsay Jones Am 04.08.2013 23:29, schrieb Fredrik Gustafsson: > On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote: >> But we'll have to use sm_path here (like everywhere else in the >> submodule script), because we'll run into problems under Windows >> otherwise (see 64394e3ae9 for details). Apart from that the patch >> is fine. > > We're still using path= in the foreach-script. Or rather, we're setting > it. From what I can see and from the commit message 64394e3ae9 it could > possible be a problem. The commit message of 64394e3ae9 explicitly talks about that one: We note that the foreach subcommand provides $path to user scripts (ie it is part of the API), so we can't simply rename it to $sm_path. > Not sure how to solve it though... Just a simple correction would break > all script depending on that. That's exactly why that one is still there. We could deprecate it in favor of $sm_path and remove it some time later, but I'm not convinced it is an urgent issue. But me thinks the documentation of foreach should be updated, as it still talks about $path. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-04 21:29 ` Fredrik Gustafsson 2013-08-06 17:33 ` Jens Lehmann @ 2013-08-08 17:44 ` Ramsay Jones 2013-08-09 17:26 ` Fredrik Gustafsson ` (2 more replies) 1 sibling, 3 replies; 17+ messages in thread From: Ramsay Jones @ 2013-08-08 17:44 UTC (permalink / raw) To: Fredrik Gustafsson Cc: Jens Lehmann, Jonathan Nieder, brian m. carlson, git, judge.packham, Jorge-Juan.Garcia-Garcia, gitster Fredrik Gustafsson wrote: > On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote: >> But we'll have to use sm_path here (like everywhere else in the >> submodule script), because we'll run into problems under Windows >> otherwise (see 64394e3ae9 for details). Apart from that the patch >> is fine. > > We're still using path= in the foreach-script. Or rather, we're setting > it. From what I can see and from the commit message 64394e3ae9 it could > possible be a problem. Please do not use a $path variable in any script intended to be run on windows; those poor souls who would otherwise have to fix the bugs will thank you! :-D Actually, it's not so much the use of a $path variable, rather the act of _exporting_ such a variable that causes the problem. (Which is why using $path with eval_gettext[ln] is such a problem, of course.) As noted in the above commit, $path is unfortunately a documented part of the public API for the foreach subcommand. However, the foreach subcommand is (mostly) fine; given the fact that the user script is eval-ed in a context in which $path is not exported. The reason for the 'mostly' is simply that the user could shoot himself in the foot by export-ing $path in their script, so that something like: $ git submodule foreach 'export path; echo $path `git rev-parse HEAD`' will indeed fail (ie git rev-parse will not execute). > Not sure how to solve it though... Just a simple correction would break > all script depending on that. $path is part of the public API, so we can't just remove it. It would require a deprecation period, etc,. (Adding/documenting $sm_path as an alternative *may* be worth doing. dunno.) HTH ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-08 17:44 ` Ramsay Jones @ 2013-08-09 17:26 ` Fredrik Gustafsson 2013-08-09 18:53 ` Junio C Hamano 2013-08-11 19:53 ` Mark Levedahl 2 siblings, 0 replies; 17+ messages in thread From: Fredrik Gustafsson @ 2013-08-09 17:26 UTC (permalink / raw) To: Ramsay Jones Cc: Fredrik Gustafsson, Jens Lehmann, Jonathan Nieder, brian m. carlson, git, judge.packham, Jorge-Juan.Garcia-Garcia, gitster On Thu, Aug 08, 2013 at 06:44:22PM +0100, Ramsay Jones wrote: > $path is part of the public API, so we can't just remove it. It would > require a deprecation period, etc,. (Adding/documenting $sm_path as an > alternative *may* be worth doing. dunno.) Maybe something for git 2.0? Well, Jens and Junio is the ones who can make sane decissions about this. I trust they make a good decision. The state as now is that this is a bug for case insesitive systems. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iveqy@iveqy.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-08 17:44 ` Ramsay Jones 2013-08-09 17:26 ` Fredrik Gustafsson @ 2013-08-09 18:53 ` Junio C Hamano 2013-08-11 19:53 ` Mark Levedahl 2 siblings, 0 replies; 17+ messages in thread From: Junio C Hamano @ 2013-08-09 18:53 UTC (permalink / raw) To: Ramsay Jones Cc: Fredrik Gustafsson, Jens Lehmann, Jonathan Nieder, brian m. carlson, git, judge.packham, Jorge-Juan.Garcia-Garcia Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > $path is part of the public API, so we can't just remove it. It would > require a deprecation period, etc,. (Adding/documenting $sm_path as an > alternative *may* be worth doing. dunno.) I think exporting sm_path (if not done already) and documenting the transition may be a good starting point, but deprecation of $path may be tricky. There is no good way for us to warn people who continue using $path and ask them to fix their fingers and scripts to use $sm_path instead. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] submodule: fix confusing variable name 2013-08-08 17:44 ` Ramsay Jones 2013-08-09 17:26 ` Fredrik Gustafsson 2013-08-09 18:53 ` Junio C Hamano @ 2013-08-11 19:53 ` Mark Levedahl 2 siblings, 0 replies; 17+ messages in thread From: Mark Levedahl @ 2013-08-11 19:53 UTC (permalink / raw) To: Ramsay Jones Cc: Fredrik Gustafsson, Jens Lehmann, Jonathan Nieder, brian m. carlson, git, judge.packham, Jorge-Juan.Garcia-Garcia, gitster On 08/08/2013 01:44 PM, Ramsay Jones wrote: > Fredrik Gustafsson wrote: >> On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote: >>> But we'll have to use sm_path here (like everywhere else in the >>> submodule script), because we'll run into problems under Windows >>> otherwise (see 64394e3ae9 for details). Apart from that the patch >>> is fine. >> >> We're still using path= in the foreach-script. Or rather, we're setting >> it. From what I can see and from the commit message 64394e3ae9 it could >> possible be a problem. > > Please do not use a $path variable in any script intended to be run on > windows; those poor souls who would otherwise have to fix the bugs will > thank you! :-D > > Actually, it's not so much the use of a $path variable, rather the act > of _exporting_ such a variable that causes the problem. (Which is why > using $path with eval_gettext[ln] is such a problem, of course.) > Please note that especially in this case, Cygwin != Windows. Cygwin allows $path, $Path, $PATH, etc., to all coexist and be accessed case sensitively. Exporting $path causes no problem, either. Should the eval invoke a Windows program, $PATH is converted to Windows format and exported, the other case-sensitive variants of path remain in the environment and can be accessed by any program implementing case-sensitive lookup as well. Not sure what will happen with case-insensitive lookups, but a quick test showed that cmd.exe is not bothered by $path given $PATH exists. Mark ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-03 17:14 Don't print status output with submodule.<name>.ignore=all brian m. carlson 2013-08-03 17:14 ` [PATCH 1/2] submodule: fix confusing variable name brian m. carlson @ 2013-08-03 17:14 ` brian m. carlson 2013-08-03 18:24 ` Jonathan Nieder 1 sibling, 1 reply; 17+ messages in thread From: brian m. carlson @ 2013-08-03 17:14 UTC (permalink / raw) To: git; +Cc: judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, 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. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- git-submodule.sh | 2 ++ t/t7508-status.sh | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 30b7fc1..5694ae6 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1034,6 +1034,8 @@ cmd_summary() { sane_egrep '^:([0-7]* )?160000' | while read mod_src mod_dst sha1_src sha1_dst status path do + name=$(module_name "$path") + test $(get_submodule_config "$name" ignore none) = all && continue # Always show modules deleted or type-changed (blob<->module) test $status = D -o $status = T && echo "$path" && continue # Also show added or modified modules which are checked out 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.rc1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-03 17:14 ` [PATCH 2/2] submodule: don't print status output with ignore=all brian m. carlson @ 2013-08-03 18:24 ` Jonathan Nieder 2013-08-04 18:24 ` Jens Lehmann 2013-08-11 16:03 ` brian m. carlson 0 siblings, 2 replies; 17+ messages in thread From: Jonathan Nieder @ 2013-08-03 18:24 UTC (permalink / raw) To: brian m. carlson Cc: git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Jens Lehmann brian m. carlson wrote: > 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. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > git-submodule.sh | 2 ++ > t/t7508-status.sh | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) Thanks. Cc-ing Jens, who wrote that test and knows this code much better than I do. :) [...] > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -1034,6 +1034,8 @@ cmd_summary() { > sane_egrep '^:([0-7]* )?160000' | > while read mod_src mod_dst sha1_src sha1_dst status path > do > + name=$(module_name "$path") > + test $(get_submodule_config "$name" ignore none) = all && continue > # Always show modules deleted or type-changed (blob<->module) > test $status = D -o $status = T && echo "$path" && continue I'm not sure what the exact semantics should be here, though that's mostly because of my unfamiliarity with submodules in general. If I have '[submodule "favorite"] ignore = all' and I then replace that submodule with a blob, should "git submodule status" not mention that path? If I just renamed a submodule, will 'module_name "$path"' do the right thing with the old path? (rest of the patch kept unsnipped for reference) > # Also show added or modified modules which are checked out > 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.rc1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-03 18:24 ` Jonathan Nieder @ 2013-08-04 18:24 ` Jens Lehmann 2013-08-10 16:37 ` brian m. carlson 2013-08-11 16:03 ` brian m. carlson 1 sibling, 1 reply; 17+ messages in thread From: Jens Lehmann @ 2013-08-04 18:24 UTC (permalink / raw) To: Jonathan Nieder Cc: brian m. carlson, git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster Am 03.08.2013 20:24, schrieb Jonathan Nieder: > brian m. carlson wrote: > >> 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. I'm a bit confused. The commit message talks about "git status", but the code you changed handles "git submodule summary". Looks like you are trying to fix the output of status when the status.submodulesummary option is set, right? That's a good thing to do. But your patch also changes the default behavior of "git submodule summary", which is a change in behavior as that is currently not configurable via the ignore option (and I believe it should stay that way for backward compatibility reasons unless actual users provide sound reasons to change that). So a NACK on this patch from me (and a note to self that tests are missing that should have failed due to this change). As a short term solution you could honor the submodule.<name>.ignore setting only if --for-status is used, as that is explicitly given by "git status" when it forks the "git submodule summary" script (to make it prepend "# " to each line, which it could do easily itself nowadays using recently added code ;-). If you want to fix that issue and make git status perform a lot better, you should make the status.submodulesummary use what we already have in the diff machinery instead of forking the submodule script (which it does for hysterical raisins). Basically you'd need to run just what "git diff-files" and "git diff-index HEAD" run when they are given the --submodule option, prepend "# " to the output and limit it to the amount of lines configured via the status.submodulesummary option. Then we could get rid of the --for-status option of submodule summary and move some more functionality from that script into core git. I'll be glad to help you fixing this problem either way. >> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> >> --- >> git-submodule.sh | 2 ++ >> t/t7508-status.sh | 4 ++-- >> 2 files changed, 4 insertions(+), 2 deletions(-) > > Thanks. Cc-ing Jens, who wrote that test and knows this code much > better than I do. :) > > [...] >> --- a/git-submodule.sh >> +++ b/git-submodule.sh >> @@ -1034,6 +1034,8 @@ cmd_summary() { >> sane_egrep '^:([0-7]* )?160000' | >> while read mod_src mod_dst sha1_src sha1_dst status path >> do >> + name=$(module_name "$path") >> + test $(get_submodule_config "$name" ignore none) = all && continue >> # Always show modules deleted or type-changed (blob<->module) >> test $status = D -o $status = T && echo "$path" && continue > > I'm not sure what the exact semantics should be here, though that's > mostly because of my unfamiliarity with submodules in general. > > If I have '[submodule "favorite"] ignore = all' and I then replace > that submodule with a blob, should "git submodule status" not mention > that path? > > If I just renamed a submodule, will 'module_name "$path"' do the right > thing with the old path? > > (rest of the patch kept unsnipped for reference) >> # Also show added or modified modules which are checked out >> 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.rc1 > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-04 18:24 ` Jens Lehmann @ 2013-08-10 16:37 ` brian m. carlson 0 siblings, 0 replies; 17+ messages in thread From: brian m. carlson @ 2013-08-10 16:37 UTC (permalink / raw) To: Jens Lehmann Cc: Jonathan Nieder, git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster [-- Attachment #1: Type: text/plain, Size: 1487 bytes --] On Sun, Aug 04, 2013 at 08:24:09PM +0200, Jens Lehmann wrote: > I'm a bit confused. The commit message talks about "git status", but the code > you changed handles "git submodule summary". Looks like you are trying to fix > the output of status when the status.submodulesummary option is set, right? > That's a good thing to do. > > But your patch also changes the default behavior of "git submodule summary", > which is a change in behavior as that is currently not configurable via the > ignore option (and I believe it should stay that way for backward compatibility > reasons unless actual users provide sound reasons to change that). So a NACK > on this patch from me (and a note to self that tests are missing that should > have failed due to this change). Right, that wasn't the intent. > As a short term solution you could honor the submodule.<name>.ignore setting > only if --for-status is used, as that is explicitly given by "git status" when > it forks the "git submodule summary" script (to make it prepend "# " to each > line, which it could do easily itself nowadays using recently added code ;-). I think I'm going to go this route. My goal is to fix up the TODO tests and make them work so I can get more familiar with the code. -- 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] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-03 18:24 ` Jonathan Nieder 2013-08-04 18:24 ` Jens Lehmann @ 2013-08-11 16:03 ` brian m. carlson 2013-08-11 18:33 ` Jonathan Nieder 2013-08-17 16:27 ` brian m. carlson 1 sibling, 2 replies; 17+ messages in thread From: brian m. carlson @ 2013-08-11 16:03 UTC (permalink / raw) To: Jonathan Nieder Cc: git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Jens Lehmann [-- Attachment #1: Type: text/plain, Size: 986 bytes --] On Sat, Aug 03, 2013 at 11:24:20AM -0700, Jonathan Nieder wrote: > If I have '[submodule "favorite"] ignore = all' and I then replace > that submodule with a blob, should "git submodule status" not mention > that path? Yes, I think it should. I'll fix this in the reroll. > If I just renamed a submodule, will 'module_name "$path"' do the right > thing with the old path? module_name uses whatever's in .gitmodules. I'm not sure what you mean by "renamed a submodule", since "git mv foo bar" fails with: vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq fatal: source directory is empty, source=.vim/bundle/ctrlp, destination=.vim/bundle/ctrlq Can you provide me a set of steps to reproduce that operation so I can test it effectively? -- 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] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-11 16:03 ` brian m. carlson @ 2013-08-11 18:33 ` Jonathan Nieder 2013-08-17 16:27 ` brian m. carlson 1 sibling, 0 replies; 17+ messages in thread From: Jonathan Nieder @ 2013-08-11 18:33 UTC (permalink / raw) To: brian m. carlson Cc: git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Jens Lehmann brian m. carlson wrote: > module_name uses whatever's in .gitmodules. I'm not sure what you mean > by "renamed a submodule", since "git mv foo bar" fails with: > > vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq > fatal: source directory is empty, source=.vim/bundle/ctrlp, destination=.vim/bundle/ctrlq > > Can you provide me a set of steps to reproduce that operation so I can > test it effectively? Ah, I forgot Jens's "submodule-mv" patch series has not hit master yet. You can get it by running git merge origin/pu^{/jl/submodule-mv}^2 before building git. Thanks, Jonathan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] submodule: don't print status output with ignore=all 2013-08-11 16:03 ` brian m. carlson 2013-08-11 18:33 ` Jonathan Nieder @ 2013-08-17 16:27 ` brian m. carlson 1 sibling, 0 replies; 17+ messages in thread From: brian m. carlson @ 2013-08-17 16:27 UTC (permalink / raw) To: Jonathan Nieder Cc: git, judge.packham, iveqy, Jorge-Juan.Garcia-Garcia, gitster, Jens Lehmann [-- Attachment #1: Type: text/plain, Size: 1307 bytes --] On Sun, Aug 11, 2013 at 04:03:17PM +0000, brian m. carlson wrote: > On Sat, Aug 03, 2013 at 11:24:20AM -0700, Jonathan Nieder wrote: > > If I just renamed a submodule, will 'module_name "$path"' do the right > > thing with the old path? > > module_name uses whatever's in .gitmodules. I'm not sure what you mean > by "renamed a submodule", since "git mv foo bar" fails with: > > vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq > fatal: source directory is empty, source=.vim/bundle/ctrlp, destination=.vim/bundle/ctrlq Okay, I've tested this against next and it seems that the code handles it properly (at least I think it does). I left the following code # Always show modules deleted or type-changed (blob<->module) test $status = D -o $status = T && echo "$sm_path" && continue before the ignore code, so I adjusted the new ignore code so that it prints both the new and the old path. I don't know that it's possible to print neither in that case, since we no longer can look up the old path in .gitmodules. I'll be sending my reroll shortly. -- 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] 17+ messages in thread
end of thread, other threads:[~2013-08-17 16:27 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-03 17:14 Don't print status output with submodule.<name>.ignore=all brian m. carlson 2013-08-03 17:14 ` [PATCH 1/2] submodule: fix confusing variable name brian m. carlson 2013-08-03 18:14 ` Jonathan Nieder 2013-08-04 17:34 ` Jens Lehmann 2013-08-04 21:29 ` Fredrik Gustafsson 2013-08-06 17:33 ` Jens Lehmann 2013-08-08 17:44 ` Ramsay Jones 2013-08-09 17:26 ` Fredrik Gustafsson 2013-08-09 18:53 ` Junio C Hamano 2013-08-11 19:53 ` Mark Levedahl 2013-08-03 17:14 ` [PATCH 2/2] submodule: don't print status output with ignore=all brian m. carlson 2013-08-03 18:24 ` Jonathan Nieder 2013-08-04 18:24 ` Jens Lehmann 2013-08-10 16:37 ` brian m. carlson 2013-08-11 16:03 ` brian m. carlson 2013-08-11 18:33 ` Jonathan Nieder 2013-08-17 16:27 ` brian m. carlson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.