Re: submodule: if $command was not matched, don't parse other args
On 12-09-23 01:36 PM, Jens Lehmann wrote: Am 22.09.2012 22:31, schrieb Junio C Hamano: Ramkumar Ramachandra artag...@gmail.com writes: diff --git a/git-submodule.sh b/git-submodule.sh index a7e933e..dfec45d 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1108,7 +1108,15 @@ do done # No command word defaults to status -test -n $command || command=status +if test -z $command +then +if test $# = 0 +then + command=status +else + usage +fi +fi I personally feel no command means this default is a mistake for git submodule, even if there is no pathspec or other arguments, but I am not a heavy user of submodules, so others should discuss this. The commit message of 97a5d8cce9 (git-submodule: re-enable 'status' as the default subcommand) back from 2007 indicates that Lars did back then think that status is a sane default. I agree with Junio that this is not optimal, but I'd rather tend to not change that behavior which has been there from day one for backward compatibility reasons. But if many others see that as an improvement too I won't object against changing it the way Ramkumar proposes (but he'd have to change the documentation too ;-). Since diff and status learned to display submodule status information (except for a submodule being uninitialized) I almost never use this option myself, so I'd be interested to hear what submodule users who do use git submodule [status] frequently think. I also almost never use git submodule [status], and I also agree that git-submodule shouldn't have a default sub-command. (Honestly, submodule's status sub-command has always felt more like plumbing to me than something a user would work with directly. Maybe it's just the full-length SHA's that put me off...) M. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: submodule: if $command was not matched, don't parse other args
Marc Branchaud mbranch...@xiplink.com writes: On 12-09-23 01:36 PM, Jens Lehmann wrote: Am 22.09.2012 22:31, schrieb Junio C Hamano: Ramkumar Ramachandra artag...@gmail.com writes: diff --git a/git-submodule.sh b/git-submodule.sh index a7e933e..dfec45d 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1108,7 +1108,15 @@ do done # No command word defaults to status -test -n $command || command=status +if test -z $command +then +if test $# = 0 +then + command=status +else + usage +fi +fi I personally feel no command means this default is a mistake for git submodule, even if there is no pathspec or other arguments, but I am not a heavy user of submodules, so others should discuss this. ... but I'd rather tend to not change that behavior which has been there from day one for backward compatibility reasons. But if many others see that as an improvement too I won't object against changing it the way Ramkumar proposes (but he'd have to change the documentation too ;-). Since diff and status learned to display submodule status information (except for a submodule being uninitialized) I almost never use this option myself, so I'd be interested to hear what submodule users who do use git submodule [status] frequently think. I also almost never use git submodule [status], and I also agree that git-submodule shouldn't have a default sub-command. OK, I do not think Ramkumar's patch hurts anybody, but dropping the nothing on the command line defaults to 'status' action could. So let's queue the patch as-is at least for now and leave the default discussion to a separarte thread if needed. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: submodule: if $command was not matched, don't parse other args
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: OK, I do not think Ramkumar's patch hurts anybody, but dropping the nothing on the command line defaults to 'status' action could. So let's queue the patch as-is at least for now and leave the default discussion to a separarte thread if needed. Please don't do that, because it breaks test 41 in t7400-submodule-bash. I'll add a hunk to remove the test and send a patch tomorrow. I personally see no need waiting for something trivial like this. Isn't it sufficient to squash the following in? Is anything else needed? Documentation/git-submodule.txt | 1 - t/t7400-submodule-basic.sh | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git i/Documentation/git-submodule.txt w/Documentation/git-submodule.txt index 2de7bf0..b4683bb 100644 --- i/Documentation/git-submodule.txt +++ w/Documentation/git-submodule.txt @@ -112,7 +112,6 @@ status:: initialized, `+` if the currently checked out submodule commit does not match the SHA-1 found in the index of the containing repository and `U` if the submodule has merge conflicts. - This command is the default command for 'git submodule'. + If `--recursive` is specified, this command will recurse into nested submodules, and show their status as well. diff --git i/t/t7400-submodule-basic.sh w/t/t7400-submodule-basic.sh index 0278f48..442dc44 100755 --- i/t/t7400-submodule-basic.sh +++ w/t/t7400-submodule-basic.sh @@ -438,8 +438,8 @@ test_expect_success 'moving to a commit without submodule does not leave empty d git checkout second ' -test_expect_success 'submodule invalid-path warns' ' - test_failure_with_unknown_submodule +test_expect_success 'submodule invalid-subcommand fails' ' + test_must_fail git submodule no-such-subcommand ' test_expect_success 'add submodules without specifying an explicit path' ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: submodule: if $command was not matched, don't parse other args
Am 22.09.2012 22:31, schrieb Junio C Hamano: Ramkumar Ramachandra artag...@gmail.com writes: diff --git a/git-submodule.sh b/git-submodule.sh index a7e933e..dfec45d 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1108,7 +1108,15 @@ do done # No command word defaults to status -test -n $command || command=status +if test -z $command +then +if test $# = 0 +then +command=status +else +usage +fi +fi I personally feel no command means this default is a mistake for git submodule, even if there is no pathspec or other arguments, but I am not a heavy user of submodules, so others should discuss this. The commit message of 97a5d8cce9 (git-submodule: re-enable 'status' as the default subcommand) back from 2007 indicates that Lars did back then think that status is a sane default. I agree with Junio that this is not optimal, but I'd rather tend to not change that behavior which has been there from day one for backward compatibility reasons. But if many others see that as an improvement too I won't object against changing it the way Ramkumar proposes (but he'd have to change the documentation too ;-). Since diff and status learned to display submodule status information (except for a submodule being uninitialized) I almost never use this option myself, so I'd be interested to hear what submodule users who do use git submodule [status] frequently think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
