{"thread":{"id":"42935","subject":"[PATCH v2 3/3] subtree: adjust style to match CodingGuidelines","startedAt":"2016-07-27T00:58:27Z","lastAt":"2016-07-27T17:11:28Z","messageCount":2,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"292293","messageId":"20160727005638.9972-1-davvid@gmail.com","threadId":"42935","inReplyTo":null,"subject":"[PATCH v2 3/3] subtree: adjust style to match CodingGuidelines","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-27T00:56:38Z","receivedAt":"2016-07-27T00:58:27Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Prefer \"test\" over \"[ ... ]\", use double-quotes around variables, break\nlong lines, and properly indent \"case\" statements.\n\nHelped-by: Johannes Sixt <j6t@kdbg.org>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis is a replacement patch that addresses the notes from Hannes' review.\n\n contrib/subtree/git-subtree.sh | 570 +++++++++++++++++++++++++----------------\n 1 file changed, 354 insertions(+), 216 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex b567eae..9fd3b6a 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -4,8 +4,9 @@\n #\n # Copyright (C) 2009 Avery Pennarun <apenwarr@gmail.com>\n #\n-if [ $# -eq 0 ]; then\n-    set -- -h\n+if test $# -eq 0\n+then\n+\tset -- -h\n fi\n OPTS_SPEC=\"\\\n git subtree add   --prefix=<prefix> <commit>\n@@ -50,87 +51,145 @@ prefix=\n \n debug()\n {\n-\tif [ -n \"$debug\" ]; then\n+\tif test -n \"$debug\"\n+\tthen\n \t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n say()\n {\n-\tif [ -z \"$quiet\" ]; then\n+\tif test -z \"$quiet\"\n+\tthen\n \t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n progress()\n {\n-\tif [ -z \"$quiet\" ]; then\n+\tif test -z \"$quiet\"\n+\tthen\n \t\tprintf \"%s\\r\" \"$*\" >&2\n \tfi\n }\n \n assert()\n {\n-\tif \"$@\"; then\n-\t\t:\n-\telse\n+\tif ! \"$@\"\n+\tthen\n \t\tdie \"assertion failed: \" \"$@\"\n \tfi\n }\n \n \n-#echo \"Options: $*\"\n-\n-while [ $# -gt 0 ]; do\n+while test $# -gt 0\n+do\n \topt=\"$1\"\n \tshift\n+\n \tcase \"$opt\" in\n-\t\t-q) quiet=1 ;;\n-\t\t-d) debug=1 ;;\n-\t\t--annotate) annotate=\"$1\"; shift ;;\n-\t\t--no-annotate) annotate= ;;\n-\t\t-b) branch=\"$1\"; shift ;;\n-\t\t-P) prefix=\"${1%/}\"; shift ;;\n-\t\t-m) message=\"$1\"; shift ;;\n-\t\t--no-prefix) prefix= ;;\n-\t\t--onto) onto=\"$1\"; shift ;;\n-\t\t--no-onto) onto= ;;\n-\t\t--rejoin) rejoin=1 ;;\n-\t\t--no-rejoin) rejoin= ;;\n-\t\t--ignore-joins) ignore_joins=1 ;;\n-\t\t--no-ignore-joins) ignore_joins= ;;\n-\t\t--squash) squash=1 ;;\n-\t\t--no-squash) squash= ;;\n-\t\t--) break ;;\n-\t\t*) die \"Unexpected option: $opt\" ;;\n+\t-q)\n+\t\tquiet=1\n+\t\t;;\n+\t-d)\n+\t\tdebug=1\n+\t\t;;\n+\t--annotate)\n+\t\tannotate=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-annotate)\n+\t\tannotate=\n+\t\t;;\n+\t-b)\n+\t\tbranch=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t-P)\n+\t\tprefix=\"${1%/}\"\n+\t\tshift\n+\t\t;;\n+\t-m)\n+\t\tmessage=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-prefix)\n+\t\tprefix=\n+\t\t;;\n+\t--onto)\n+\t\tonto=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-onto)\n+\t\tonto=\n+\t\t;;\n+\t--rejoin)\n+\t\trejoin=1\n+\t\t;;\n+\t--no-rejoin)\n+\t\trejoin=\n+\t\t;;\n+\t--ignore-joins)\n+\t\tignore_joins=1\n+\t\t;;\n+\t--no-ignore-joins)\n+\t\tignore_joins=\n+\t\t;;\n+\t--squash)\n+\t\tsquash=1\n+\t\t;;\n+\t--no-squash)\n+\t\tsquash=\n+\t\t;;\n+\t--)\n+\t\tbreak\n+\t\t;;\n+\t*)\n+\t\tdie \"Unexpected option: $opt\"\n+\t\t;;\n \tesac\n done\n \n command=\"$1\"\n shift\n+\n case \"$command\" in\n-\tadd|merge|pull) default= ;;\n-\tsplit|push) default=\"--default HEAD\" ;;\n-\t*) die \"Unknown command '$command'\" ;;\n+add|merge|pull)\n+\tdefault=\n+\t;;\n+split|push)\n+\tdefault=\"--default HEAD\"\n+\t;;\n+*)\n+\tdie \"Unknown command '$command'\"\n+\t;;\n esac\n \n-if [ -z \"$prefix\" ]; then\n+if test -z \"$prefix\"\n+then\n \tdie \"You must provide the --prefix option.\"\n fi\n \n case \"$command\" in\n-\tadd) [ -e \"$prefix\" ] && \n-\t\tdie \"prefix '$prefix' already exists.\" ;;\n-\t*)   [ -e \"$prefix\" ] || \n-\t\tdie \"'$prefix' does not exist; use 'git subtree add'\" ;;\n+add)\n+\ttest -e \"$prefix\" && die \"prefix '$prefix' already exists.\"\n+\t;;\n+*)\n+\ttest -e \"$prefix\" ||\n+\tdie \"'$prefix' does not exist; use 'git subtree add'\"\n+\t;;\n esac\n \n dir=\"$(dirname \"$prefix/.\")\"\n \n-if [ \"$command\" != \"pull\" -a \"$command\" != \"add\" -a \"$command\" != \"push\" ]; then\n+if test \"$command\" != \"pull\" &&\n+\t\ttest \"$command\" != \"add\" &&\n+\t\ttest \"$command\" != \"push\"\n+then\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n-\tdirs=\"$(git rev-parse --no-revs --no-flags \"$@\")\" || exit $?\n-\tif [ -n \"$dirs\" ]; then\n+\tdirs=$(git rev-parse --no-revs --no-flags \"$@\") || exit $?\n+\tif test -n \"$dirs\"\n+\tthen\n \t\tdie \"Error: Use --prefix instead of bare filenames.\"\n \tfi\n fi\n@@ -145,16 +204,21 @@ debug\n cache_setup()\n {\n \tcachedir=\"$GIT_DIR/subtree-cache/$$\"\n-\trm -rf \"$cachedir\" || die \"Can't delete old cachedir: $cachedir\"\n-\tmkdir -p \"$cachedir\" || die \"Can't create new cachedir: $cachedir\"\n-\tmkdir -p \"$cachedir/notree\" || die \"Can't create new cachedir: $cachedir/notree\"\n+\trm -rf \"$cachedir\" ||\n+\t\tdie \"Can't delete old cachedir: $cachedir\"\n+\tmkdir -p \"$cachedir\" ||\n+\t\tdie \"Can't create new cachedir: $cachedir\"\n+\tmkdir -p \"$cachedir/notree\" ||\n+\t\tdie \"Can't create new cachedir: $cachedir/notree\"\n \tdebug \"Using cachedir: $cachedir\" >&2\n }\n \n cache_get()\n {\n-\tfor oldrev in $*; do\n-\t\tif [ -r \"$cachedir/$oldrev\" ]; then\n+\tfor oldrev in \"$@\"\n+\tdo\n+\t\tif test -r \"$cachedir/$oldrev\"\n+\t\tthen\n \t\t\tread newrev <\"$cachedir/$oldrev\"\n \t\t\techo $newrev\n \t\tfi\n@@ -163,8 +227,10 @@ cache_get()\n \n cache_miss()\n {\n-\tfor oldrev in $*; do\n-\t\tif [ ! -r \"$cachedir/$oldrev\" ]; then\n+\tfor oldrev in \"$@\"\n+\tdo\n+\t\tif ! test -r \"$cachedir/$oldrev\"\n+\t\tthen\n \t\t\techo $oldrev\n \t\tfi\n \tdone\n@@ -172,9 +238,11 @@ cache_miss()\n \n check_parents()\n {\n-\tmissed=$(cache_miss $*)\n-\tfor miss in $missed; do\n-\t\tif [ ! -r \"$cachedir/notree/$miss\" ]; then\n+\tmissed=$(cache_miss \"$@\")\n+\tfor miss in $missed\n+\tdo\n+\t\tif ! test -r \"$cachedir/notree/$miss\"\n+\t\tthen\n \t\t\tdebug \"  incorrect order: $miss\"\n \t\tfi\n \tdone\n@@ -189,9 +257,10 @@ cache_set()\n {\n \toldrev=\"$1\"\n \tnewrev=\"$2\"\n-\tif [ \"$oldrev\" != \"latest_old\" \\\n-\t     -a \"$oldrev\" != \"latest_new\" \\\n-\t     -a -e \"$cachedir/$oldrev\" ]; then\n+\tif test \"$oldrev\" != \"latest_old\" &&\n+\t\ttest \"$oldrev\" != \"latest_new\" &&\n+\t\ttest -e \"$cachedir/$oldrev\"\n+\tthen\n \t\tdie \"cache for $oldrev already exists!\"\n \tfi\n \techo \"$newrev\" >\"$cachedir/$oldrev\"\n@@ -199,7 +268,8 @@ cache_set()\n \n rev_exists()\n {\n-\tif git rev-parse \"$1\" >/dev/null 2>&1; then\n+\tif git rev-parse \"$1\" >/dev/null 2>&1\n+\tthen\n \t\treturn 0\n \telse\n \t\treturn 1\n@@ -210,10 +280,11 @@ rev_is_descendant_of_branch()\n {\n \tnewrev=\"$1\"\n \tbranch=\"$2\"\n-\tbranch_hash=$(git rev-parse $branch)\n-\tmatch=$(git rev-list -1 $branch_hash ^$newrev)\n+\tbranch_hash=$(git rev-parse \"$branch\")\n+\tmatch=$(git rev-list -1 \"$branch_hash\" \"^$newrev\")\n \n-\tif [ -z \"$match\" ]; then\n+\tif test -z \"$match\"\n+\tthen\n \t\treturn 0\n \telse\n \t\treturn 1\n@@ -225,7 +296,8 @@ rev_is_descendant_of_branch()\n # be there anyway, so do nothing in that case.\n try_remove_previous()\n {\n-\tif rev_exists \"$1^\"; then\n+\tif rev_exists \"$1^\"\n+\tthen\n \t\techo \"^$1^\"\n \tfi\n }\n@@ -239,31 +311,38 @@ find_latest_squash()\n \tsub=\n \tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n \t\t--pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |\n-\twhile read a b junk; do\n+\twhile read a b junk\n+\tdo\n \t\tdebug \"$a $b $junk\"\n \t\tdebug \"{{$sq/$main/$sub}}\"\n \t\tcase \"$a\" in\n-\t\t\tSTART) sq=\"$b\" ;;\n-\t\t\tgit-subtree-mainline:) main=\"$b\" ;;\n-\t\t\tgit-subtree-split:)\n-\t\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n-\t\t\t\t    die \"could not rev-parse split hash $b from commit $sq\"\n-\t\t\t\t;;\n-\t\t\tEND)\n-\t\t\t\tif [ -n \"$sub\" ]; then\n-\t\t\t\t\tif [ -n \"$main\" ]; then\n-\t\t\t\t\t\t# a rejoin commit?\n-\t\t\t\t\t\t# Pretend its sub was a squash.\n-\t\t\t\t\t\tsq=\"$sub\"\n-\t\t\t\t\tfi\n-\t\t\t\t\tdebug \"Squash found: $sq $sub\"\n-\t\t\t\t\techo \"$sq\" \"$sub\"\n-\t\t\t\t\tbreak\n+\t\tSTART)\n+\t\t\tsq=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-mainline:)\n+\t\t\tmain=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-split:)\n+\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n+\t\t\tdie \"could not rev-parse split hash $b from commit $sq\"\n+\t\t\t;;\n+\t\tEND)\n+\t\t\tif test -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\tif test -n \"$main\"\n+\t\t\t\tthen\n+\t\t\t\t\t# a rejoin commit?\n+\t\t\t\t\t# Pretend its sub was a squash.\n+\t\t\t\t\tsq=\"$sub\"\n \t\t\t\tfi\n-\t\t\t\tsq=\n-\t\t\t\tmain=\n-\t\t\t\tsub=\n-\t\t\t\t;;\n+\t\t\t\tdebug \"Squash found: $sq $sub\"\n+\t\t\t\techo \"$sq\" \"$sub\"\n+\t\t\t\tbreak\n+\t\t\tfi\n+\t\t\tsq=\n+\t\t\tmain=\n+\t\t\tsub=\n+\t\t\t;;\n \t\tesac\n \tdone\n }\n@@ -277,31 +356,38 @@ find_existing_splits()\n \tsub=\n \tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n \t\t--pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |\n-\twhile read a b junk; do\n+\twhile read a b junk\n+\tdo\n \t\tcase \"$a\" in\n-\t\t\tSTART) sq=\"$b\" ;;\n-\t\t\tgit-subtree-mainline:) main=\"$b\" ;;\n-\t\t\tgit-subtree-split:)\n-\t\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n-\t\t\t\t    die \"could not rev-parse split hash $b from commit $sq\"\n-\t\t\t\t;;\n-\t\t\tEND)\n-\t\t\t\tdebug \"  Main is: '$main'\"\n-\t\t\t\tif [ -z \"$main\" -a -n \"$sub\" ]; then\n-\t\t\t\t\t# squash commits refer to a subtree\n-\t\t\t\t\tdebug \"  Squash: $sq from $sub\"\n-\t\t\t\t\tcache_set \"$sq\" \"$sub\"\n-\t\t\t\tfi\n-\t\t\t\tif [ -n \"$main\" -a -n \"$sub\" ]; then\n-\t\t\t\t\tdebug \"  Prior: $main -> $sub\"\n-\t\t\t\t\tcache_set $main $sub\n-\t\t\t\t\tcache_set $sub $sub\n-\t\t\t\t\ttry_remove_previous \"$main\"\n-\t\t\t\t\ttry_remove_previous \"$sub\"\n-\t\t\t\tfi\n-\t\t\t\tmain=\n-\t\t\t\tsub=\n-\t\t\t\t;;\n+\t\tSTART)\n+\t\t\tsq=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-mainline:)\n+\t\t\tmain=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-split:)\n+\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n+\t\t\tdie \"could not rev-parse split hash $b from commit $sq\"\n+\t\t\t;;\n+\t\tEND)\n+\t\t\tdebug \"  Main is: '$main'\"\n+\t\t\tif test -z \"$main\" -a -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\t# squash commits refer to a subtree\n+\t\t\t\tdebug \"  Squash: $sq from $sub\"\n+\t\t\t\tcache_set \"$sq\" \"$sub\"\n+\t\t\tfi\n+\t\t\tif test -n \"$main\" -a -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\tdebug \"  Prior: $main -> $sub\"\n+\t\t\t\tcache_set $main $sub\n+\t\t\t\tcache_set $sub $sub\n+\t\t\t\ttry_remove_previous \"$main\"\n+\t\t\t\ttry_remove_previous \"$sub\"\n+\t\t\tfi\n+\t\t\tmain=\n+\t\t\tsub=\n+\t\t\t;;\n \t\tesac\n \tdone\n }\n@@ -325,7 +411,10 @@ copy_commit()\n \t\t\tGIT_COMMITTER_NAME \\\n \t\t\tGIT_COMMITTER_EMAIL \\\n \t\t\tGIT_COMMITTER_DATE\n-\t\t(printf \"%s\" \"$annotate\"; cat ) |\n+\t\t(\n+\t\t\tprintf \"%s\" \"$annotate\"\n+\t\t\tcat\n+\t\t) |\n \t\tgit commit-tree \"$2\" $3  # reads the rest of stdin\n \t) || die \"Can't copy commit $1\"\n }\n@@ -335,14 +424,15 @@ add_msg()\n \tdir=\"$1\"\n \tlatest_old=\"$2\"\n \tlatest_new=\"$3\"\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\tcommit_message=\"$message\"\n \telse\n \t\tcommit_message=\"Add '$dir/' from commit '$latest_new'\"\n \tfi\n \tcat <<-EOF\n \t\t$commit_message\n-\t\t\n+\n \t\tgit-subtree-dir: $dir\n \t\tgit-subtree-mainline: $latest_old\n \t\tgit-subtree-split: $latest_new\n@@ -351,7 +441,8 @@ add_msg()\n \n add_squashed_msg()\n {\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\techo \"$message\"\n \telse\n \t\techo \"Merge commit '$1' as '$2'\"\n@@ -363,14 +454,15 @@ rejoin_msg()\n \tdir=\"$1\"\n \tlatest_old=\"$2\"\n \tlatest_new=\"$3\"\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\tcommit_message=\"$message\"\n \telse\n \t\tcommit_message=\"Split '$dir/' into commit '$latest_new'\"\n \tfi\n \tcat <<-EOF\n \t\t$commit_message\n-\t\t\n+\n \t\tgit-subtree-dir: $dir\n \t\tgit-subtree-mainline: $latest_old\n \t\tgit-subtree-split: $latest_new\n@@ -383,8 +475,9 @@ squash_msg()\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n \tnewsub_short=$(git rev-parse --short \"$newsub\")\n-\t\n-\tif [ -n \"$oldsub\" ]; then\n+\n+\tif test -n \"$oldsub\"\n+\tthen\n \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n \t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n \t\techo\n@@ -393,7 +486,7 @@ squash_msg()\n \telse\n \t\techo \"Squashed '$dir/' content from commit $newsub_short\"\n \tfi\n-\t\n+\n \techo\n \techo \"git-subtree-dir: $dir\"\n \techo \"git-subtree-split: $newsub\"\n@@ -410,10 +503,11 @@ subtree_for_commit()\n \tcommit=\"$1\"\n \tdir=\"$2\"\n \tgit ls-tree \"$commit\" -- \"$dir\" |\n-\twhile read mode type tree name; do\n-\t\tassert [ \"$name\" = \"$dir\" ]\n-\t\tassert [ \"$type\" = \"tree\" -o \"$type\" = \"commit\" ]\n-\t\t[ \"$type\" = \"commit\" ] && continue  # ignore submodules\n+\twhile read mode type tree name\n+\tdo\n+\t\tassert test \"$name\" = \"$dir\"\n+\t\tassert test \"$type\" = \"tree\" -o \"$type\" = \"commit\"\n+\t\ttest \"$type\" = \"commit\" && continue  # ignore submodules\n \t\techo $tree\n \t\tbreak\n \tdone\n@@ -423,11 +517,13 @@ tree_changed()\n {\n \ttree=$1\n \tshift\n-\tif [ $# -ne 1 ]; then\n+\tif test $# -ne 1\n+\tthen\n \t\treturn 0   # weird parents, consider it changed\n \telse\n \t\tptree=$(toptree_for_commit $1)\n-\t\tif [ \"$ptree\" != \"$tree\" ]; then\n+\t\tif test \"$ptree\" != \"$tree\"\n+\t\tthen\n \t\t\treturn 0   # changed\n \t\telse\n \t\t\treturn 1   # not changed\n@@ -441,8 +537,9 @@ new_squash_commit()\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n \ttree=$(toptree_for_commit $newsub) || exit $?\n-\tif [ -n \"$old\" ]; then\n-\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" | \n+\tif test -n \"$old\"\n+\tthen\n+\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n \telse\n \t\tsquash_msg \"$dir\" \"\" \"$newsub\" |\n@@ -455,58 +552,68 @@ copy_or_skip()\n \trev=\"$1\"\n \ttree=\"$2\"\n \tnewparents=\"$3\"\n-\tassert [ -n \"$tree\" ]\n+\tassert test -n \"$tree\"\n \n \tidentical=\n \tnonidentical=\n \tp=\n \tgotparents=\n-\tfor parent in $newparents; do\n+\tfor parent in $newparents\n+\tdo\n \t\tptree=$(toptree_for_commit $parent) || exit $?\n-\t\t[ -z \"$ptree\" ] && continue\n-\t\tif [ \"$ptree\" = \"$tree\" ]; then\n+\t\ttest -z \"$ptree\" && continue\n+\t\tif test \"$ptree\" = \"$tree\"\n+\t\tthen\n \t\t\t# an identical parent could be used in place of this rev.\n \t\t\tidentical=\"$parent\"\n \t\telse\n \t\t\tnonidentical=\"$parent\"\n \t\tfi\n-\t\t\n+\n \t\t# sometimes both old parents map to the same newparent;\n \t\t# eliminate duplicates\n \t\tis_new=1\n-\t\tfor gp in $gotparents; do\n-\t\t\tif [ \"$gp\" = \"$parent\" ]; then\n+\t\tfor gp in $gotparents\n+\t\tdo\n+\t\t\tif test \"$gp\" = \"$parent\"\n+\t\t\tthen\n \t\t\t\tis_new=\n \t\t\t\tbreak\n \t\t\tfi\n \t\tdone\n-\t\tif [ -n \"$is_new\" ]; then\n+\t\tif test -n \"$is_new\"\n+\t\tthen\n \t\t\tgotparents=\"$gotparents $parent\"\n \t\t\tp=\"$p -p $parent\"\n \t\tfi\n \tdone\n \n \tcopycommit=\n-\tif [ -n \"$identical\" ] && [ -n \"$nonidentical\" ]; then\n+\tif test -n \"$identical\" && test -n \"$nonidentical\"\n+\tthen\n \t\textras=$(git rev-list --count $identical..$nonidentical)\n-\t\tif [ \"$extras\" -ne 0 ]; then\n+\t\tif test \"$extras\" -ne 0\n+\t\tthen\n \t\t\t# we need to preserve history along the other branch\n \t\t\tcopycommit=1\n \t\tfi\n \tfi\n-\tif [ -n \"$identical\" ] && [ -z \"$copycommit\" ]; then\n+\tif test -n \"$identical\" && test -z \"$copycommit\"\n+\tthen\n \t\techo $identical\n \telse\n-\t\tcopy_commit $rev $tree \"$p\" || exit $?\n+\t\tcopy_commit \"$rev\" \"$tree\" \"$p\" || exit $?\n \tfi\n }\n \n ensure_clean()\n {\n-\tif ! git diff-index HEAD --exit-code --quiet 2>&1; then\n+\tif ! git diff-index HEAD --exit-code --quiet 2>&1\n+\tthen\n \t\tdie \"Working tree has modifications.  Cannot add.\"\n \tfi\n-\tif ! git diff-index --cached HEAD --exit-code --quiet 2>&1; then\n+\tif ! git diff-index --cached HEAD --exit-code --quiet 2>&1\n+\tthen\n \t\tdie \"Index has modifications.  Cannot add.\"\n \tfi\n }\n@@ -514,34 +621,38 @@ ensure_clean()\n ensure_valid_ref_format()\n {\n \tgit check-ref-format \"refs/heads/$1\" ||\n-\t    die \"'$1' does not look like a ref\"\n+\t\tdie \"'$1' does not look like a ref\"\n }\n \n cmd_add()\n {\n-\tif [ -e \"$dir\" ]; then\n+\tif test -e \"$dir\"\n+\tthen\n \t\tdie \"'$dir' already exists.  Cannot add.\"\n \tfi\n \n \tensure_clean\n-\t\n-\tif [ $# -eq 1 ]; then\n-\t    git rev-parse -q --verify \"$1^{commit}\" >/dev/null ||\n-\t    die \"'$1' does not refer to a commit\"\n-\n-\t    \"cmd_add_commit\" \"$@\"\n-\telif [ $# -eq 2 ]; then\n-\t    # Technically we could accept a refspec here but we're\n-\t    # just going to turn around and add FETCH_HEAD under the\n-\t    # specified directory.  Allowing a refspec might be\n-\t    # misleading because we won't do anything with any other\n-\t    # branches fetched via the refspec.\n-\t    ensure_valid_ref_format \"$2\"\n-\n-\t    \"cmd_add_repository\" \"$@\"\n+\n+\tif test $# -eq 1\n+\tthen\n+\t\tgit rev-parse -q --verify \"$1^{commit}\" >/dev/null ||\n+\t\t\tdie \"'$1' does not refer to a commit\"\n+\n+\t\tcmd_add_commit \"$@\"\n+\n+\telif test $# -eq 2\n+\tthen\n+\t\t# Technically we could accept a refspec here but we're\n+\t\t# just going to turn around and add FETCH_HEAD under the\n+\t\t# specified directory.  Allowing a refspec might be\n+\t\t# misleading because we won't do anything with any other\n+\t\t# branches fetched via the refspec.\n+\t\tensure_valid_ref_format \"$2\"\n+\n+\t\tcmd_add_repository \"$@\"\n \telse\n-\t    say \"error: parameters were '$@'\"\n-\t    die \"Provide either a commit or a repository and commit.\"\n+\t\tsay \"error: parameters were '$@'\"\n+\t\tdie \"Provide either a commit or a repository and commit.\"\n \tfi\n }\n \n@@ -561,30 +672,32 @@ cmd_add_commit()\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tset -- $revs\n \trev=\"$1\"\n-\t\n+\n \tdebug \"Adding $dir as '$rev'...\"\n \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n \tgit checkout -- \"$dir\" || exit $?\n \ttree=$(git write-tree) || exit $?\n-\t\n+\n \theadrev=$(git rev-parse HEAD) || exit $?\n-\tif [ -n \"$headrev\" -a \"$headrev\" != \"$rev\" ]; then\n+\tif test -n \"$headrev\" && test \"$headrev\" != \"$rev\"\n+\tthen\n \t\theadp=\"-p $headrev\"\n \telse\n \t\theadp=\n \tfi\n-\t\n-\tif [ -n \"$squash\" ]; then\n+\n+\tif test -n \"$squash\"\n+\tthen\n \t\trev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n \t\tcommit=$(add_squashed_msg \"$rev\" \"$dir\" |\n-\t\t\t git commit-tree $tree $headp -p \"$rev\") || exit $?\n+\t\t\tgit commit-tree \"$tree\" $headp -p \"$rev\") || exit $?\n \telse\n \t\trevp=$(peel_committish \"$rev\") &&\n-\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n-\t\t\t git commit-tree $tree $headp -p \"$revp\") || exit $?\n+\t\tcommit=$(add_msg \"$dir\" $headrev \"$rev\" |\n+\t\t\tgit commit-tree \"$tree\" $headp -p \"$revp\") || exit $?\n \tfi\n \tgit reset \"$commit\" || exit $?\n-\t\n+\n \tsay \"Added dir '$dir'\"\n }\n \n@@ -592,24 +705,27 @@ cmd_split()\n {\n \tdebug \"Splitting $dir...\"\n \tcache_setup || exit $?\n-\t\n-\tif [ -n \"$onto\" ]; then\n+\n+\tif test -n \"$onto\"\n+\tthen\n \t\tdebug \"Reading history for --onto=$onto...\"\n \t\tgit rev-list $onto |\n-\t\twhile read rev; do\n+\t\twhile read rev\n+\t\tdo\n \t\t\t# the 'onto' history is already just the subdir, so\n \t\t\t# any parent we find there can be used verbatim\n \t\t\tdebug \"  cache: $rev\"\n-\t\t\tcache_set $rev $rev\n+\t\t\tcache_set \"$rev\" \"$rev\"\n \t\tdone\n \tfi\n-\t\n-\tif [ -n \"$ignore_joins\" ]; then\n+\n+\tif test -n \"$ignore_joins\"\n+\tthen\n \t\tunrevs=\n \telse\n \t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n \tfi\n-\t\n+\n \t# We can't restrict rev-list to only $dir here, because some of our\n \t# parents have the $dir contents the root, and those won't match.\n \t# (and rev-list --follow doesn't seem to solve this)\n@@ -618,12 +734,14 @@ cmd_split()\n \trevcount=0\n \tcreatecount=0\n \teval \"$grl\" |\n-\twhile read rev parents; do\n+\twhile read rev parents\n+\tdo\n \t\trevcount=$(($revcount + 1))\n \t\tprogress \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n-\t\texists=$(cache_get $rev)\n-\t\tif [ -n \"$exists\" ]; then\n+\t\texists=$(cache_get \"$rev\")\n+\t\tif test -n \"$exists\"\n+\t\tthen\n \t\t\tdebug \"  prior: $exists\"\n \t\t\tcontinue\n \t\tfi\n@@ -631,54 +749,63 @@ cmd_split()\n \t\tdebug \"  parents: $parents\"\n \t\tnewparents=$(cache_get $parents)\n \t\tdebug \"  newparents: $newparents\"\n-\t\t\n-\t\ttree=$(subtree_for_commit $rev \"$dir\")\n+\n+\t\ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n \t\tdebug \"  tree is: $tree\"\n \n \t\tcheck_parents $parents\n-\t\t\n+\n \t\t# ugly.  is there no better way to tell if this is a subtree\n \t\t# vs. a mainline commit?  Does it matter?\n-\t\tif [ -z $tree ]; then\n-\t\t\tset_notree $rev\n-\t\t\tif [ -n \"$newparents\" ]; then\n-\t\t\t\tcache_set $rev $rev\n+\t\tif test -z \"$tree\"\n+\t\tthen\n+\t\t\tset_notree \"$rev\"\n+\t\t\tif test -n \"$newparents\"\n+\t\t\tthen\n+\t\t\t\tcache_set \"$rev\" \"$rev\"\n \t\t\tfi\n \t\t\tcontinue\n \t\tfi\n \n \t\tnewrev=$(copy_or_skip \"$rev\" \"$tree\" \"$newparents\") || exit $?\n \t\tdebug \"  newrev is: $newrev\"\n-\t\tcache_set $rev $newrev\n-\t\tcache_set latest_new $newrev\n-\t\tcache_set latest_old $rev\n+\t\tcache_set \"$rev\" \"$newrev\"\n+\t\tcache_set latest_new \"$newrev\"\n+\t\tcache_set latest_old \"$rev\"\n \tdone || exit $?\n+\n \tlatest_new=$(cache_get latest_new)\n-\tif [ -z \"$latest_new\" ]; then\n+\tif test -z \"$latest_new\"\n+\tthen\n \t\tdie \"No new revisions were found\"\n \tfi\n-\t\n-\tif [ -n \"$rejoin\" ]; then\n+\n+\tif test -n \"$rejoin\"\n+\tthen\n \t\tdebug \"Merging split branch into HEAD...\"\n \t\tlatest_old=$(cache_get latest_old)\n \t\tgit merge -s ours \\\n \t\t\t--allow-unrelated-histories \\\n-\t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n-\t\t\t$latest_new >&2 || exit $?\n-\tfi\n-\tif [ -n \"$branch\" ]; then\n-\t\tif rev_exists \"refs/heads/$branch\"; then\n-\t\t\tif ! rev_is_descendant_of_branch $latest_new $branch; then\n+\t\t\t-m \"$(rejoin_msg \"$dir\" \"$latest_old\" \"$latest_new\")\" \\\n+\t\t\t\"$latest_new\" >&2 || exit $?\n+\tfi\n+\tif test -n \"$branch\"\n+\tthen\n+\t\tif rev_exists \"refs/heads/$branch\"\n+\t\tthen\n+\t\t\tif ! rev_is_descendant_of_branch \"$latest_new\" \"$branch\"\n+\t\t\tthen\n \t\t\t\tdie \"Branch '$branch' is not an ancestor of commit '$latest_new'.\"\n \t\t\tfi\n \t\t\taction='Updated'\n \t\telse\n \t\t\taction='Created'\n \t\tfi\n-\t\tgit update-ref -m 'subtree split' \"refs/heads/$branch\" $latest_new || exit $?\n+\t\tgit update-ref -m 'subtree split' \\\n+\t\t\t\"refs/heads/$branch\" \"$latest_new\" || exit $?\n \t\tsay \"$action branch '$branch'\"\n \tfi\n-\techo $latest_new\n+\techo \"$latest_new\"\n \texit 0\n }\n \n@@ -686,22 +813,26 @@ cmd_merge()\n {\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tensure_clean\n-\t\n+\n \tset -- $revs\n-\tif [ $# -ne 1 ]; then\n+\tif test $# -ne 1\n+\tthen\n \t\tdie \"You must provide exactly one revision.  Got: '$revs'\"\n \tfi\n \trev=\"$1\"\n-\t\n-\tif [ -n \"$squash\" ]; then\n+\n+\tif test -n \"$squash\"\n+\tthen\n \t\tfirst_split=\"$(find_latest_squash \"$dir\")\"\n-\t\tif [ -z \"$first_split\" ]; then\n+\t\tif test -z \"$first_split\"\n+\t\tthen\n \t\t\tdie \"Can't squash-merge: '$dir' was never added.\"\n \t\tfi\n \t\tset $first_split\n \t\told=$1\n \t\tsub=$2\n-\t\tif [ \"$sub\" = \"$rev\" ]; then\n+\t\tif test \"$sub\" = \"$rev\"\n+\t\tthen\n \t\t\tsay \"Subtree is already at commit $rev.\"\n \t\t\texit 0\n \t\tfi\n@@ -711,15 +842,19 @@ cmd_merge()\n \tfi\n \n \tversion=$(git version)\n-\tif [ \"$version\" \\< \"git version 1.7\" ]; then\n-\t\tif [ -n \"$message\" ]; then\n-\t\t\tgit merge -s subtree --message=\"$message\" $rev\n+\tif test \"$version\" \\< \"git version 1.7\"\n+\tthen\n+\t\tif test -n \"$message\"\n+\t\tthen\n+\t\t\tgit merge -s subtree --message=\"$message\" \"$rev\"\n \t\telse\n-\t\t\tgit merge -s subtree $rev\n+\t\t\tgit merge -s subtree \"$rev\"\n \t\tfi\n \telse\n-\t\tif [ -n \"$message\" ]; then\n-\t\t\tgit merge -Xsubtree=\"$prefix\" --message=\"$message\" $rev\n+\t\tif test -n \"$message\"\n+\t\tthen\n+\t\t\tgit merge -Xsubtree=\"$prefix\" \\\n+\t\t\t\t--message=\"$message\" \"$rev\"\n \t\telse\n \t\t\tgit merge -Xsubtree=\"$prefix\" $rev\n \t\tfi\n@@ -728,8 +863,9 @@ cmd_merge()\n \n cmd_pull()\n {\n-\tif [ $# -ne 2 ]; then\n-\t    die \"You must provide <repository> <ref>\"\n+\tif test $# -ne 2\n+\tthen\n+\t\tdie \"You must provide <repository> <ref>\"\n \tfi\n \tensure_clean\n \tensure_valid_ref_format \"$2\"\n@@ -741,18 +877,20 @@ cmd_pull()\n \n cmd_push()\n {\n-\tif [ $# -ne 2 ]; then\n-\t    die \"You must provide <repository> <ref>\"\n+\tif test $# -ne 2\n+\tthen\n+\t\tdie \"You must provide <repository> <ref>\"\n \tfi\n \tensure_valid_ref_format \"$2\"\n-\tif [ -e \"$dir\" ]; then\n-\t    repository=$1\n-\t    refspec=$2\n-\t    echo \"git push using: \" $repository $refspec\n-\t    localrev=$(git subtree split --prefix=\"$prefix\") || die\n-\t    git push \"$repository\" $localrev:refs/heads/$refspec\n+\tif test -e \"$dir\"\n+\tthen\n+\t\trepository=$1\n+\t\trefspec=$2\n+\t\techo \"git push using: \" \"$repository\" \"$refspec\"\n+\t\tlocalrev=$(git subtree split --prefix=\"$prefix\") || die\n+\t\tgit push \"$repository\" \"$localrev\":\"refs/heads/$refspec\"\n \telse\n-\t    die \"'$dir' must already exist. Try 'git subtree add'.\"\n+\t\tdie \"'$dir' must already exist. Try 'git subtree add'.\"\n \tfi\n }\n \n-- \n2.9.2.469.ga693980\n\n"},{"id":"292332","messageId":"xmqq1t2fxayw.fsf@gitster.mtv.corp.google.com","threadId":"42935","inReplyTo":"20160727005638.9972-1-davvid@gmail.com","subject":"Re: [PATCH v2 3/3] subtree: adjust style to match CodingGuidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-27T17:11:19Z","receivedAt":"2016-07-27T17:11:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Prefer \"test\" over \"[ ... ]\", use double-quotes around variables, break\n> long lines, and properly indent \"case\" statements.\n>\n> Helped-by: Johannes Sixt <j6t@kdbg.org>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> This is a replacement patch that addresses the notes from Hannes' review.\n\nThanks.\n\n>  case \"$command\" in\n> -\tadd) [ -e \"$prefix\" ] && \n> -\t\tdie \"prefix '$prefix' already exists.\" ;;\n> -\t*)   [ -e \"$prefix\" ] || \n> -\t\tdie \"'$prefix' does not exist; use 'git subtree add'\" ;;\n> +add)\n> +\ttest -e \"$prefix\" && die \"prefix '$prefix' already exists.\"\n> +\t;;\n> +*)\n> +\ttest -e \"$prefix\" ||\n> +\tdie \"'$prefix' does not exist; use 'git subtree add'\"\n> +\t;;\n>  esac\n\nComparing the above with the below\n\n> @@ -145,16 +204,21 @@ debug\n>  cache_setup()\n>  {\n>  \tcachedir=\"$GIT_DIR/subtree-cache/$$\"\n> -\trm -rf \"$cachedir\" || die \"Can't delete old cachedir: $cachedir\"\n> -\tmkdir -p \"$cachedir\" || die \"Can't create new cachedir: $cachedir\"\n> -\tmkdir -p \"$cachedir/notree\" || die \"Can't create new cachedir: $cachedir/notree\"\n> +\trm -rf \"$cachedir\" ||\n> +\t\tdie \"Can't delete old cachedir: $cachedir\"\n> +\tmkdir -p \"$cachedir\" ||\n> +\t\tdie \"Can't create new cachedir: $cachedir\"\n> +\tmkdir -p \"$cachedir/notree\" ||\n> +\t\tdie \"Can't create new cachedir: $cachedir/notree\"\n>  \tdebug \"Using cachedir: $cachedir\" >&2\n>  }\n\nmakes me think the former would look more readble if consistently\nformatted line this (note: I untabified to keep the alignment, so\nplease do not cut and paste from here):\n\n        case \"$command\" in\n        add)\n                test -e \"$prefix\" &&\n                        die \"prefix '$prefix' already exists.\"\n                ;;\n        *)\n                test -e \"$prefix\" ||\n                        die \"'$prefix' does not exist; use 'git subtree add'\"\n                ;;\n        esac\n\n>  cache_get()\n>  {\n\nI noticed this in the CodingGuidelines:\n\n - We prefer a space between the function name and the parentheses,\n   and no space inside the parentheses. The opening \"{\" should also\n   be on the same line.\n\nperhaps do it as well while we are at it?\n\n> -\tfor oldrev in $*; do\n> -\t\tif [ -r \"$cachedir/$oldrev\" ]; then\n> +\tfor oldrev in \"$@\"\n> +\tdo\n\nThis looked fishy, but all the callers of cache_get are doing the\nright thing.  They either throw a single token (\"latest_new\",\n\"latest_old\") at it, or give \"$rev\" (dquoted) or $parents (not\ndquoted) that is taken by reading the \"rev-list --parents\" output\nwith \"read rev parents\" one line at a time.\n\n>  cache_miss()\n>  {\n> -\tfor oldrev in $*; do\n> -\t\tif [ ! -r \"$cachedir/$oldrev\" ]; then\n> +\tfor oldrev in \"$@\"\n> +\tdo\n\nDitto.\n\n> @@ -172,9 +238,11 @@ cache_miss()\n>  \n>  check_parents()\n>  {\n> -\tmissed=$(cache_miss $*)\n> -\tfor miss in $missed; do\n> -\t\tif [ ! -r \"$cachedir/notree/$miss\" ]; then\n> +\tmissed=$(cache_miss \"$@\")\n> +\tfor miss in $missed\n\nDitto.\n\nThere is an obvious \"optimization potential\" here in that $missed is\notherwise never used, but it is outside the scope of the stylefix\npatch.\n\n> +\tif test -n \"$old\"\n> +\tthen\n> +\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n>  \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n\nThe change to the third line is to remove the trailing SP, which is\nwelcome.  While we are touching it up, \"git commit-tree\" should be\ndedented to align with \"squash_msg\" (the same in the else clause)\nto be consistent with the remainder of the script (and the system),\nI would think.\n"}]}