These caught my eye browsing through my inbox. I'm not a subtree user.

Am 26.07.2016 um 06:14 schrieb David Aguilar:
@@ -50,87 +51,145 @@ prefix=

 debug()
 {
-       if [ -n "$debug" ]; then
-               printf "%s\n" "$*" >&2
+       if test -n "$debug"
+       then
+               printf "%s\n" "$@" >&2

Are you sure you want this? It prints each argument of the 'debug' invocation on its own line.

        fi
 }

 say()
 {
-       if [ -z "$quiet" ]; then
-               printf "%s\n" "$*" >&2
+       if test -z "$quiet"
+       then
+               printf "%s\n" "$@" >&2

Same here.

        fi
 }

 progress()
 {
-       if [ -z "$quiet" ]; then
-               printf "%s\r" "$*" >&2
+       if test -z "$quiet"
+       then
+               printf "%s\r" "$@" >&2

But here I'm pretty sure that this is not wanted; the original is clearly correct.

        fi
 }
...
@@ -139,22 +198,27 @@ debug "command: {$command}"
 debug "quiet: {$quiet}"
 debug "revs: {$revs}"
 debug "dir: {$dir}"
-debug "opts: {$*}"
+debug "opts: {$@}"

When the arguments of a script or function are to be printed for the user's entertainment/education, then it is safer (and, therefore, idiomatic) to use "$*".

 debug
...
 cache_get()
 {
-       for oldrev in $*; do
-               if [ -r "$cachedir/$oldrev" ]; then
+       for oldrev in "$@"
+       do

It is idiomatic to write this as

        for oldrev
        do

(But your move from bare $* to quoted "$@" fits better under the "fix quoting" topic of this patch.)

+               if test -r "$cachedir/$oldrev"
+               then
                        read newrev <"$cachedir/$oldrev"
                        echo $newrev
                fi
...
@@ -631,17 +749,19 @@ cmd_split()
                debug "  parents: $parents"
                newparents=$(cache_get $parents)
                debug "  newparents: $newparents"
-               
+
                tree=$(subtree_for_commit $rev "$dir")
                debug "  tree is: $tree"

                check_parents $parents
-               
+
                # ugly.  is there no better way to tell if this is a subtree
                # vs. a mainline commit?  Does it matter?
-               if [ -z $tree ]; then
+               if test -z $tree

This works by accident. When $tree is empty, this reduces to 'test -z', which happens to evaluate to true, just what we want. But it be appropriate to put $tree in double-quotes nevertheless.

+               then
                        set_notree $rev
-                       if [ -n "$newparents" ]; then
+                       if test -n "$newparents"
+                       then
                                cache_set $rev $rev
                        fi
                        continue

-- Hannes

--
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

Reply via email to