Re: [PATCH 3/3] subtree: adjust style to match CodingGuidelines
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Jul 26, 2016, 06:12 UTC
- Message-ID
- <a12604f4-7dc9-7c35-0853-c3bffd734281@kdbg.org>
- In-Reply-To
- <20160726041416.9438-3-davvid@gmail.com>
These caught my eye browsing through my inbox. I'm not a subtree user.
Am 26.07.2016 um 06:14 schrieb David Aguilar:
Show 9 quoted lines
> @@ -50,87 +51,145 @@ prefix=
>
> debug()
> {
> - if [ -n "$debug" ]; then
> - printf "%s\n" "$*" >&2
> + if test -n "$debug"
> + then
> + printf "%s\n" "$@" >&2Are you sure you want this? It prints each argument of the 'debug' invocation on its own line.
Show 10 quoted lines
> fi
> }
>
> say()
> {
> - if [ -z "$quiet" ]; then
> - printf "%s\n" "$*" >&2
> + if test -z "$quiet"
> + then
> + printf "%s\n" "$@" >&2Same here.
Show 10 quoted lines
> fi
> }
>
> progress()
> {
> - if [ -z "$quiet" ]; then
> - printf "%s\r" "$*" >&2
> + if test -z "$quiet"
> + then
> + printf "%s\r" "$@" >&2But here I'm pretty sure that this is not wanted; the original is clearly correct.
> fi > }
...
Show 6 quoted lines
> @@ -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
...
Show 6 quoted lines
> cache_get()
> {
> - for oldrev in $*; do
> - if [ -r "$cachedir/$oldrev" ]; then
> + for oldrev in "$@"
> + doIt 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.)
Show 5 quoted lines
> + if test -r "$cachedir/$oldrev" > + then > read newrev <"$cachedir/$oldrev" > echo $newrev > fi
...
Show 16 quoted lines
> @@ -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.
Show 8 quoted lines
> + then > set_notree $rev > - if [ -n "$newparents" ]; then > + if test -n "$newparents" > + then > cache_set $rev $rev > fi > continue
-- Hannes