| Message ID | 20241207135201.2536-2-royeldar0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | git-submodule.sh: improve parsing of options | expand |
Roy Eldar <royeldar0@gmail.com> writes: > When git-submodule.sh parses various options and switchs, it sets some > variables to values; in particular, every switch that is passed causes a > corresponding variable to be set to 1, which then affects the options > given to git-submodule--helper. > > There are some variables are assigned "$1", although there is no reason > for it; this was actually noticed in 757d092 for the "$cached" variable. Wearing devil's advocate hat on. This can cut both ways. If you are doing a thin wrapper front-end, when calling into the underlying machinery that has its own option parser, it is often easier to write and read submodule--helper $force $cached instead of submodule--helper ${force:+--force} ${cached:+--cached} In addition, when debugging such a script by running it under "sh -x", a typical construct like if test "$force" = 1 then ... do the force thing ... would appear in the "-x" trace as +test 1 = 1 +... do the force thing ... if you force yourself to use "1", but if test "$force" = "--force" then ... do the force thing ... would show either +test --force = --force or +test "" = --force and the latter is especially useful when you are wondering why the "--force" you gave from the command line is not taking effect. Having said all that, as long as the use of these switch variables are done consistently (e.g., consistently set effective ones to "1", and consistently set ones that are off to ""), we are OK. Thanks.
On Sat, Dec 7, 2024 at 8:53 AM Roy Eldar <royeldar0@gmail.com> wrote: > When git-submodule.sh parses various options and switchs, it sets some s/switchs/switches/ > variables to values; in particular, every switch that is passed causes a > corresponding variable to be set to 1, which then affects the options > given to git-submodule--helper. > > There are some variables are assigned "$1", although there is no reason s/are assigned/assigned/ > for it; this was actually noticed in 757d092 for the "$cached" variable. > > Make some variables boolean, in order to increase consistency throught s/throught/throughout/ > the script and reduce possible confusion. > > Signed-off-by: Roy Eldar <royeldar0@gmail.com>
diff --git a/git-submodule.sh b/git-submodule.sh index 03c5a220a2..107011f613 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -78,7 +78,7 @@ cmd_add() shift ;; -f | --force) - force=$1 + force=1 ;; -q|--quiet) quiet=1 @@ -231,7 +231,7 @@ cmd_deinit() do case "$1" in -f|--force) - force=$1 + force=1 ;; -q|--quiet) quiet=1 @@ -294,7 +294,7 @@ cmd_update() nofetch=1 ;; -f|--force) - force=$1 + force=1 ;; -r|--rebase) rebase=1 @@ -500,10 +500,10 @@ cmd_summary() { cached=1 ;; --files) - files="$1" + files=1 ;; --for-status) - for_status="$1" + for_status=1 ;; -n|--summary-limit) summary_limit="$2"
When git-submodule.sh parses various options and switchs, it sets some variables to values; in particular, every switch that is passed causes a corresponding variable to be set to 1, which then affects the options given to git-submodule--helper. There are some variables are assigned "$1", although there is no reason for it; this was actually noticed in 757d092 for the "$cached" variable. Make some variables boolean, in order to increase consistency throught the script and reduce possible confusion. Signed-off-by: Roy Eldar <royeldar0@gmail.com> --- git-submodule.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)