{"thread":{"id":"10687","subject":"[PATCH 0/5] some shell portability fixes","startedAt":"2007-11-06T20:15:18Z","lastAt":"2007-11-12T11:20:54Z","messageCount":38,"participants":["Ralf Wildenhues","Junio C Hamano","Mike Hommey","Johannes Schindelin","Andreas Ericsson","Wincent Colaiuta","Mike Ralphson","Nguyen Thai Ngoc Duy","Jon Loeliger","Robin Rosenberg","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"58577","messageId":"20071106201518.GA6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":null,"subject":"[PATCH 0/5] some shell portability fixes","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:15:18Z","receivedAt":"2007-11-06T20:15:18Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"Here's a bunch of rather trivial patches against master to fix\nportability issues, mostly shell ones. FWIW I did not test the\npatches other than ensuring the set of testsuite errors remains\nthe same, and that I only did on one system.\n\nAll of the issues are ones that are encountered on some real\nsystem, most are documented either in POSIX or in the shell\nportability section of the Autoconf manual.\n\nCheers,\nRalf\n"},{"id":"58579","messageId":"20071106201707.GB6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"[PATCH 1/5] Avoid a few unportable, needlessly nested \"...`...\".","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:17:08Z","receivedAt":"2007-11-06T20:17:08Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"Some shells wrongly interpret double-quote backquote double-quote\nnesting. OTOH, the right hand side of an assignment is never split\nin words, so the outer double quotes are needless there.\n---\n git-rebase--interactive.sh | 2 +-\n git-request-pull.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 76dc679..ecc6778 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -391,7 +391,7 @@ do\n \t-s|--strategy)\n \t\tcase \"$#,$1\" in\n \t\t*,*=*)\n-\t\t\tSTRATEGY=\"-s `expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\" ;;\n+\t\t\tSTRATEGY=\"-s \"`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'` ;;\n \t\t1,*)\n \t\t\tusage ;;\n \t\t*)\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex a992430..ec367d7 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -24,7 +24,7 @@ headrev=`git rev-parse --verify \"$head\"^0` || exit\n merge_base=`git merge-base $baserev $headrev` ||\n die \"fatal: No commits in common between $base and $head\"\n \n-url=\"`get_remote_url \"$url\"`\"\n+url=`get_remote_url \"$url\"`\n branch=`git peek-remote \"$url\" \\\n \t| sed -n -e \"/^$headrev\trefs.heads./{\n \t\ts/^.*\trefs.heads.//\n-- \n1.5.3.5.561.g140d\n"},{"id":"58580","messageId":"20071106201737.GC6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"[PATCH 2/5] Fix sed script to work with AIX sed.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:17:37Z","receivedAt":"2007-11-06T20:17:37Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"\\n is not portable in a s/// replacement string, only\nin the regex part. backslash-newline helps.\n---\n git-bisect.sh | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex b74f44d..1ed44e5 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -275,7 +275,8 @@ exit_if_skipped_commits () {\n \tif expr \"$_tried\" : \".*[|].*\" > /dev/null ; then\n \t\techo \"There are only 'skip'ped commit left to test.\"\n \t\techo \"The first bad commit could be any of:\"\n-\t\techo \"$_tried\" | sed -e 's/[|]/\\n/g'\n+\t\techo \"$_tried\" | sed -e 's/[|]/\\\n+/g'\n \t\techo \"We cannot bisect more!\"\n \t\texit 2\n \tfi\n-- \n1.5.3.5.561.g140d\n"},{"id":"58581","messageId":"20071106201809.GD6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"Re: [PATCH 3/5] Replace $((...)) with expr invocations.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:18:09Z","receivedAt":"2007-11-06T20:18:09Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"---\n git-filter-branch.sh | 4 ++--\n git-rebase--interactive.sh | 8 ++++----\n git-rebase.sh | 8 ++++----\n 3 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex ffcc408..2d5c247 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -253,7 +253,7 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n \n i=0\n while read commit parents; do\n-\ti=$(($i+1))\n+\ti=$(expr $i + 1)\n \tprintf \"\\rRewrite $commit ($i/$commits)\"\n \n \tcase \"$filter_subdir\" in\n@@ -374,7 +374,7 @@ do\n \t;;\n \tesac\n \tgit update-ref -m \"filter-branch: backup\" \"$orig_namespace$ref\" $sha1\n-\tcount=$(($count+1))\n+\tcount=$(expr $count + 1)\n done < \"$tempdir\"/heads\n \n # TODO: This should possibly go, with the semantics that all positive given\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex ecc6778..da48aaf 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -71,8 +71,8 @@ mark_action_done () {\n \tsed -e 1q < \"$TODO\" >> \"$DONE\"\n \tsed -e 1d < \"$TODO\" >> \"$TODO\".new\n \tmv -f \"$TODO\".new \"$TODO\"\n-\tcount=$(($(grep -ve '^$' -e '^#' < \"$DONE\" | wc -l)))\n-\ttotal=$(($count+$(grep -ve '^$' -e '^#' < \"$TODO\" | wc -l)))\n+\tcount=$(grep -ve '^$' -e '^#' < \"$DONE\" | wc -l)\n+\ttotal=$(expr $count + $(grep -ve '^$' -e '^#' < \"$TODO\" | wc -l))\n \tprintf \"Rebasing (%d/%d)\\r\" $count $total\n \ttest -z \"$VERBOSE\" || echo\n }\n@@ -205,8 +205,8 @@ nth_string () {\n \n make_squash_message () {\n \tif test -f \"$SQUASH_MSG\"; then\n-\t\tCOUNT=$(($(sed -n \"s/^# This is [^0-9]*\\([1-9][0-9]*\\).*/\\1/p\" \\\n-\t\t\t< \"$SQUASH_MSG\" | tail -n 1)+1))\n+\t\tCOUNT=$(expr $(sed -n \"s/^# This is [^0-9]*\\([1-9][0-9]*\\).*/\\1/p\" \\\n+\t\t\t< \"$SQUASH_MSG\" | tail -n 1) + 1)\n \t\techo \"# This is a combination of $COUNT commits.\"\n \t\tsed -n \"2,\\$p\" < \"$SQUASH_MSG\"\n \telse\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 224cca9..daa347a 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -79,7 +79,7 @@ continue_merge () {\n \techo \"$prev_head\" > \"$dotest/prev_head\"\n \n \t# onto the next patch:\n-\tmsgnum=$(($msgnum + 1))\n+\tmsgnum=$(expr $msgnum + 1)\n \techo \"$msgnum\" >\"$dotest/msgnum\"\n }\n \n@@ -90,7 +90,7 @@ call_merge () {\n \tcmt_name=$(git symbolic-ref HEAD)\n \tmsgnum=$(cat \"$dotest/msgnum\")\n \tend=$(cat \"$dotest/end\")\n-\teval GITHEAD_$cmt='\"${cmt_name##refs/heads/}~$(($end - $msgnum))\"'\n+\teval GITHEAD_$cmt='\"${cmt_name##refs/heads/}~$(expr $end - $msgnum)\"'\n \teval GITHEAD_$hd='$(cat \"$dotest/onto_name\")'\n \texport GITHEAD_$cmt GITHEAD_$hd\n \tgit-merge-$strategy \"$cmt^\" -- \"$hd\" \"$cmt\"\n@@ -163,7 +163,7 @@ do\n \t\t\tprev_head=$(cat \"$dotest/prev_head\")\n \t\t\tend=$(cat \"$dotest/end\")\n \t\t\tmsgnum=$(cat \"$dotest/msgnum\")\n-\t\t\tmsgnum=$(($msgnum + 1))\n+\t\t\tmsgnum=$(expr $msgnum + 1)\n \t\t\tonto=$(cat \"$dotest/onto\")\n \t\t\twhile test \"$msgnum\" -le \"$end\"\n \t\t\tdo\n@@ -349,7 +349,7 @@ echo \"$prev_head\" > \"$dotest/prev_head\"\n msgnum=0\n for cmt in `git rev-list --reverse --no-merges \"$upstream\"..ORIG_HEAD`\n do\n-\tmsgnum=$(($msgnum + 1))\n+\tmsgnum=$(expr $msgnum + 1)\n \techo \"$cmt\" > \"$dotest/cmt.$msgnum\"\n done\n \n-- \n1.5.3.5.561.g140d\n"},{"id":"58582","messageId":"20071106202002.GE6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"[PATCH 4/5] Fix sed string regex escaping in module_name.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:20:02Z","receivedAt":"2007-11-06T20:20:02Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"When escaping a string to be used as a sed regex, it is important\nto only escape active characters. Escaping other characters is\nundefined according to POSIX, and may lead to problems with some\nsed's extensions.\n---\nWith this one, I'm not sure whether it is necessary, simply because I\ndon't know what kinds of characters can be part of a submodule path.\nIt anyway should be safe though.\n\n git-submodule.sh | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 4aaaaab..5af28ec 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -73,7 +73,7 @@ resolve_relative_url ()\n module_name()\n {\n \t# Do we have \"submodule..path = $1\" defined in .gitmodules file?\n-\tre=$(printf '%s' \"$1\" | sed -e 's/\\([^a-zA-Z0-9_]\\)/\\\\\\1/g')\n+\tre=$(printf '%s' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n \tname=$( GIT_CONFIG=.gitmodules \\\n \t\tgit config --get-regexp '^submodule\\..*\\.path$' |\n \t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-- \n1.5.3.5.561.g140d\n"},{"id":"58583","messageId":"20071106202035.GF6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"[PATCH 5/5] Avoid \"test -o\" and \"test -a\" which are not POSIX, only XSI.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:20:35Z","receivedAt":"2007-11-06T20:20:35Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"---\n git-clone.sh | 2 +-\n git-commit.sh | 4 ++--\n git-merge-stupid.sh | 2 +-\n git-merge.sh | 4 ++--\n git-mergetool.sh | 2 +-\n 5 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-clone.sh b/git-clone.sh\nindex 3f00693..1ae137a 100755\n--- a/git-clone.sh\n+++ b/git-clone.sh\n@@ -462,7 +462,7 @@ then\n \n \tcase \"$no_checkout\" in\n \t'')\n-\t\ttest \"z$quiet\" = z -a \"z$no_progress\" = z && v=-v || v=\n+\t\ttest \"z$quiet\" = z && test \"z$no_progress\" = z && v=-v || v=\n \t\tgit read-tree -m -u $v HEAD HEAD\n \tesac\n fi\ndiff --git a/git-commit.sh b/git-commit.sh\nindex fcb8443..78fe518 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -50,7 +50,7 @@ run_status () {\n \t\texport GIT_INDEX_FILE\n \tfi\n \n-\tif test \"$status_only\" = \"t\" -o \"$use_status_color\" = \"t\"; then\n+\tif test \"$status_only\" = \"t\" || test \"$use_status_color\" = \"t\"; then\n \t\tcolor=\n \telse\n \t\tcolor=--nocolor\n@@ -290,7 +290,7 @@ t,,[1-9]*)\n \tdie \"No paths with -i does not make sense.\" ;;\n esac\n \n-if test ! -z \"$templatefile\" -a -z \"$log_given\"\n+if test ! -z \"$templatefile\" && test -z \"$log_given\"\n then\n \tif test ! -f \"$templatefile\"\n \tthen\ndiff --git a/git-merge-stupid.sh b/git-merge-stupid.sh\nindex f612d47..95adbb3 100755\n--- a/git-merge-stupid.sh\n+++ b/git-merge-stupid.sh\n@@ -44,7 +44,7 @@ case \"$bases\" in\n \t\t\t 2>/dev/null ||\tcontinue\n \t\t# Count the paths that are unmerged.\n \t\tcnt=`GIT_INDEX_FILE=$G git ls-files --unmerged | wc -l`\n-\t\tif test $best_cnt -le 0 -o $cnt -le $best_cnt\n+\t\tif test $best_cnt -le 0 || test $cnt -le $best_cnt\n \t\tthen\n \t\t\tbest=$c\n \t\t\tbest_cnt=$cnt\ndiff --git a/git-merge.sh b/git-merge.sh\nindex c2092a2..b6a35d4 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -124,7 +124,7 @@ merge_name () {\n \t\tgit show-ref -q --verify \"refs/heads/$truname\" 2>/dev/null\n \tthen\n \t\techo \"$rh\t\tbranch '$truname' (early part) of .\"\n-\telif test \"$remote\" = \"FETCH_HEAD\" -a -r \"$GIT_DIR/FETCH_HEAD\"\n+\telif test \"$remote\" = \"FETCH_HEAD\" && test -r \"$GIT_DIR/FETCH_HEAD\"\n \tthen\n \t\tsed -e 's/\tnot-for-merge\t/\t\t/' -e 1q \\\n \t\t\t\"$GIT_DIR/FETCH_HEAD\"\n@@ -478,7 +478,7 @@ do\n \t\tgit diff-files --name-only\n \t\tgit ls-files --unmerged\n \t } | wc -l`\n-\t if test $best_cnt -le 0 -o $cnt -le $best_cnt\n+\t if test $best_cnt -le 0 || test $cnt -le $best_cnt\n \t then\n \t\tbest_strategy=$strategy\n \t\tbest_cnt=$cnt\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex a68b403..9f7386b 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -167,7 +167,7 @@ merge_file () {\n local_present && git cat-file blob \":2:$prefix$path\" >\"$LOCAL\" 2>/dev/null\n remote_present && git cat-file blob \":3:$prefix$path\" >\"$REMOTE\" 2>/dev/null\n \n- if test -z \"$local_mode\" -o -z \"$remote_mode\"; then\n+ if test -z \"$local_mode\" || test -z \"$remote_mode\"; then\n \techo \"Deleted merge conflict for '$path':\"\n \tdescribe_file \"$local_mode\" \"local\" \"$LOCAL\"\n \tdescribe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n-- \n1.5.3.5.561.g140d\n"},{"id":"58585","messageId":"20071106202600.GH6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"20071106201809.GD6361@ins.uni-bonn.de","subject":"Re: [PATCH 3/5] Replace $((...)) with expr invocations.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T20:26:00Z","receivedAt":"2007-11-06T20:26:00Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"* Ralf Wildenhues wrote on Tue, Nov 06, 2007 at 09:18:09PM CET:\n> ---\n> git-filter-branch.sh | 4 ++--\n> git-rebase--interactive.sh | 8 ++++----\n> git-rebase.sh | 8 ++++----\n> 3 files changed, 10 insertions(+), 10 deletions(-)\n\nHmm, maybe this one is overkill. $((...)) is POSIX, I temporarily\nforgot (thanks Benoît!).\n\nI'm unsure whether git targets non-POSIX Bourne shells like Solaris\n/bin/sh. That would however mean replacing stuff like $(cmd) with\n`cmd` as well, and from grepping the source it looks like you'd rather\navoid that.\n\nCheers,\nRalf\n"},{"id":"58587","messageId":"7v8x5bgl04.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"20071106201518.GA6361@ins.uni-bonn.de","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-06T20:46:35Z","receivedAt":"2007-11-06T20:46:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"All missing Signed-off-by: lines.\n\n[1/5] In addition to take advantage of the fact that the RHS of\n assignment is not split, I'd prefer replacing `` with $()\n with these cases.\t Much easier to read if your shell\n supports it (and all the modern ones do).\n\n[2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n would want to get Yays from people with non GNU sed. Is\n busybox sed good enough to grok our scripts these days?\n Please ask help and collect Acks at least from folks on\n Solaris, MacOS, FBSD, and OBSD.\n\n[3/5] Arithmetic expansion. Have you caught _all_ of them, or\n is this patch about only the ones you noticed?\n\n We used to have expr all over the place as I was one of\n the primary authors of our shell scripts, and I am\n \"80-ish\" old fashioned. There was a long discussion on\n scripts in the past and we ruled that $(( ... )) is easier\n to read and supported widely enough to be acceptable.\n\n This patch goes backwards. Will drop, unless you can\n demonstrate that an implementation does not support it and\n convince people that the implementation is important.\n /bin/sh on Solaris does not count as you can configure\n SHELL_PATH to point at xpg4 shell or ksh on that platform.\n\n[4/5] I wonder if use of fgrep would be easier to read and more\n portable with this one:\n\n\tname=$( GIT_CONFIG=.gitmodules \\\n\t\tgit config --get-regexp '^submodule\\..*\\.path$' |\n\t\tfgrep \"submodule.$1.path\" |\n\t\tsed -e 's/^submodule\\.\\(.*\\)\\.path$/\\1/'\n\t)\n\n[5/5] Again, have you covered all of them? I am not opposed to\n this one, although I am a bit curious who lacks -a/-o in\n practice.\n"},{"id":"58588","messageId":"20071106210210.GA32159@glandium.org","threadId":"10687","inReplyTo":"7v8x5bgl04.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-11-06T21:02:10Z","receivedAt":"2007-11-06T21:02:10Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Tue, Nov 06, 2007 at 12:46:35PM -0800, Junio C Hamano wrote:\n> [5/5] Again, have you covered all of them? I am not opposed to\n> this one, although I am a bit curious who lacks -a/-o in\n> practice.\n\nSolaris's /bin/sh, but it already doesn't support $() and other stuff\nused all over the place in git, so it's not like it's changing anything.\n\nMaybe some other obscure old crappy shell ?\n\nMike\n"},{"id":"58589","messageId":"7vtznzf5jb.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"20071106202600.GH6361@ins.uni-bonn.de","subject":"Re: [PATCH 3/5] Replace $((...)) with expr invocations.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-06T21:06:00Z","receivedAt":"2007-11-06T21:06:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Wildenhues writes:\n\n> * Ralf Wildenhues wrote on Tue, Nov 06, 2007 at 09:18:09PM CET:\n>> ---\n>> git-filter-branch.sh | 4 ++--\n>> git-rebase--interactive.sh | 8 ++++----\n>> git-rebase.sh | 8 ++++----\n>> 3 files changed, 10 insertions(+), 10 deletions(-)\n>\n> Hmm, maybe this one is overkill. $((...)) is POSIX, I temporarily\n> forgot (thanks Benoît!).\n>\n> I'm unsure whether git targets non-POSIX Bourne shells like Solaris\n> /bin/sh. That would however mean replacing stuff like $(cmd) with\n> `cmd` as well, and from grepping the source it looks like you'd rather\n> avoid that.\n\nFor git, two rough rules are:\n\n - Most importantly, we never say \"It's in POSIX; we'll happily\n screw your system that does not conform.\" We live in the\n real world.\n\n - However, we often say \"Let's stay away from that construct,\n it's not even in POSIX\".\n\nFor shell scripts specifically (not exhaustive):\n\n - We prefer $( ... ) for command substitution; unlike ``, it\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\n\n - We use ${parameter-word} and its [-=?+] siblings, and their\n colon'ed \"unset or null\" form.\n\n - We use ${parameter#word} and its [#%] siblings, and their\n doubled \"longest matching\" form.\n\n - We use Arithmetic Expansion $(( ... )).\n\n - No \"Substring Expansion\" ${parameter:offset:length}.\n\n - No shell arrays.\n\n - No strlen ${#parameter}.\n\n - No regexp ${parameter/pattern/string}.\n\n - We do not use Process Substitution <(list) or >(list).\n\n - We prefer \"test\" over \"[ ... ]\".\n\n - We do not write noiseword \"function\" in front of shell\n functions.\n"},{"id":"58592","messageId":"20071106210925.GJ6361@ins.uni-bonn.de","threadId":"10687","inReplyTo":"7v8x5bgl04.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-06T21:09:25Z","receivedAt":"2007-11-06T21:09:25Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"Hello Junio,\n\n* Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:\n> All missing Signed-off-by: lines.\n\nOops. Sorry.\n\n> [1/5] In addition to take advantage of the fact that the RHS of\n> assignment is not split, I'd prefer replacing `` with $()\n> with these cases.\t Much easier to read if your shell\n> supports it (and all the modern ones do).\n\nOK.\n\n> [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> would want to get Yays from people with non GNU sed. Is\n> busybox sed good enough to grok our scripts these days?\n> Please ask help and collect Acks at least from folks on\n> Solaris, MacOS, FBSD, and OBSD.\n\nFWIW, I have little experience with busybox sed, but for the others here\nyou go: With\n\necho axbyc | sed 's,x,\\n,; s,y,\\\n,'\n\nI get on OpenBSD, FreeBSD, Solaris, and Darwin (minus indentation):\n anb\n c\n\nGNU sed gives\n a\n b\n c\n\n> [3/5] Arithmetic expansion. Have you caught _all_ of them, or\n> is this patch about only the ones you noticed?\n\nI have grepped *.sh. But let's drop that, I see that it goes backwards.\n\n> [4/5] I wonder if use of fgrep would be easier to read and more\n> portable with this one:\n> \n> \tname=$( GIT_CONFIG=.gitmodules \\\n> \t\tgit config --get-regexp '^submodule\\..*\\.path$' |\n> \t\tfgrep \"submodule.$1.path\" |\n> \t\tsed -e 's/^submodule\\.\\(.*\\)\\.path$/\\1/'\n> \t)\n\nCertainly easier to read. But fgrep itself is not portable (it could be\ngrep -F). Also, isn't the $1 to be matched at the end, after a \"=\"\nhere? FWIW the pattern I posted has survived a few years in Automake,\nso there is some hope that it works.\n\n> [5/5] Again, have you covered all of them?\n\nNo, oops again. As I searched for `test.*-[oa]' I have missed line\nwraps and [ ... -o ... ].\n\n> I am not opposed to\n> this one, although I am a bit curious who lacks -a/-o in\n> practice.\n\nHmm, good question. I actually don't know whether there is a shell\nthat isn't ruled out by $() anyway. Let's drop that one, too, then.\n\nCheers,\nRalf\n"},{"id":"58610","messageId":"Pine.LNX.4.64.0711062317330.4362@racer.site","threadId":"10687","inReplyTo":"7vtznzf5jb.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Add Documentation/CodingStyle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-06T23:17:50Z","receivedAt":"2007-11-06T23:17:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven if our code is quite a good documentation for our coding style,\nsome people seem to prefer a document describing it.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tI long resisted in adding this, as I really believe our code\n\tbase is a very clean one, and a good description of what we\n\tprefer.\n\n\tBut it seems that not everybody has the time to study our\n\tcode in depth, beautiful as it is ;-)\n\n\tBTW the first to catch the allusion to a certain movie \n\twins a drink with me.\n\n Documentation/CodingStyle | 87 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 87 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/CodingStyle\n\ndiff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\nnew file mode 100644\nindex 0000000..622b80b\n--- /dev/null\n+++ b/Documentation/CodingStyle\n@@ -0,0 +1,87 @@\n+As a popular project, we also have some guidelines to keep to the\n+code. For git in general, two rough rules are:\n+\n+ - Most importantly, we never say \"It's in POSIX; we'll happily\n+ screw your system that does not conform.\" We live in the\n+ real world.\n+\n+ - However, we often say \"Let's stay away from that construct,\n+ it's not even in POSIX\".\n+\n+As for more concrete guidelines, just imitate the existing code\n+(this is a good guideline, no matter which project you are contributing\n+to...). But if you must have some list of rules, here they are.\n+\n+For shell scripts specifically (not exhaustive):\n+\n+ - We prefer $( ... ) for command substitution; unlike ``, it\n+ properly nests. It should have been the way Bourne spelled\n+ it from day one, but unfortunately isn't.\n+\n+ - We use ${parameter-word} and its [-=?+] siblings, and their\n+ colon'ed \"unset or null\" form.\n+\n+ - We use ${parameter#word} and its [#%] siblings, and their\n+ doubled \"longest matching\" form.\n+\n+ - We use Arithmetic Expansion $(( ... )).\n+\n+ - No \"Substring Expansion\" ${parameter:offset:length}.\n+\n+ - No shell arrays.\n+\n+ - No strlen ${#parameter}.\n+\n+ - No regexp ${parameter/pattern/string}.\n+\n+ - We do not use Process Substitution <(list) or >(list).\n+\n+ - We prefer \"test\" over \"[ ... ]\".\n+\n+ - We do not write noiseword \"function\" in front of shell\n+ functions.\n+\n+For C programs:\n+\n+ - Use tabs to increment, and interpret tabs as taking up to 8 spaces\n+\n+ - Try to keep to 80 characters per line\n+\n+ - When declaring pointers, the star sides with the variable name, i.e.\n+ \"char *string\", not \"char* string\" or \"char * string\". This makes\n+ it easier to understand \"char *string, c;\"\n+\n+ - Do not use curly brackets unnecessarily. I.e.\n+\n+\tif (bla) {\n+\t\tx = 1;\n+\t}\n+\n+ is frowned upon. A gray area is when the statement extends over a\n+ few lines, and/or you have a lengthy comment atop of it.\n+\n+ - Try to make your code understandable. You may put comments in, but\n+ comments invariably tend to stale out when the code they were\n+ describing changes. Often splitting a function into two makes the\n+ intention of the code much clearer.\n+\n+ Double negation is often harder to understand than no negation at\n+ all.\n+\n+ Some clever tricks, like using the !! operator with arithmetic\n+ constructs, can be extremely confusing to others. Avoid them,\n+ unless there is a compelling reason to use them.\n+\n+ - Use the API. No, really. We have a strbuf (variable length string),\n+ several arrays with the ALLOC_GROW() macro, a path_list for sorted\n+ string lists, a hash map (mapping struct objects) named\n+ \"struct decorate\", amongst other things.\n+\n+ - #include system headers in git-compat-util.h. Some headers on some\n+ systems show subtle breakages when you change the order, so it is\n+ best to keep them in one place.\n+\n+ - if you are planning a new command, consider writing it in shell or\n+ perl first, so that changes in semantics can be easily changed and\n+ discussed. Many git commands started out like that, and a few are\n+ still scripts.\n-- \n1.5.3.5.1597.g7191\n"},{"id":"58612","messageId":"Pine.LNX.4.64.0711062324590.4362@racer.site","threadId":"10687","inReplyTo":"20071106210210.GA32159@glandium.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-06T23:25:35Z","receivedAt":"2007-11-06T23:25:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2007, Mike Hommey wrote:\n\n> On Tue, Nov 06, 2007 at 12:46:35PM -0800, Junio C Hamano wrote:\n> > [5/5] Again, have you covered all of them? I am not opposed to\n> > this one, although I am a bit curious who lacks -a/-o in\n> > practice.\n> \n> Solaris's /bin/sh, but it already doesn't support $() and other stuff\n> used all over the place in git, so it's not like it's changing anything.\n> \n> Maybe some other obscure old crappy shell ?\n\nAs Junio commented in the part you did not quote, there are better shells \nin Solaris. Use those.\n\nCiao,\nDscho\n"},{"id":"58620","messageId":"47310104.5040202@op5.se","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062317330.4362@racer.site","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-07T00:04:20Z","receivedAt":"2007-11-07T00:04:20Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Even if our code is quite a good documentation for our coding style,\n> some people seem to prefer a document describing it.\n> \n\nSweet. You just saved me 40 minutes of writing one just like that for\ncompany use. I owe you a drink, and then you can tell me what movie\nyou alluded to ;-)\n\n> +\n> + - if you are planning a new command, consider writing it in shell or\n> + perl first, so that changes in semantics can be easily changed and\n> + discussed. Many git commands started out like that, and a few are\n> + still scripts.\n\nI'd skip this part though and just add a pointer to contrib/examples/,\nsaying something along the lines of\n\n- if you're planning a new command, sneak a peak in contrib/examples/\n for ample study-material of retired git commands implemented in perl\n and shell.\n\nPossibly with s/retired // on that paragraph.\n\nThere's nothing particular wrong with writing in C from the start after\nall.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"58623","messageId":"7v640ega5q.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062317330.4362@racer.site","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-07T00:40:49Z","receivedAt":"2007-11-07T00:40:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\n> new file mode 100644\n> index 0000000..622b80b\n> --- /dev/null\n> +++ b/Documentation/CodingStyle\n> @@ -0,0 +1,87 @@\n> +As a popular project, we also have some guidelines to keep to the\n> +code. For git in general, two rough rules are:\n> +\n> + - Most importantly, we never say \"It's in POSIX; we'll happily\n> + screw your system that does not conform.\" We live in the\n> + real world.\n> +\n> + - However, we often say \"Let's stay away from that construct,\n> + it's not even in POSIX\".\n> +\n\nI am not sure if we want to have CodingStyle document, but the\nabove are not CodingStyle issues.\n\nIf we are to write this down, I'd like to have the more\nimportant third rule, which is:\n\n - In spite of the above two rules, we sometimes say \"Although\n this is not in POSIX, it (is so convenient | makes the code\n much more readable | has other good characteristics) and\n practically all the platforms we care about support it, so\n let's use it\". Again, we live in the real world, and it is\n sometimes a judgement call, decided based more on real world\n constraints people face than what the paper standard says.\n\n> +For C programs:\n> +\n> + - Use tabs to increment, and interpret tabs as taking up to 8 spaces\n\nWhat's the character for decrement? DEL? ;-)\n\n> + Double negation is often harder to understand than no negation at\n> + all.\n> +\n> + Some clever tricks, like using the !! operator with arithmetic\n> + constructs, can be extremely confusing to others. Avoid them,\n> + unless there is a compelling reason to use them.\n\nI actually think (!!var) idiom is already established in our\ncodebase.\n\n> + - Use the API. No, really. We have a strbuf (variable length string),\n> + several arrays with the ALLOC_GROW() macro, a path_list for sorted\n> + string lists, a hash map (mapping struct objects) named\n> + \"struct decorate\", amongst other things.\n\n - When you come up with an API, document it.\n\n> + - if you are planning a new command, consider writing it in shell or\n> + perl first, so that changes in semantics can be easily changed and\n> + discussed. Many git commands started out like that, and a few are\n> + still scripts.\n\nNo Python allowed?\n"},{"id":"58643","messageId":"4749D87D-9660-472B-97CF-8E649435AFD7@wincent.com","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062317330.4362@racer.site","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-07T07:53:06Z","receivedAt":"2007-11-07T07:53:06Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 7/11/2007, a las 0:17, Johannes Schindelin escribió:\n\n> Even if our code is quite a good documentation for our coding style,\n> some people seem to prefer a document describing it.\n\nGreat idea, Johannes, especially your nice concise summary of \nconventions for shell-scripts (which is an area where we most often \nsee list traffic on which way to write things).\n\nCheers,\nWincent\n"},{"id":"58653","messageId":"47317CD7.5040506@op5.se","threadId":"10687","inReplyTo":"7v640ega5q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-07T08:52:39Z","receivedAt":"2007-11-07T08:52:39Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Johannes Schindelin writes:\n> \n>> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\n>> new file mode 100644\n>> index 0000000..622b80b\n>> --- /dev/null\n>> +++ b/Documentation/CodingStyle\n>> @@ -0,0 +1,87 @@\n>> +As a popular project, we also have some guidelines to keep to the\n>> +code. For git in general, two rough rules are:\n>> +\n>> + - Most importantly, we never say \"It's in POSIX; we'll happily\n>> + screw your system that does not conform.\" We live in the\n>> + real world.\n>> +\n>> + - However, we often say \"Let's stay away from that construct,\n>> + it's not even in POSIX\".\n>> +\n> \n> I am not sure if we want to have CodingStyle document, but the\n> above are not CodingStyle issues.\n> \n> If we are to write this down, I'd like to have the more\n> important third rule, which is:\n> \n> - In spite of the above two rules, we sometimes say \"Although\n> this is not in POSIX, it (is so convenient | makes the code\n> much more readable | has other good characteristics) and\n> practically all the platforms we care about support it, so\n> let's use it\". Again, we live in the real world, and it is\n> sometimes a judgement call, decided based more on real world\n> constraints people face than what the paper standard says.\n> \n>> +For C programs:\n>> +\n>> + - Use tabs to increment, and interpret tabs as taking up to 8 spaces\n> \n> What's the character for decrement? DEL? ;-)\n> \n>> + Double negation is often harder to understand than no negation at\n>> + all.\n>> +\n>> + Some clever tricks, like using the !! operator with arithmetic\n>> + constructs, can be extremely confusing to others. Avoid them,\n>> + unless there is a compelling reason to use them.\n> \n> I actually think (!!var) idiom is already established in our\n> codebase.\n> \n\nNot very widely for arithmetic expressions though. I believe this\nalludes to the (!!a + !!b + !!c) idiom used earlier, where it's not\nexactly clear *why* a, b and c can be > 1 if != 0 is the only thing\nwe care about.\n\n>> + - Use the API. No, really. We have a strbuf (variable length string),\n>> + several arrays with the ALLOC_GROW() macro, a path_list for sorted\n>> + string lists, a hash map (mapping struct objects) named\n>> + \"struct decorate\", amongst other things.\n> \n> - When you come up with an API, document it.\n> \n>> + - if you are planning a new command, consider writing it in shell or\n>> + perl first, so that changes in semantics can be easily changed and\n>> + discussed. Many git commands started out like that, and a few are\n>> + still scripts.\n> \n> No Python allowed?\n\n\nPerhaps with this addendum?\n\n- Think very, very hard before introducing a new dependency into git. This\n means you should stay away from scripting languages not already used in\n the git core command set unless your command is clearly separate from it,\n such as an importer to convert random-scm-X repositories to git.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"58654","messageId":"47317D02.3050703@op5.se","threadId":"10687","inReplyTo":"4749D87D-9660-472B-97CF-8E649435AFD7@wincent.com","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-07T08:53:22Z","receivedAt":"2007-11-07T08:53:22Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Wincent Colaiuta wrote:\n> El 7/11/2007, a las 0:17, Johannes Schindelin escribió:\n> \n>> Even if our code is quite a good documentation for our coding style,\n>> some people seem to prefer a document describing it.\n> \n> Great idea, Johannes, especially your nice concise summary of \n> conventions for shell-scripts (which is an area where we most often see \n> list traffic on which way to write things).\n> \n\nThat was ripped from a mail Junio sent to the list though ;-)\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"58673","messageId":"e2b179460711070617h7e9af64egcde5122808a4d924@mail.gmail.com","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062324590.4362@racer.site","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2007-11-07T14:17:22Z","receivedAt":"2007-11-07T14:17:22Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:\n> [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> would want to get Yays from people with non GNU sed. Is\n> busybox sed good enough to grok our scripts these days?\n> Please ask help and collect Acks at least from folks on\n> Solaris, MacOS, FBSD, and OBSD.\n\nOn Nov 6, 2007 11:25 PM, Johannes Schindelin wrote:\n> As Junio commented in the part you did not quote, there are better shells\n> in Solaris. Use those.\n\nEqually GNU sed is available as a drop-in rpm for AIX. I wonder if it\nwould be worth adding\nMakefile support for a PATH prefix for the git scripts, so they could\nprepend (in this case)\nsomething like /opt/freeware/bin or /usr/linux/bin ?\n\nIn our AIX environment many GNU tools are installed but I can't\nguarantee they come first\nin the paths of the git users.\n\nI'm willing to work up a patch if there's any interest.\n\nMike\n"},{"id":"58675","messageId":"Pine.LNX.4.64.0711071446190.4362@racer.site","threadId":"10687","inReplyTo":"e2b179460711070617h7e9af64egcde5122808a4d924@mail.gmail.com","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T14:47:29Z","receivedAt":"2007-11-07T14:47:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2007, Mike Ralphson wrote:\n\n> Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:\n> > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> > would want to get Yays from people with non GNU sed. Is\n> > busybox sed good enough to grok our scripts these days?\n> > Please ask help and collect Acks at least from folks on\n> > Solaris, MacOS, FBSD, and OBSD.\n> \n> On Nov 6, 2007 11:25 PM, Johannes Schindelin wrote:\n> > As Junio commented in the part you did not quote, there are better shells\n> > in Solaris. Use those.\n> \n> Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it\n> would be worth adding\n> Makefile support for a PATH prefix for the git scripts, so they could\n> prepend (in this case)\n> something like /opt/freeware/bin or /usr/linux/bin ?\n> \n> In our AIX environment many GNU tools are installed but I can't\n> guarantee they come first\n> in the paths of the git users.\n> \n> I'm willing to work up a patch if there's any interest.\n\nWould that be a task for configure? Because I am not sure if the GNU \ntools are installed in the same place on all AIX boxen...\n\nCiao,\nDscho\n"},{"id":"58677","messageId":"Pine.LNX.4.64.0711071451010.4362@racer.site","threadId":"10687","inReplyTo":"7v640ega5q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T14:54:27Z","receivedAt":"2007-11-07T14:54:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\n> > new file mode 100644\n> > index 0000000..622b80b\n> > --- /dev/null\n> > +++ b/Documentation/CodingStyle\n> > @@ -0,0 +1,87 @@\n> > +As a popular project, we also have some guidelines to keep to the\n> > +code. For git in general, two rough rules are:\n> > +\n> > + - Most importantly, we never say \"It's in POSIX; we'll happily\n> > + screw your system that does not conform.\" We live in the\n> > + real world.\n> > +\n> > + - However, we often say \"Let's stay away from that construct,\n> > + it's not even in POSIX\".\n> > +\n> \n> I am not sure if we want to have CodingStyle document, but the\n> above are not CodingStyle issues.\n\nWould you like to call it CodingConventions?\n\n> If we are to write this down, I'd like to have the more\n> important third rule, which is:\n> \n> - In spite of the above two rules, we sometimes say \"Although\n> this is not in POSIX, it (is so convenient | makes the code\n> much more readable | has other good characteristics) and\n> practically all the platforms we care about support it, so\n> let's use it\". Again, we live in the real world, and it is\n> sometimes a judgement call, decided based more on real world\n> constraints people face than what the paper standard says.\n\nOkay, will add.\n\n> > +For C programs:\n> > +\n> > + - Use tabs to increment, and interpret tabs as taking up to 8 spaces\n> \n> What's the character for decrement? DEL? ;-)\n\nHehe. As you undoubtedly guessed, I meant \"indent\"...\n\n> > + Double negation is often harder to understand than no negation at\n> > + all.\n> > +\n> > + Some clever tricks, like using the !! operator with arithmetic\n> > + constructs, can be extremely confusing to others. Avoid them,\n> > + unless there is a compelling reason to use them.\n> \n> I actually think (!!var) idiom is already established in our\n> codebase.\n\nYep, but when there are easier variants, AFAICT they were preferred.\n\n> > + - Use the API. No, really. We have a strbuf (variable length string),\n> > + several arrays with the ALLOC_GROW() macro, a path_list for sorted\n> > + string lists, a hash map (mapping struct objects) named\n> > + \"struct decorate\", amongst other things.\n> \n> - When you come up with an API, document it.\n\nOkay.\n\n> > + - if you are planning a new command, consider writing it in shell or\n> > + perl first, so that changes in semantics can be easily changed and\n> > + discussed. Many git commands started out like that, and a few are\n> > + still scripts.\n> \n> No Python allowed?\n\nWell, maybe we should allow that again. Although I hoped we are over that \nnow, as it would complicate the msysGit efforts tremendously.\n\nCiao,\nDscho\n"},{"id":"58679","messageId":"Pine.LNX.4.64.0711071456440.4362@racer.site","threadId":"10687","inReplyTo":"47317CD7.5040506@op5.se","subject":"[PATCH v2] Add Documentation/CodingStyle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T14:59:52Z","receivedAt":"2007-11-07T14:59:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven if our code is quite a good documentation for our coding style,\nsome people seem to prefer a document describing it.\n\nThe part about the shell scripts is clearly just copied from one of\nJunio's helpful mails, and some parts were added from comments by\nJunio and Andreas Ericsson.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Wed, 7 Nov 2007, Andreas Ericsson wrote:\n\n\t> Perhaps with this addendum?\n\t> \n\t> - Think very, very hard before introducing a new dependency into git. This\n\t> means you should stay away from scripting languages not already used in\n\t> the git core command set unless your command is clearly separate from it,\n\t> such as an importer to convert random-scm-X repositories to git.\n\n\tI edited it minimally.\n\n Documentation/CodingStyle | 103 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 103 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/CodingStyle\n\ndiff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\nnew file mode 100644\nindex 0000000..38a3d9f\n--- /dev/null\n+++ b/Documentation/CodingStyle\n@@ -0,0 +1,103 @@\n+As a popular project, we also have some guidelines to keep to the\n+code. For git in general, two rough rules are:\n+\n+ - Most importantly, we never say \"It's in POSIX; we'll happily\n+ screw your system that does not conform.\" We live in the\n+ real world.\n+\n+ - However, we often say \"Let's stay away from that construct,\n+ it's not even in POSIX\".\n+\n+ - In spite of the above two rules, we sometimes say \"Although\n+ this is not in POSIX, it (is so convenient | makes the code\n+ much more readable | has other good characteristics) and\n+ practically all the platforms we care about support it, so\n+ let's use it\". Again, we live in the real world, and it is\n+ sometimes a judgement call, decided based more on real world\n+ constraints people face than what the paper standard says.\n+\n+\n+As for more concrete guidelines, just imitate the existing code\n+(this is a good guideline, no matter which project you are contributing\n+to...). But if you must have some list of rules, here they are.\n+\n+For shell scripts specifically (not exhaustive):\n+\n+ - We prefer $( ... ) for command substitution; unlike ``, it\n+ properly nests. It should have been the way Bourne spelled\n+ it from day one, but unfortunately isn't.\n+\n+ - We use ${parameter-word} and its [-=?+] siblings, and their\n+ colon'ed \"unset or null\" form.\n+\n+ - We use ${parameter#word} and its [#%] siblings, and their\n+ doubled \"longest matching\" form.\n+\n+ - We use Arithmetic Expansion $(( ... )).\n+\n+ - No \"Substring Expansion\" ${parameter:offset:length}.\n+\n+ - No shell arrays.\n+\n+ - No strlen ${#parameter}.\n+\n+ - No regexp ${parameter/pattern/string}.\n+\n+ - We do not use Process Substitution <(list) or >(list).\n+\n+ - We prefer \"test\" over \"[ ... ]\".\n+\n+ - We do not write noiseword \"function\" in front of shell\n+ functions.\n+\n+For C programs:\n+\n+ - Use tabs to indent, and interpret tabs as taking up to 8 spaces\n+\n+ - Try to keep to 80 characters per line\n+\n+ - When declaring pointers, the star sides with the variable name, i.e.\n+ \"char *string\", not \"char* string\" or \"char * string\". This makes\n+ it easier to understand \"char *string, c;\"\n+\n+ - Do not use curly brackets unnecessarily. I.e.\n+\n+\tif (bla) {\n+\t\tx = 1;\n+\t}\n+\n+ is frowned upon. A gray area is when the statement extends over a\n+ few lines, and/or you have a lengthy comment atop of it.\n+\n+ - Try to make your code understandable. You may put comments in, but\n+ comments invariably tend to stale out when the code they were\n+ describing changes. Often splitting a function into two makes the\n+ intention of the code much clearer.\n+\n+ Double negation is often harder to understand than no negation at\n+ all.\n+\n+ Some clever tricks, like using the !! operator with arithmetic\n+ constructs, can be extremely confusing to others. Avoid them,\n+ unless there is a compelling reason to use them.\n+\n+ - Use the API. No, really. We have a strbuf (variable length string),\n+ several arrays with the ALLOC_GROW() macro, a path_list for sorted\n+ string lists, a hash map (mapping struct objects) named\n+ \"struct decorate\", amongst other things.\n+\n+ - When you come up with an API, document it.\n+\n+ - #include system headers in git-compat-util.h. Some headers on some\n+ systems show subtle breakages when you change the order, so it is\n+ best to keep them in one place.\n+\n+ - if you are planning a new command, consider writing it in shell or\n+ perl first, so that changes in semantics can be easily changed and\n+ discussed. Many git commands started out like that, and a few are\n+ still scripts.\n+\n+ - Avoid introducing a new dependency into git. This means you usually\n+ should stay away from scripting languages not already used in the git\n+ core command set (unless your command is clearly separate from it,\n+ such as an importer to convert random-scm-X repositories to git).\n-- \n1.5.3.5.1597.g7191\n"},{"id":"58689","messageId":"e2b179460711070730w4ca95989y14872665ddc8bfca@mail.gmail.com","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711071446190.4362@racer.site","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2007-11-07T15:30:16Z","receivedAt":"2007-11-07T15:30:16Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"On Nov 7, 2007 2:47 PM, Johannes Schindelin wrote:\n> > Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it\n> > would be worth adding Makefile support for a PATH prefix for the git\n> > scripts, so they could prepend (in this case) something like\n> > /opt/freeware/bin or /usr/linux/bin ?\n> >\n> > In our AIX environment many GNU tools are installed but I can't\n> > guarantee they come first in the paths of the git users.\n> >\n> > I'm willing to work up a patch if there's any interest.\n>\n> Would that be a task for configure? Because I am not sure if the GNU\n> tools are installed in the same place on all AIX boxen...\n\nWell let's say the patch would arrive earlier if it was based on the\nshipped Makefile rather than the unholy abomination that is\nautoconf... If the GNU tools have been installed via the IBM AIX\nToolbox for Linux Applications[1] then they'll be installed in\n/opt/freeware/bin and /usr/linux/bin will be a set of symlinks to\nthem.\n\nThat said, there may be 32/64bit differences and of course anyone\ncould have rolled their own sed, awk, diff, patch, grep, sort etc in\n/usr/local/bin or anywhere else, and I'd guess this might be useful\nfor Solaris / HPUX users etc.\n\nI was thinking along the lines of the existing $SHELL_PATH, i.e. a\nbuild-time manually-set Makefile/environment variable. I'd also like\nto be able to override gitexecdir in the same way without having my\nbuilds marked dirty.\n\nCheers, Mike\n\n[1] http://www-03.ibm.com/systems/p/os/aix/linux/download.html\n"},{"id":"58691","messageId":"Pine.LNX.4.64.0711071536180.4362@racer.site","threadId":"10687","inReplyTo":"e2b179460711070730w4ca95989y14872665ddc8bfca@mail.gmail.com","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T15:37:01Z","receivedAt":"2007-11-07T15:37:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2007, Mike Ralphson wrote:\n\n> On Nov 7, 2007 2:47 PM, Johannes Schindelin wrote:\n>\n> > I am not sure if the GNU tools are installed in the same place on all \n> > AIX boxen...\n> \n> Well let's say the patch would arrive earlier if it was based on the\n> shipped Makefile rather than the unholy abomination that is\n> autoconf... If the GNU tools have been installed via the IBM AIX\n> Toolbox for Linux Applications[1] then they'll be installed in\n> /opt/freeware/bin and /usr/linux/bin will be a set of symlinks to\n> them.\n\nI guess Makefile is the better place, then. You are the expert on AIX.\n\nCiao,\nDscho\n"},{"id":"58694","messageId":"fcaeb9bf0711070758w5832ab83ic16e8fb4edb80972@mail.gmail.com","threadId":"10687","inReplyTo":"7v8x5bgl04.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2007-11-07T15:58:50Z","receivedAt":"2007-11-07T15:58:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 11/7/07, Junio C Hamano wrote:\n> [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> would want to get Yays from people with non GNU sed. Is\n> busybox sed good enough to grok our scripts these days?\n> Please ask help and collect Acks at least from folks on\n> Solaris, MacOS, FBSD, and OBSD.\n\nI haven't extensively used all the scripts. There seems to be no\nsed-related failure from git testsuite results in my git-box branch.\nSo I would say for now it's good enough.\n-- \nDuy\n"},{"id":"58695","messageId":"fcaeb9bf0711070805x49228d8dq7e8e6f26060c47ab@mail.gmail.com","threadId":"10687","inReplyTo":"fcaeb9bf0711070758w5832ab83ic16e8fb4edb80972@mail.gmail.com","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2007-11-07T16:05:35Z","receivedAt":"2007-11-07T16:05:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 11/7/07, Nguyen Thai Ngoc Duy wrote:\n> On 11/7/07, Junio C Hamano wrote:\n> > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> > would want to get Yays from people with non GNU sed. Is\n> > busybox sed good enough to grok our scripts these days?\n> > Please ask help and collect Acks at least from folks on\n> > Solaris, MacOS, FBSD, and OBSD.\n>\n> I haven't extensively used all the scripts. There seems to be no\n> sed-related failure from git testsuite results in my git-box branch.\n> So I would say for now it's good enough.\n\nArgh, should have made it clear, busybox sed is good enough.\n\n-- \nDuy\n"},{"id":"58710","messageId":"1194464406.14978.9.camel@ld0161-tx32","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062317330.4362@racer.site","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Jon Loeliger","fromEmail":"jdl@freescale.com","sentAt":"2007-11-07T19:40:06Z","receivedAt":"2007-11-07T19:40:06Z","isPatch":true,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"On Tue, 2007-11-06 at 17:17, Johannes Schindelin wrote:\n\n> +\n> + - Do not use curly brackets unnecessarily. I.e.\n> +\n> +\tif (bla) {\n> +\t\tx = 1;\n> +\t}\n\nIn my opinion, I think this is a bad guideline.\n\n> + is frowned upon. A gray area is when the statement extends over a\n> + few lines, and/or you have a lengthy comment atop of it.\n\nOr if it is some macro, or any number of vague problem areas.\n\nAgain, in my opinion, one should always take the safer\ndefensive programming tactic and always use braces.\nHaving them really never produces errors, while omitting\nthem is often error prone.\n\nYes, I know that is not a popular opinion by example,\nbut I'm still allowed to state it. :-)\nFeel free to ignore me as well. :-)\n\njdl\n"},{"id":"58714","messageId":"Pine.LNX.4.64.0711072013010.4362@racer.site","threadId":"10687","inReplyTo":"1194464406.14978.9.camel@ld0161-tx32","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T20:13:58Z","receivedAt":"2007-11-07T20:13:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2007, Jon Loeliger wrote:\n\n> On Tue, 2007-11-06 at 17:17, Johannes Schindelin wrote:\n> \n> > +\n> > + - Do not use curly brackets unnecessarily. I.e.\n> > +\n> > +\tif (bla) {\n> > +\t\tx = 1;\n> > +\t}\n> \n> In my opinion, I think this is a bad guideline.\n\nIn my opinion, this is a good guideline.\n\nSo now what? Let's have another pointless flamewar?\n\nCiao,\nDscho\n"},{"id":"58718","messageId":"7v3avhbxdq.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"fcaeb9bf0711070805x49228d8dq7e8e6f26060c47ab@mail.gmail.com","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-07T20:42:41Z","receivedAt":"2007-11-07T20:42:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nguyen Thai Ngoc Duy\" writes:\n\n> On 11/7/07, Nguyen Thai Ngoc Duy wrote:\n>> On 11/7/07, Junio C Hamano wrote:\n>> > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n>> > would want to get Yays from people with non GNU sed. Is\n>> > busybox sed good enough to grok our scripts these days?\n>> > Please ask help and collect Acks at least from folks on\n>> > Solaris, MacOS, FBSD, and OBSD.\n>>\n>> I haven't extensively used all the scripts. There seems to be no\n>> sed-related failure from git testsuite results in my git-box branch.\n>> So I would say for now it's good enough.\n>\n> Argh, should have made it clear, busybox sed is good enough.\n\nThanks. And you can also happy grok Ralf's rewritten construct,\nright?\n\nThat is, existing\n\n $ sed -e 's/foo/\\n/' file\n\nwill be rewritten by the patch [2/5] to\n\n $ sed -e 's/foo/\\\n /' file\n"},{"id":"58728","messageId":"200711072243.21086.robin.rosenberg.lists@dewire.com","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711071456440.4362@racer.site","subject":"Re: [PATCH v2] Add Documentation/CodingStyle","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-11-07T21:43:20Z","receivedAt":"2007-11-07T21:43:20Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 07 november 2007 skrev Johannes Schindelin:\n> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle\n> new file mode 100644\n> index 0000000..38a3d9f\n> --- /dev/null\n> +++ b/Documentation/CodingStyle\n> @@ -0,0 +1,103 @@\n> +As a popular project, we also have some guidelines to keep to the\n> +code. For git in general, two rough rules are:\n> +\n> + - Most importantly, we never say \"It's in POSIX; we'll happily\n> + screw your system that does not conform.\" We live in the\n> + real world.\n\nCan we use less offensive wording in documentation than we do on the list or IRC?\nI'm not hurt by it but it doesn't look serious.\n\n> + - Try to keep to 80 characters per line\nless than?\n\n> +\n> + - When declaring pointers, the star sides with the variable name, i.e.\n> + \"char *string\", not \"char* string\" or \"char * string\". This makes\n> + it easier to understand \"char *string, c;\"\n\nRationale: The C syntax is defined the way it is. C programmers should\nunderstand it.\n\nI'd amend: Don't mix different types in declarations.\n\nWrite:\n\tchar *string;\n\tchar c;\n\nRather than:\n\n\tchar *string,c;\n\n> + - Do not use curly brackets unnecessarily. I.e.\n> +\n> +\tif (bla) {\n> +\t\tx = 1;\n> +\t}\n> +\n> + is frowned upon. A gray area is when the statement extends over a\n> + few lines, and/or you have a lengthy comment atop of it.\n\nAvoid unnecessary curly brackets but use them if unsure. (This is less \nstrict than \"do not\". There are cases where we do want \"unnecessary\"\nbrackets.\n\n-- robin\n"},{"id":"58744","messageId":"Pine.LNX.4.64.0711072233010.4362@racer.site","threadId":"10687","inReplyTo":"200711072243.21086.robin.rosenberg.lists@dewire.com","subject":"[PATCH v3] Add Documentation/CodingGuidelines","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T22:35:33Z","receivedAt":"2007-11-07T22:35:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven if our code is quite a good documentation for our coding style,\nsome people seem to prefer a document describing it.\n\nThe part about the shell scripts is clearly just copied from one of\nJunio's helpful mails, and some parts were added from comments by\nJunio, Andreas Ericsson and Robin Rosenberg.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tI think I owe this list an apology for starting yet another\n\tthread which seems to instigate comments by not many\n\tcode contributors.\n\n\tMy intention was to make things simpler, but it appears\n\tthat the human brain was created such that it needs complexity\n\tand creates it when it is absent.\n\n\tFor example, when I wrote \"guidelines\" I fully expected that\n\tit was understood that these are no natural laws which you\n\tcannot break, should the circumstances afford it.\n\n\tGiven the discussion, I am half convinced that even this\n\tpatch is a bad idea, and am quite willing to just stop sending\n\tupdates to it.\n\n Documentation/CodingGuidelines | 106 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 106 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/CodingGuidelines\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nnew file mode 100644\nindex 0000000..2d8656f\n--- /dev/null\n+++ b/Documentation/CodingGuidelines\n@@ -0,0 +1,106 @@\n+As a popular project, we also have some guidelines to keep to the\n+code. For git in general, two rough rules are:\n+\n+ - Most importantly, we never say \"It's in POSIX; we'll happily\n+ ignore your needs should your system that does not conform.\"\n+ We live in the real world.\n+\n+ - However, we often say \"Let's stay away from that construct,\n+ it's not even in POSIX\".\n+\n+ - In spite of the above two rules, we sometimes say \"Although\n+ this is not in POSIX, it (is so convenient | makes the code\n+ much more readable | has other good characteristics) and\n+ practically all the platforms we care about support it, so\n+ let's use it\". Again, we live in the real world, and it is\n+ sometimes a judgement call, decided based more on real world\n+ constraints people face than what the paper standard says.\n+\n+\n+As for more concrete guidelines, just imitate the existing code\n+(this is a good guideline, no matter which project you are contributing\n+to...). But if you must have some list of rules, here they are.\n+\n+For shell scripts specifically (not exhaustive):\n+\n+ - We prefer $( ... ) for command substitution; unlike ``, it\n+ properly nests. It should have been the way Bourne spelled\n+ it from day one, but unfortunately isn't.\n+\n+ - We use ${parameter-word} and its [-=?+] siblings, and their\n+ colon'ed \"unset or null\" form.\n+\n+ - We use ${parameter#word} and its [#%] siblings, and their\n+ doubled \"longest matching\" form.\n+\n+ - We use Arithmetic Expansion $(( ... )).\n+\n+ - No \"Substring Expansion\" ${parameter:offset:length}.\n+\n+ - No shell arrays.\n+\n+ - No strlen ${#parameter}.\n+\n+ - No regexp ${parameter/pattern/string}.\n+\n+ - We do not use Process Substitution <(list) or >(list).\n+\n+ - We prefer \"test\" over \"[ ... ]\".\n+\n+ - We do not write noiseword \"function\" in front of shell\n+ functions.\n+\n+For C programs:\n+\n+ - Use tabs to indent, and interpret tabs as taking up to 8 spaces\n+\n+ - Try to keep to at most 80 characters per line\n+\n+ - When declaring pointers, the star sides with the variable name, i.e.\n+ \"char *string\", not \"char* string\" or \"char * string\". This makes\n+ it easier to understand \"char *string, c;\"\n+\n+ - Do not use curly brackets unnecessarily. I.e.\n+\n+\tif (bla) {\n+\t\tx = 1;\n+\t}\n+\n+ is frowned upon. A gray area is when the statement extends over a\n+ few lines, and/or you have a lengthy comment atop of it. Also,\n+ like in the Linux kernel, if there is a long list of \"else if\"\n+ statements, it can make sense to add curly brackets to single\n+ line blocks.\n+\n+ - Try to make your code understandable. You may put comments in, but\n+ comments invariably tend to stale out when the code they were\n+ describing changes. Often splitting a function into two makes the\n+ intention of the code much clearer.\n+\n+ Double negation is often harder to understand than no negation at\n+ all.\n+\n+ Some clever tricks, like using the !! operator with arithmetic\n+ constructs, can be extremely confusing to others. Avoid them,\n+ unless there is a compelling reason to use them.\n+\n+ - Use the API. No, really. We have a strbuf (variable length string),\n+ several arrays with the ALLOC_GROW() macro, a path_list for sorted\n+ string lists, a hash map (mapping struct objects) named\n+ \"struct decorate\", amongst other things.\n+\n+ - When you come up with an API, document it.\n+\n+ - #include system headers in git-compat-util.h. Some headers on some\n+ systems show subtle breakages when you change the order, so it is\n+ best to keep them in one place.\n+\n+ - if you are planning a new command, consider writing it in shell or\n+ perl first, so that changes in semantics can be easily changed and\n+ discussed. Many git commands started out like that, and a few are\n+ still scripts.\n+\n+ - Avoid introducing a new dependency into git. This means you usually\n+ should stay away from scripting languages not already used in the git\n+ core command set (unless your command is clearly separate from it,\n+ such as an importer to convert random-scm-X repositories to git).\n-- \n1.5.3.5.1597.g7191\n"},{"id":"58754","messageId":"7vtznx8x86.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711072233010.4362@racer.site","subject":"Re: [PATCH v3] Add Documentation/CodingGuidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-07T23:14:17Z","receivedAt":"2007-11-07T23:14:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> new file mode 100644\n> index 0000000..2d8656f\n> --- /dev/null\n> +++ b/Documentation/CodingGuidelines\n> @@ -0,0 +1,106 @@\n> +As a popular project, we also have some guidelines to keep to the\n> +code. For git in general, two rough rules are:\n\nI'd rather not to say \"As a popular project\" here. That is\nsomething for others to decide, not for us to advertise.\n\nAlso we now have three rules here ;-).\n\n> +\n> + - Most importantly, we never say \"It's in POSIX; we'll happily\n> + ignore your needs should your system that does not conform.\"\n\n\"should your system not conform\"? \"if your system does not\nconform\"?\n\n> +As for more concrete guidelines, just imitate the existing code\n> +(this is a good guideline, no matter which project you are contributing\n> +to...). But if you must have some list of rules, here they are.\n\ns/\\.\\.\\.//;\n\n> +For C programs:\n> +\n> + - Use tabs to indent, and interpret tabs as taking up to 8 spaces\n> +\n> + - Try to keep to at most 80 characters per line\n> +\n> + - When declaring pointers, the star sides with the variable name, i.e.\n> + \"char *string\", not \"char* string\" or \"char * string\". This makes\n> + it easier to understand \"char *string, c;\"\n\nEnd each sentence with a full stop \".\" for consistency.\n\n> +\n> + - Do not use curly brackets unnecessarily. I.e.\n> +\n> +\tif (bla) {\n> +\t\tx = 1;\n> +\t}\n> +\n> + is frowned upon. A gray area is when the statement extends over a\n> + few lines, and/or you have a lengthy comment atop of it. Also,\n> + like in the Linux kernel, if there is a long list of \"else if\"\n> + statements, it can make sense to add curly brackets to single\n> + line blocks.\n\nAs Robin suggests, s/Do not use/Avoid using/ feels more in line\nwith the spirit with the \"A gray area is...\" description.\n\nI think the official name for {} are \"braces\", by the way.\n\n> + Double negation is often harder to understand than no negation at\n> + all.\n> +\n> + Some clever tricks, like using the !! operator with arithmetic\n> + constructs, can be extremely confusing to others. Avoid them,\n> + unless there is a compelling reason to use them.\n\nNeed a bullet point \"-\" before \"Double\" (but not \"Some\").\n\n> + - #include system headers in git-compat-util.h. Some headers on some\n> + systems show subtle breakages when you change the order, so it is\n> + best to keep them in one place.\n> +\n\nThe first #include in C files except platform specific compat/\nimplementation should be git-compat-util.h or another header\nfile that includes it, such as cache.h and builtin.h.\n\n> + - if you are planning a new command, consider writing it in shell or\n> + perl first, so that changes in semantics can be easily changed and\n> + discussed. Many git commands started out like that, and a few are\n> + still scripts.\n\nBegin a statement with a Capital letter, for consistency.\n"},{"id":"58774","messageId":"Pine.LNX.4.64.0711080033080.4362@racer.site","threadId":"10687","inReplyTo":"7vtznx8x86.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v4] Add Documentation/CodingGuidelines","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-08T00:33:19Z","receivedAt":"2007-11-08T00:33:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven if our code is quite a good documentation for our coding style,\nsome people seem to prefer a document describing it.\n\nThe part about the shell scripts is clearly just copied from one of\nJunio's helpful mails, and some parts were added from comments by\nJunio, Andreas Ericsson and Robin Rosenberg.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/CodingGuidelines | 112 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 112 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/CodingGuidelines\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nnew file mode 100644\nindex 0000000..3b042db\n--- /dev/null\n+++ b/Documentation/CodingGuidelines\n@@ -0,0 +1,112 @@\n+Like other projects, we also have some guidelines to keep to the\n+code. For git in general, three rough rules are:\n+\n+ - Most importantly, we never say \"It's in POSIX; we'll happily\n+ ignore your needs should your system not conform to it.\"\n+ We live in the real world.\n+\n+ - However, we often say \"Let's stay away from that construct,\n+ it's not even in POSIX\".\n+\n+ - In spite of the above two rules, we sometimes say \"Although\n+ this is not in POSIX, it (is so convenient | makes the code\n+ much more readable | has other good characteristics) and\n+ practically all the platforms we care about support it, so\n+ let's use it\".\n+\n+ Again, we live in the real world, and it is sometimes a\n+ judgement call, the decision based more on real world\n+ constraints people face than what the paper standard says.\n+\n+\n+As for more concrete guidelines, just imitate the existing code\n+(this is a good guideline, no matter which project you are\n+contributing to). But if you must have a list of rules,\n+here they are.\n+\n+For shell scripts specifically (not exhaustive):\n+\n+ - We prefer $( ... ) for command substitution; unlike ``, it\n+ properly nests. It should have been the way Bourne spelled\n+ it from day one, but unfortunately isn't.\n+\n+ - We use ${parameter-word} and its [-=?+] siblings, and their\n+ colon'ed \"unset or null\" form.\n+\n+ - We use ${parameter#word} and its [#%] siblings, and their\n+ doubled \"longest matching\" form.\n+\n+ - We use Arithmetic Expansion $(( ... )).\n+\n+ - No \"Substring Expansion\" ${parameter:offset:length}.\n+\n+ - No shell arrays.\n+\n+ - No strlen ${#parameter}.\n+\n+ - No regexp ${parameter/pattern/string}.\n+\n+ - We do not use Process Substitution <(list) or >(list).\n+\n+ - We prefer \"test\" over \"[ ... ]\".\n+\n+ - We do not write the noiseword \"function\" in front of shell\n+ functions.\n+\n+For C programs:\n+\n+ - We use tabs to indent, and interpret tabs as taking up to\n+ 8 spaces.\n+\n+ - We try to keep to at most 80 characters per line.\n+\n+ - When declaring pointers, the star sides with the variable\n+ name, i.e. \"char *string\", not \"char* string\" or\n+ \"char * string\". This makes it easier to understand code\n+ like \"char *string, c;\".\n+\n+ - We avoid using braces unnecessarily. I.e.\n+\n+\tif (bla) {\n+\t\tx = 1;\n+\t}\n+\n+ is frowned upon. A gray area is when the statement extends\n+ over a few lines, and/or you have a lengthy comment atop of\n+ it. Also, like in the Linux kernel, if there is a long list\n+ of \"else if\" statements, it can make sense to add braces to\n+ single line blocks.\n+\n+ - Try to make your code understandable. You may put comments\n+ in, but comments invariably tend to stale out when the code\n+ they were describing changes. Often splitting a function\n+ into two makes the intention of the code much clearer.\n+\n+ - Double negation is often harder to understand than no negation\n+ at all.\n+\n+ - Some clever tricks, like using the !! operator with arithmetic\n+ constructs, can be extremely confusing to others. Avoid them,\n+ unless there is a compelling reason to use them.\n+\n+ - Use the API. No, really. We have a strbuf (variable length\n+ string), several arrays with the ALLOC_GROW() macro, a\n+ path_list for sorted string lists, a hash map (mapping struct\n+ objects) named \"struct decorate\", amongst other things.\n+\n+ - When you come up with an API, document it.\n+\n+ - The first #include in C files, except in platform specific\n+ compat/ implementations, should be git-compat-util.h or another\n+ header file that includes it, such as cache.h or builtin.h.\n+\n+ - If you are planning a new command, consider writing it in shell\n+ or perl first, so that changes in semantics can be easily\n+ changed and discussed. Many git commands started out like\n+ that, and a few are still scripts.\n+\n+ - Avoid introducing a new dependency into git. This means you\n+ usually should stay away from scripting languages not already\n+ used in the git core command set (unless your command is clearly\n+ separate from it, such as an importer to convert random-scm-X\n+ repositories to git).\n-- \n1.5.3.5.1597.g7191\n"},{"id":"58777","messageId":"7vhcjx8tci.fsf@gitster.siamese.dyndns.org","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711080033080.4362@racer.site","subject":"Re: [PATCH v4] Add Documentation/CodingGuidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-08T00:38:05Z","receivedAt":"2007-11-08T00:38:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ok, will queue this for 'maint' ;-).\n"},{"id":"58800","messageId":"20071108061456.GA28509@ins.uni-bonn.de","threadId":"10687","inReplyTo":"7v3avhbxdq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-08T06:14:56Z","receivedAt":"2007-11-08T06:14:56Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"* Junio C Hamano wrote on Wed, Nov 07, 2007 at 09:42:41PM CET:\n> \"Nguyen Thai Ngoc Duy\" writes:\n> >\n> > Argh, should have made it clear, busybox sed is good enough.\n> \n> Thanks. And you can also happy grok Ralf's rewritten construct,\n> right?\n> \n> That is, existing\n> \n> $ sed -e 's/foo/\\n/' file\n> \n> will be rewritten by the patch [2/5] to\n> \n> $ sed -e 's/foo/\\\n> /' file\n\nThe original was something like\n sed 's/[|]/\\n/g'\n\nUsing instead\n tr '|' '\\n'\n\nshould work for the original construct, and AFAIK only /usr/ucb/tr on\nSolaris fails to understand \\n correctly. Would that be better for you?\nOr even\n tr '|' '\\012'\n\nwhich fails only on EBCDIC, which I don't think git targets.\n\nI'll resend the patches tonight.\n\nCheers,\nRalf\n"},{"id":"58860","messageId":"e2b179460711080329j4979dc00w421ec79a6f3159ed@mail.gmail.com","threadId":"10687","inReplyTo":"Pine.LNX.4.64.0711062317330.4362@racer.site","subject":"Re: [PATCH] Add Documentation/CodingStyle","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2007-11-08T11:29:32Z","receivedAt":"2007-11-08T11:29:32Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"On Nov 6, 2007 11:17 PM, Johannes Schindelin wrote:\n>\n> Even if our code is quite a good documentation for our coding style,\n> some people seem to prefer a document describing it.\n\nMight this file be a place to document exactly which licenses we\naccept code under? I.e. is it GPLv2 only or any license compatible\nwith GPLv2 ?\n\nCurrently I see only GPLv2 in core git, but I have a series of patches\nwhich give me a speed-up of two orders of magnitude for some cases of\nat least git-status (on my platform, with my repos etc) and so far my\ntwo approaches involve bringing in code which is either public domain\nwith restrictions (which may be too onerous for us to carry in\nmainline) or 3-clause BSD.\n\nCheers, Mike\n"},{"id":"59246","messageId":"87r6ixag31.fsf@catnip.gol.com","threadId":"10687","inReplyTo":"7v8x5bgl04.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2007-11-10T22:30:42Z","receivedAt":"2007-11-10T22:30:42Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Junio C Hamano writes:\n> /bin/sh on Solaris does not count as you can configure\n> SHELL_PATH to point at xpg4 shell or ksh on that platform.\n\nThat's likely to be a rather confusing point for many people trying to\ninstall however (solaris is rather common), unless git somehow arranges\nfor the right thing to happen automatically, or at least spits out a big\nnoticeable warning message at install time...\n\n-Miles\n-- \nSaa, shall we dance? (from a dance-class advertisement)\n"},{"id":"59467","messageId":"fcaeb9bf0711120320jce181d4xd962c538a898c039@mail.gmail.com","threadId":"10687","inReplyTo":"7v3avhbxdq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] some shell portability fixes","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2007-11-12T11:20:54Z","receivedAt":"2007-11-12T11:20:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Nov 8, 2007 3:42 AM, Junio C Hamano wrote:\n>\n> \"Nguyen Thai Ngoc Duy\" writes:\n>\n> > On 11/7/07, Nguyen Thai Ngoc Duy wrote:\n> >> On 11/7/07, Junio C Hamano wrote:\n> >> > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but\n> >> > would want to get Yays from people with non GNU sed. Is\n> >> > busybox sed good enough to grok our scripts these days?\n> >> > Please ask help and collect Acks at least from folks on\n> >> > Solaris, MacOS, FBSD, and OBSD.\n> >>\n> >> I haven't extensively used all the scripts. There seems to be no\n> >> sed-related failure from git testsuite results in my git-box branch.\n> >> So I would say for now it's good enough.\n> >\n> > Argh, should have made it clear, busybox sed is good enough.\n>\n> Thanks. And you can also happy grok Ralf's rewritten construct,\n> right?\n>\n> That is, existing\n>\n> $ sed -e 's/foo/\\n/' file\n>\n> will be rewritten by the patch [2/5] to\n>\n> $ sed -e 's/foo/\\\n> /' file\n>\n>\n\nYes it worked well\n\n-- \nDuy\n"}]}