{"thread":{"id":"9971","subject":"[PATCH] Allow shell scripts to run with non-Bash /bin/sh","startedAt":"2007-09-21T21:43:46Z","lastAt":"2007-09-25T10:46:52Z","messageCount":45,"participants":["Eygene Ryabinkin","Junio C Hamano","David Kastrup","Vineet Kumar","Adam Flott","Mike Hommey","David Symonds","Pierre Habouzit","Johannes Schindelin","Miles Bader","Avi Kivity"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53759","messageId":"20070921214346.GF97288@void.codelabs.ru","threadId":"9971","inReplyTo":null,"subject":"[PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Eygene Ryabinkin","fromEmail":"rea-git@codelabs.ru","sentAt":"2007-09-21T21:43:46Z","receivedAt":"2007-09-21T21:43:46Z","isPatch":true,"sender":{"key":"rea-git@codelabs.ru","avatar":null},"body":"Good day.\n\nI had found that FreeBSD's /bin/sh refuses to work with git 1.5.3.2.\ncorrectly: no flags are recognized.  The details and fix are below.\nI don't currently know if the Bash's behaviour is POSIXly correct\nor the 'case' statement semantics is not very well defined.   But\nthe following patch fixes the things for the FreeBSD.\n\nHere we go.\n\n-----\n\nOption parsing in the Git shell scripts uses the construct 'while\ncase \"$#\" in 0) break ;; esac; do ... done'.  This is neat, because\nit needs no external commands invocation.  But in the case when\n/bin/sh is not GNU Bash (for example, on FreeBSD) this cycle will\nnot be executed at all.\n\nThe fix is to add the case branch '*) : ;;'.  It also needs no\nexternal commands invocation and it does its work, because ':'\nalways returns zero.\n\nSigned-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>\n---\n git-am.sh                  |    2 +-\n git-clean.sh               |    2 +-\n git-commit.sh              |    2 +-\n git-fetch.sh               |    2 +-\n git-filter-branch.sh       |    2 +-\n git-instaweb.sh            |    2 +-\n git-ls-remote.sh           |    2 +-\n git-merge.sh               |    2 +-\n git-mergetool.sh           |    2 +-\n git-pull.sh                |    2 +-\n git-quiltimport.sh         |    2 +-\n git-rebase--interactive.sh |    2 +-\n git-rebase.sh              |    2 +-\n git-repack.sh              |    2 +-\n git-reset.sh               |    2 +-\n git-submodule.sh           |    2 +-\n 16 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 6809aa0..0bd8d34 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -109,7 +109,7 @@ dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=\n resolvemsg= resume=\n git_apply_opt=\n \n-while case \"$#\" in 0) break;; esac\n+while case \"$#\" in 0) break;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-d=*|--d=*|--do=*|--dot=*|--dote=*|--dotes=*|--dotest=*)\ndiff --git a/git-clean.sh b/git-clean.sh\nindex a5cfd9f..1fac731 100755\n--- a/git-clean.sh\n+++ b/git-clean.sh\n@@ -26,7 +26,7 @@ rmrf=\"rm -rf --\"\n rm_refuse=\"echo Not removing\"\n echo1=\"echo\"\n \n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-d)\ndiff --git a/git-commit.sh b/git-commit.sh\nindex bb113e8..5f298c1 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -89,7 +89,7 @@ force_author=\n only_include_assumed=\n untracked_files=\n templatefile=\"`git config commit.template`\"\n-while case \"$#\" in 0) break;; esac\n+while case \"$#\" in 0) break;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-F|--F|-f|--f|--fi|--fil|--file)\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex c3a2001..dac2d72 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -27,7 +27,7 @@ shallow_depth=\n no_progress=\n test -t 1 || no_progress=--no-progress\n quiet=\n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-a|--a|--ap|--app|--appe|--appen|--append)\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a4b6577..02b567b 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -105,7 +105,7 @@ filter_tag_name=\n filter_subdir=\n orig_namespace=refs/original/\n force=\n-while case \"$#\" in 0) usage;; esac\n+while case \"$#\" in 0) usage;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t--)\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex b79c6b6..c85f8c0 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -61,7 +61,7 @@ stop_httpd () {\n \ttest -f \"$fqgitdir/pid\" && kill `cat \"$fqgitdir/pid\"`\n }\n \n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t--stop|stop)\ndiff --git a/git-ls-remote.sh b/git-ls-remote.sh\nindex b7e5d04..4ef4341 100755\n--- a/git-ls-remote.sh\n+++ b/git-ls-remote.sh\n@@ -13,7 +13,7 @@ die () {\n }\n \n exec=\n-while case \"$#\" in 0) break;; esac\n+while case \"$#\" in 0) break;; *) : ;; esac\n do\n   case \"$1\" in\n   -h|--h|--he|--hea|--head|--heads)\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 3a01db0..94a50aa 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -122,7 +122,7 @@ merge_name () {\n case \"$#\" in 0) usage ;; esac\n \n have_message=\n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\\\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 47a8055..0e286dd 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -268,7 +268,7 @@ merge_file () {\n     cleanup_temp_files\n }\n \n-while case $# in 0) break ;; esac\n+while case $# in 0) break ;; *) : ;; esac\n do\n     case \"$1\" in\n \t-t|--tool*)\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 5e96d1f..722ed4e 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -16,7 +16,7 @@ test -z \"$(git ls-files -u)\" ||\n \tdie \"You are in the middle of a conflicted merge.\"\n \n strategy_args= no_summary= no_commit= squash=\n-while case \"$#,$1\" in 0) break ;; *,-*) ;; *) break ;; esac\n+while case \"$#,$1\" in 0) break ;; *,-*) : ;; *) break ;; esac\n do\n \tcase \"$1\" in\n \t-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\\\ndiff --git a/git-quiltimport.sh b/git-quiltimport.sh\nindex 9de54d1..4039617 100755\n--- a/git-quiltimport.sh\n+++ b/git-quiltimport.sh\n@@ -5,7 +5,7 @@ SUBDIRECTORY_ON=Yes\n \n dry_run=\"\"\n quilt_author=\"\"\n-while case \"$#\" in 0) break;; esac\n+while case \"$#\" in 0) break;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t--au=*|--aut=*|--auth=*|--autho=*|--author=*)\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex abc2b1c..54e4299 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -317,7 +317,7 @@ do_rest () {\n \tdone\n }\n \n-while case $# in 0) break ;; esac\n+while case $# in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t--continue)\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 3bd66b0..f7ae22c 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -122,7 +122,7 @@ finish_rb_merge () {\n \n is_interactive () {\n \ttest -f \"$dotest\"/interactive ||\n-\twhile case $#,\"$1\" in 0,|*,-i|*,--interactive) break ;; esac\n+\twhile case $#,\"$1\" in 0,|*,-i|*,--interactive) break ;; *) : ;; esac\n \tdo\n \t\tshift\n \tdone && test -n \"$1\"\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 156c5e8..aac771e 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -9,7 +9,7 @@ SUBDIRECTORY_OK='Yes'\n \n no_update_info= all_into_one= remove_redundant=\n local= quiet= no_reuse= extra=\n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t-n)\tno_update_info=t ;;\ndiff --git a/git-reset.sh b/git-reset.sh\nindex 1dc606f..eb92610 100755\n--- a/git-reset.sh\n+++ b/git-reset.sh\n@@ -11,7 +11,7 @@ require_work_tree\n update= reset_type=--mixed\n unset rev\n \n-while case $# in 0) break ;; esac\n+while case $# in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \t--mixed | --soft | --hard)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3320998..78a25ad 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -251,7 +251,7 @@ modules_list()\n \tdone\n }\n \n-while case \"$#\" in 0) break ;; esac\n+while case \"$#\" in 0) break ;; *) : ;; esac\n do\n \tcase \"$1\" in\n \tadd)\n-- \n1.5.3.2\n-- \nEygene\n"},{"id":"53764","messageId":"7v8x6zinjf.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"20070921214346.GF97288@void.codelabs.ru","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-21T23:52:52Z","receivedAt":"2007-09-21T23:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n\n> Good day.\n>\n> I had found that FreeBSD's /bin/sh refuses to work with git 1.5.3.2.\n> correctly: no flags are recognized.  The details and fix are below.\n> I don't currently know if the Bash's behaviour is POSIXly correct\n> or the 'case' statement semantics is not very well defined.   But\n> the following patch fixes the things for the FreeBSD.\n>\n> Here we go.\n>\n> -----\n>\n> Option parsing in the Git shell scripts uses the construct 'while\n> case \"$#\" in 0) break ;; esac; do ... done'.  This is neat, because\n> it needs no external commands invocation.  But in the case when\n> /bin/sh is not GNU Bash (for example, on FreeBSD) this cycle will\n> not be executed at all.\n\nI do not doubt that \"while case $# in 0) break ;; esac\" does not\nwork for your shell.  But I think the above comment is grossly\nmisleading.\n\nDon't mention bash there.  You sound as if you are blaming\nbashism, but the thing is, your shell is simply broken.\n\nYou have other choices than bash on BSD don't you?\n\nMy quick test shows that ksh, pdksh and dash seem to work\ncorrectly.  This idiom is what I picked up around late 80's from\nsomebody, and kept using on many variants of Unices.  I would\nfind quite surprising that something that claims to be a shell\ndoes not work correctly.  Even /bin/sh that comes with Solaris\nseems to work correctly, which should tell you something.\n\nOpenBSD's /bin/sh seems to be Ok; I do not know whose shell they\nuse, but it seems to be hard-linked to /bin/ksh which is pdksh.\n"},{"id":"53768","messageId":"86abrfy377.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"7v8x6zinjf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-22T00:05:32Z","receivedAt":"2007-09-22T00:05:32Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I do not doubt that \"while case $# in 0) break ;; esac\" does not\n> work for your shell.  But I think the above comment is grossly\n> misleading.\n\nPersonally, I find this idiom distasteful.\n\nI'd do either\nwhile case $# in 0) false ;; *) true esac\n\nor, more likely \nwhile : do case $# in 0) break;; esac\n\nBut doing a break inside of the while _condition_ rather than the body\njust feels wrong to me.\n\n-- \nDavid Kastrup\n"},{"id":"53770","messageId":"7vvea3h7sn.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"86abrfy377.fsf@lola.quinscape.zz","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T00:18:16Z","receivedAt":"2007-09-22T00:18:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> But doing a break inside of the while _condition_ rather than the body\n> just feels wrong to me.\n\nSorry, but that is not the issue on the thread is about.\nBSD shell is failing the whole case statement when there is no\nmatching case arm.\n"},{"id":"53771","messageId":"7vr6krh7ny.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"7vvea3h7sn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T00:21:05Z","receivedAt":"2007-09-22T00:21:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> But doing a break inside of the while _condition_ rather than the body\n>> just feels wrong to me.\n>\n> Sorry, but that is not the issue on the thread is about.\n> BSD shell is failing the whole case statement when there is no\n> matching case arm.\n\nI did not mean \"BSD shell\" in general here.  The shell Eygene\nuses on his (unspecified version of) FreeBSD box is failing.\n"},{"id":"53772","messageId":"86zlzfwno3.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"7vvea3h7sn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-22T00:26:20Z","receivedAt":"2007-09-22T00:26:20Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> But doing a break inside of the while _condition_ rather than the body\n>> just feels wrong to me.\n>\n> Sorry, but that is not the issue on the thread is about.\n\nIt is still relevant:\n\n> BSD shell is failing the whole case statement when there is no\n> matching case arm.\n\nSure, that is a bug.  But in my opinion the idiom as such is ugly\nenough not be worth keeping anyhow.  The proposal of the patch\nsubmitter was making an already ugly idiom even uglier for the sake of\nhis shell.  I agree that this is not the way to go.\n\nI was proposing replacing the idiom by something which I find cleaner.\nThat it will most likely work with the original poster's shell is more\nor less a side effect.  At least it is a side effect that might\nmotivate somebody else rather than me to do the cleanup work (and have\na buggy test shell to see whether he got everything indeed).\n\n-- \nDavid Kastrup\n"},{"id":"53782","messageId":"7vlkazh1ji.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"20070921214346.GF97288@void.codelabs.ru","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T02:33:21Z","receivedAt":"2007-09-22T02:33:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n\n> Good day.\n>\n> I had found that FreeBSD's /bin/sh refuses to work with git 1.5.3.2.\n> correctly: no flags are recognized.\n\n> @@ -109,7 +109,7 @@ dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=\n>  resolvemsg= resume=\n>  git_apply_opt=\n>  \n> -while case \"$#\" in 0) break;; esac\n> +while case \"$#\" in 0) break;; *) : ;; esac\n>  do\n\nI am assuming that this works around _a_ bug in that /bin/sh; I\nwould make sure I understand the nature of the bug.  Is it Ok to\nunderstand that with that shell, after this construct runs:\n\n\tcase <some word> in\n        <case arm #1>)\n        \tsomething ;;\n\t<case arm #2>)\n        \tsomething else ;;\n\tesac\n\nthe status from the whole case statement is false, when <some word>\ndoes not match any of the glob patterns listed in any of the case arm?\n\nThat is, what does the shell say if you do this?\n\n\tcase Ultra in\n        Super)\n        \tfalse ;;\n\tHyper)\n        \ttrue ;;\n\tesac &&\n        echo case returned ok\n\nThe reason I ask is because\n\n\twhile case $# in 0) ... esac\n        do\n        \t...\n\tdone\n\nis not the only place the status from \"case\" itself matters in\nour scripts.  There are places that do\n\n\tsomething &&\n\tcase ... in\n        ...\n        esac &&\n        something else\n\nand we would need to add no-op match-everything arm to all of\nsuch case statements in our scripts.\n\nBesides test scripts, there is one in git-ls-remote.sh which you\nseem to have missed.\n"},{"id":"53784","messageId":"20070922035434.GA99140@void.codelabs.ru","threadId":"9971","inReplyTo":"7vlkazh1ji.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Eygene Ryabinkin","fromEmail":"rea-git@codelabs.ru","sentAt":"2007-09-22T03:54:34Z","receivedAt":"2007-09-22T03:54:34Z","isPatch":true,"sender":{"key":"rea-git@codelabs.ru","avatar":null},"body":"Junio, good day.\n\nFri, Sep 21, 2007 at 04:52:52PM -0700, Junio C Hamano wrote:\n> > Option parsing in the Git shell scripts uses the construct 'while\n> > case \"$#\" in 0) break ;; esac; do ... done'.  This is neat, because\n> > it needs no external commands invocation.  But in the case when\n> > /bin/sh is not GNU Bash (for example, on FreeBSD) this cycle will\n> > not be executed at all.\n> \n> I do not doubt that \"while case $# in 0) break ;; esac\" does not\n> work for your shell.  But I think the above comment is grossly\n> misleading.\n> \n> Don't mention bash there.  You sound as if you are blaming\n> bashism, but the thing is, your shell is simply broken.\n\nOK, you're right.  Especially if /bin/sh from Solaris and OpenBSD\nare working and they are not Bash.  But I would not tell that\nthe shell is broken now -- I had not seen the POSIX specification.\nDoes it specifies how the shell should work in this case?\n\n> You have other choices than bash on BSD don't you?\n\nDid not understand the question, sorry.  The thing is that\nFreeBSD has /bin/sh that is derived from the original Berkeley\nshell.  And it is desirable to have it working with Git\nscript, since I don't want to make bash (or whatever shell\nthat is not /bin/sh) a dependency for the port.\n\n> My quick test shows that ksh, pdksh and dash seem to work\n> correctly.  This idiom is what I picked up around late 80's from\n> somebody, and kept using on many variants of Unices.  I would\n> find quite surprising that something that claims to be a shell\n> does not work correctly.  Even /bin/sh that comes with Solaris\n> seems to work correctly, which should tell you something.\n> \n> OpenBSD's /bin/sh seems to be Ok; I do not know whose shell they\n> use, but it seems to be hard-linked to /bin/ksh which is pdksh.\n\nOK, I think I need to find out why FreeBSD's /bin/sh behaves\nlike this, because the test you propose on your next message\nworks.  See below.\n\nBy the way, my FreeBSD is 7-CURRENT, but I'll test on 6-STABLE\nand perhaps on 4-STABLE on Monday.\n\nFri, Sep 21, 2007 at 07:33:21PM -0700, Junio C Hamano wrote:\n> I am assuming that this works around _a_ bug in that /bin/sh; I\n> would make sure I understand the nature of the bug.  Is it Ok to\n> understand that with that shell, after this construct runs:\n> \n> \tcase <some word> in\n>         <case arm #1>)\n>         \tsomething ;;\n> \t<case arm #2>)\n>         \tsomething else ;;\n> \tesac\n> \n> the status from the whole case statement is false, when <some word>\n> does not match any of the glob patterns listed in any of the case arm?\n> \n> That is, what does the shell say if you do this?\n> \n> \tcase Ultra in\n>         Super)\n>         \tfalse ;;\n> \tHyper)\n>         \ttrue ;;\n> \tesac &&\n>         echo case returned ok\n\nIt says 'case returned ok', so I will try to understand why it\nworks here and does not work in the 'while' construct.\n\nThanks for the pointer!\n-- \nEygene\n"},{"id":"53785","messageId":"86tzpnwdha.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"20070922035434.GA99140@void.codelabs.ru","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-22T04:06:25Z","receivedAt":"2007-09-22T04:06:25Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n\n>> That is, what does the shell say if you do this?\n>> \n>> \tcase Ultra in\n>>         Super)\n>>         \tfalse ;;\n>> \tHyper)\n>>         \ttrue ;;\n>> \tesac &&\n>>         echo case returned ok\n>\n> It says 'case returned ok', so I will try to understand why it\n> works here and does not work in the 'while' construct.\n\nWhat you actually need to do is\n\nfalse\ncase Ultra in\n   Super)\n   \tfalse ;;\nHyper)\n   \ttrue ;;\nesac && echo case returned ok\n\n\n-- \nDavid Kastrup\n"},{"id":"53787","messageId":"7vhclngpgd.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"20070922035434.GA99140@void.codelabs.ru","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T06:54:26Z","receivedAt":"2007-09-22T06:54:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n\n> By the way, my FreeBSD is 7-CURRENT, but I'll test on 6-STABLE\n> and perhaps on 4-STABLE on Monday.\n> ...\n>> That is, what does the shell say if you do this?\n>> \n>> \tcase Ultra in\n>>         Super)\n>>         \tfalse ;;\n>> \tHyper)\n>>         \ttrue ;;\n>> \tesac &&\n>>         echo case returned ok\n>\n> It says 'case returned ok', so I will try to understand why it\n> works here and does not work in the 'while' construct.\n\nI vaguely recall somebody else had exactly this issue and he\nconcluded that the shell was busted.  I do not recall the\ndetails of the story but interestingly, if he did something that\naccesses \"$#\" before the problematic \"while case $# in ...\" the\nshell behaved for him in his experiments.\n\nJust to make sure you do not misunderstand me, I am not trying\nto be difficult.  I am trying to assess (1) if it is sensible to\nsupport that broken shell, and (2) if so what the exact breakage\nis, especially because as the above shows the breakage does not\nlook like what your \"fix\" literally suggests, and what is\ninvolved in working it around.\n\nAlso by my comment about \"/bin/sh and bash not being the only\nshells available on FreeBSD\", I did not mean that you should\nchange your /bin/sh.  You can build git with SHELL_PATH make\nvarilable pointing at a non-broken shell, which does not have to\nbe installed as /bin/sh.\n"},{"id":"53788","messageId":"7vd4wbgp9t.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"86tzpnwdha.fsf@lola.quinscape.zz","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T06:58:22Z","receivedAt":"2007-09-22T06:58:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>\n>>> That is, what does the shell say if you do this?\n>>> \n>>> \tcase Ultra in\n>>>         Super)\n>>>         \tfalse ;;\n>>> \tHyper)\n>>>         \ttrue ;;\n>>> \tesac &&\n>>>         echo case returned ok\n>>\n>> It says 'case returned ok', so I will try to understand why it\n>> works here and does not work in the 'while' construct.\n>\n> What you actually need to do is\n>\n> false\n> case Ultra in\n>    Super)\n>    \tfalse ;;\n> Hyper)\n>    \ttrue ;;\n> esac && echo case returned ok\n\nAHHHHHH.\n\nIs \"case\" supposed to be transparent?\n"},{"id":"53790","messageId":"20070922073446.GA3903@doorstop.net","threadId":"9971","inReplyTo":"7vd4wbgp9t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Vineet Kumar","fromEmail":"vineet@doorstop.net","sentAt":"2007-09-22T07:34:46Z","receivedAt":"2007-09-22T07:34:46Z","isPatch":true,"sender":{"key":"vineet@doorstop.net","avatar":"https://gravatar.com/avatar/7eaf4d9b5b3c6cadffd6515fb81ede43381557eaad3e58c7140ff1fc5ccb41dd?d=mp&s=160"},"body":"* Junio C Hamano (gitster@pobox.com) [070921 23:58]:\n> David Kastrup <dak@gnu.org> writes:\n> \n> > Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n> >\n> >>> That is, what does the shell say if you do this?\n> >>> \n> >>> \tcase Ultra in\n> >>>         Super)\n> >>>         \tfalse ;;\n> >>> \tHyper)\n> >>>         \ttrue ;;\n> >>> \tesac &&\n> >>>         echo case returned ok\n> >>\n> >> It says 'case returned ok', so I will try to understand why it\n> >> works here and does not work in the 'while' construct.\n> >\n> > What you actually need to do is\n> >\n> > false\n> > case Ultra in\n> >    Super)\n> >    \tfalse ;;\n> > Hyper)\n> >    \ttrue ;;\n> > esac && echo case returned ok\n> \n> AHHHHHH.\n> \n> Is \"case\" supposed to be transparent?\n\nThat doesn't seem to be the case (no pun intended) on either bash or\ndash.  Here's what I tested on bash (apologies for the long lines; these\nare verbatim pastes from my shell):\n\nvineet@sprocket:~$ false\nvineet@sprocket:~$ case Super in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\nsuper\nvineet@sprocket:~$ false\nvineet@sprocket:~$ case Hyper in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\nhyper\ncase returned ok\nvineet@sprocket:~$ false\nvineet@sprocket:~$ case Ultra in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\ncase returned ok\nvineet@sprocket:~$ \n\n\nand on dash:\n\nvineet@sprocket:~$ dash\n$ false\n$ case Super in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\nsuper\n$ false\n$ case Hyper in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\nhyper\ncase returned ok\n$ false\n$ case Ultra in Super) echo super ; false ;; Hyper) echo hyper ; true ;; esac && echo case returned ok\ncase returned ok\n$ \n\n\nSo it seems like a \"case\" statement isn't special; it returns a status\nlike any other statement.\n\n\nVineet\n-- \nhttp://www.doorstop.net/\n"},{"id":"53791","messageId":"7vtzpnf6c9.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"20070922035434.GA99140@void.codelabs.ru","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-22T08:32:38Z","receivedAt":"2007-09-22T08:32:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n\n> OK, you're right.  Especially if /bin/sh from Solaris and OpenBSD\n> are working and they are not Bash.  But I would not tell that\n> the shell is broken now -- I had not seen the POSIX specification.\n> Does it specifies how the shell should work in this case?\n\nI have always been assuming it to be the case (this construct is\nnot my invention but is an old school idiom I just inherited\nfrom my mentor) and never looked at the spec recently, but I\nre-read it just to make sure.  The answer is yes.\n\nVisit http://www.opengroup.org/onlinepubs/000095399/ and follow\n\"Shell and Utilities volume (XCU)\" and then \"Case conditional\nconstruct\".\n\n    Exit Status\n\n    The exit status of case shall be zero if no patterns are\n    matched. Otherwise, the exit status shall be the exit status of\n    the last command executed in the compound-list.\n\nSo, as David suggests, if\n\n        false\n        case Ultra in\n        Super) false ;;\n        Hyper) true ;;\n        esac && echo case returned ok\n\ndoes not say \"case returned ok\", then the shell has a bit of\nproblem.\n"},{"id":"53800","messageId":"85ps0buffr.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"7vd4wbgp9t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-22T11:07:04Z","receivedAt":"2007-09-22T11:07:04Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>>\n>>>> That is, what does the shell say if you do this?\n>>>> \n>>>> \tcase Ultra in\n>>>>         Super)\n>>>>         \tfalse ;;\n>>>> \tHyper)\n>>>>         \ttrue ;;\n>>>> \tesac &&\n>>>>         echo case returned ok\n>>>\n>>> It says 'case returned ok', so I will try to understand why it\n>>> works here and does not work in the 'while' construct.\n>>\n>> What you actually need to do is\n>>\n>> false\n>> case Ultra in\n>>    Super)\n>>    \tfalse ;;\n>> Hyper)\n>>    \ttrue ;;\n>> esac && echo case returned ok\n>\n> AHHHHHH.\n>\n> Is \"case\" supposed to be transparent?\n\nNot that I would know.  It is basically a revival of the\n\nfalse\nif false then : ; fi || echo \"this fails!?!\"\n\nbug that probably has been fixed by now.  For obvious reasons,\nconditionals without a taken branch are considered to have an exit\ncode of 0.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53813","messageId":"20070922162750.Y1876@localhost","threadId":"9971","inReplyTo":"7vhclngpgd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Adam Flott","fromEmail":"adam@npjh.com","sentAt":"2007-09-22T21:37:11Z","receivedAt":"2007-09-22T21:37:11Z","isPatch":true,"sender":{"key":"adam@npjh.com","avatar":"https://gravatar.com/avatar/b1499b03b4ec6de9607c0cc5c57de362b328a9cf646277f8f77b12f80c0e6228?d=mp&s=160"},"body":"\nOn Fri, 21 Sep 2007, Junio C Hamano wrote:\n\n> I vaguely recall somebody else had exactly this issue and he\n> concluded that the shell was busted.  I do not recall the\n> details of the story but interestingly, if he did something that\n> accesses \"$#\" before the problematic \"while case $# in ...\" the\n> shell behaved for him in his experiments.\n\nThat is what I did notice, just accessing $# fixed later uses of it.\n\n> Also by my comment about \"/bin/sh and bash not being the only\n> shells available on FreeBSD\", I did not mean that you should\n> change your /bin/sh.  You can build git with SHELL_PATH make\n> varilable pointing at a non-broken shell, which does not have to\n> be installed as /bin/sh.\n\nIf one's installing from the ports tree, then the port should depend on a\nnon-broken shell and set SHELL_PATH. And as for installing by hand, just print\nout a warning that SHELL_PATH points to a broken shell and be done with it.\nThis is a FreeBSD bug, not a git one.\n\nI had been meaning to write up a bug about this using a small test case, but I\ncouldn't reproduce it.\n\n\nAdam\n"},{"id":"53824","messageId":"20070923083118.GB99140@void.codelabs.ru","threadId":"9971","inReplyTo":"7vtzpnf6c9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Eygene Ryabinkin","fromEmail":"rea-git@codelabs.ru","sentAt":"2007-09-23T08:31:18Z","receivedAt":"2007-09-23T08:31:18Z","isPatch":true,"sender":{"key":"rea-git@codelabs.ru","avatar":null},"body":"Junio, *, good day.\n\nSat, Sep 22, 2007 at 01:32:38AM -0700, Junio C Hamano wrote:\n> > OK, you're right.  Especially if /bin/sh from Solaris and OpenBSD\n> > are working and they are not Bash.  But I would not tell that\n> > the shell is broken now -- I had not seen the POSIX specification.\n> > Does it specifies how the shell should work in this case?\n> \n> I have always been assuming it to be the case (this construct is\n> not my invention but is an old school idiom I just inherited\n> from my mentor) and never looked at the spec recently, but I\n> re-read it just to make sure.  The answer is yes.\n> \n> Visit http://www.opengroup.org/onlinepubs/000095399/ and follow\n> \"Shell and Utilities volume (XCU)\" and then \"Case conditional\n> construct\".\n\nYes, thanks for the pointer.\n\n> So, as David suggests, if\n> \n>         false\n>         case Ultra in\n>         Super) false ;;\n>         Hyper) true ;;\n>         esac && echo case returned ok\n> \n> does not say \"case returned ok\", then the shell has a bit of\n> problem.\n\nCorrect: the current /bin/sh for FreeBSD does not set zero exit\ncode if no case patterns were matched.  So, I apologize for my quick\ndecision on the non-brokenness of the /bin/sh -- it is broken.\n\nI had fixed the shell and filed the problem report.  May be the\nchange will be incorporated into the future release of FreeBSD.\nMeanwhile, I had added workarounds to the other places Junio mentioned\nin his follow-up and will try to push this patch to the FreeBSD\nport of Git.  The explanation had been changed too ;))\n\nThanks to all people who helped me to realize what is wrong and where!\n-- \nEygene\n"},{"id":"53825","messageId":"85ir61rc3r.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"7vtzpnf6c9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-23T08:59:36Z","receivedAt":"2007-09-23T08:59:36Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>\n>> OK, you're right.  Especially if /bin/sh from Solaris and OpenBSD\n>> are working and they are not Bash.  But I would not tell that\n>> the shell is broken now -- I had not seen the POSIX specification.\n>> Does it specifies how the shell should work in this case?\n>\n> I have always been assuming it to be the case (this construct is\n> not my invention but is an old school idiom I just inherited\n> from my mentor) and never looked at the spec recently, but I\n> re-read it just to make sure.  The answer is yes.\n\nIndependent of that: would you mind a patch replacing that idiom with\n\nwhile : do case xxx) break; esac\n\ninstead?  I find breaking out of the condition rather than the body\nawkward, and I find a non-matching case statement, POSIX or not, quite\nunobvious in the place of a true while condition.\n\nIt is a bit too much of cleverness for my taste.  Never mind that the\ncurrent FreeBSD shell does not understand it due to being buggy: I\nfind that this is not very readable to the human reader either without\na double take.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53848","messageId":"7vy7excho4.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"85ir61rc3r.fsf@lola.goethe.zz","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-23T19:20:43Z","receivedAt":"2007-09-23T19:20:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Independent of that: would you mind a patch replacing that idiom with\n>\n> while : do case xxx) break; esac\n>\n> instead?  I find breaking out of the condition rather than the body\n> awkward,...\n\nI do not have any problem with your approach at all.\n\nWhile I personally do not think it improves readability much, I\ndo not think it hurts either.  And it is a valid workaround for\nFBSD issue, so why not.\n\nBut on one condition, however.  If it is done correctly with\ndouble semi-colons before \"esac\" ;-)\n\nThanks.\n"},{"id":"53852","messageId":"853ax5mb1j.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"7vy7excho4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow shell scripts to run with non-Bash /bin/sh","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-23T19:33:44Z","receivedAt":"2007-09-23T19:33:44Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Independent of that: would you mind a patch replacing that idiom with\n>>\n>> while : do case xxx) break; esac\n>>\n>> instead?  I find breaking out of the condition rather than the body\n>> awkward,...\n>\n> I do not have any problem with your approach at all.\n\nToo bad sh does... I need to write\n\nwhile :; do case xxx) break; esac\n\ninstead which is slightly uglier concerning visual aesthetics.\n\n> But on one condition, however.  If it is done correctly with\n> double semi-colons before \"esac\" ;-)\n\nMy feeling about this is that double semi-colons before \"esac\" are\nredundant and similarly ugly as \";\" before \"end\" in Pascal.  However,\nlooking at the existing git scripts it would seem that the redundant\nstyle is ubiquitous anyway, so it would be inconsistent to do this\ndifferently just within the idiom.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53854","messageId":"85myvdktb3.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"853ax5mb1j.fsf@lola.goethe.zz","subject":"[PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-23T20:42:08Z","receivedAt":"2007-09-23T20:42:08Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"A lot of shell scripts contained stuff starting with\n\n\twhile case \"$#\" in 0) break ;; esac\n\nand similar.  I consider breaking out of the condition instead of the\nbody od the loop ugly, and the implied \"true\" value of the\nnon-matching case is not really obvious to humans at first glance.  It\nhappens not to be obvious to some BSD shells, either, but that's\nbecause they are not POSIX-compliant.  In most cases, this has been\nreplaced by a straight condition using \"test\".  \"case\" has the\nadvantage of being faster than \"test\" on vintage shells where \"test\"\nis not a builtin.  Since none of them is likely to run the git\nscripts, anyway, the added readability should be worth the change.\n\nA few loops have had their termination condition expressed\ndifferently.\n\nSigned-off-by: David Kastrup <dak@gnu.org>\n---\n\nOk, this is not really what we have been talking about except in one\ncase, but I think it is actually more of an improvement.\n\n contrib/examples/git-gc.sh         |    2 +-\n contrib/examples/git-reset.sh      |    2 +-\n contrib/examples/git-tag.sh        |    2 +-\n contrib/examples/git-verify-tag.sh |    2 +-\n git-am.sh                          |    2 +-\n git-clean.sh                       |    2 +-\n git-commit.sh                      |    2 +-\n git-fetch.sh                       |    2 +-\n git-filter-branch.sh               |    3 ++-\n git-instaweb.sh                    |    2 +-\n git-ls-remote.sh                   |    2 +-\n git-merge.sh                       |    2 +-\n git-mergetool.sh                   |    2 +-\n git-pull.sh                        |    6 +++---\n git-quiltimport.sh                 |    2 +-\n git-rebase--interactive.sh         |    2 +-\n git-rebase.sh                      |    5 ++---\n git-repack.sh                      |    2 +-\n git-submodule.sh                   |    2 +-\n 19 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/contrib/examples/git-gc.sh b/contrib/examples/git-gc.sh\nindex 2ae235b..1597e9f 100755\n--- a/contrib/examples/git-gc.sh\n+++ b/contrib/examples/git-gc.sh\n@@ -9,7 +9,7 @@ SUBDIRECTORY_OK=Yes\n . git-sh-setup\n \n no_prune=:\n-while case $# in 0) break ;; esac\n+while test $# != 0\n do\n \tcase \"$1\" in\n \t--prune)\ndiff --git a/contrib/examples/git-reset.sh b/contrib/examples/git-reset.sh\nindex 1dc606f..bafeb52 100755\n--- a/contrib/examples/git-reset.sh\n+++ b/contrib/examples/git-reset.sh\n@@ -11,7 +11,7 @@ require_work_tree\n update= reset_type=--mixed\n unset rev\n \n-while case $# in 0) break ;; esac\n+while test $# != 0\n do\n \tcase \"$1\" in\n \t--mixed | --soft | --hard)\ndiff --git a/contrib/examples/git-tag.sh b/contrib/examples/git-tag.sh\nindex 5ee3f50..7bb7486 100755\n--- a/contrib/examples/git-tag.sh\n+++ b/contrib/examples/git-tag.sh\n@@ -14,7 +14,7 @@ username=\n list=\n verify=\n LINES=0\n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n     case \"$1\" in\n     -a)\ndiff --git a/contrib/examples/git-verify-tag.sh b/contrib/examples/git-verify-tag.sh\nindex 37b0023..0902a5c 100755\n--- a/contrib/examples/git-verify-tag.sh\n+++ b/contrib/examples/git-verify-tag.sh\n@@ -5,7 +5,7 @@ SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n verbose=\n-while case $# in 0) break;; esac\n+while test $# != 0\n do\n \tcase \"$1\" in\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\ndiff --git a/git-am.sh b/git-am.sh\nindex 6809aa0..d97b528 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -109,7 +109,7 @@ dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=\n resolvemsg= resume=\n git_apply_opt=\n \n-while case \"$#\" in 0) break;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-d=*|--d=*|--do=*|--dot=*|--dote=*|--dotes=*|--dotest=*)\ndiff --git a/git-clean.sh b/git-clean.sh\nindex a5cfd9f..e847ae2 100755\n--- a/git-clean.sh\n+++ b/git-clean.sh\n@@ -26,7 +26,7 @@ rmrf=\"rm -rf --\"\n rm_refuse=\"echo Not removing\"\n echo1=\"echo\"\n \n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-d)\ndiff --git a/git-commit.sh b/git-commit.sh\nindex 3e46dbb..dcdd49b 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -89,7 +89,7 @@ force_author=\n only_include_assumed=\n untracked_files=\n templatefile=\"`git config commit.template`\"\n-while case \"$#\" in 0) break;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-F|--F|-f|--f|--fi|--fil|--file)\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex c3a2001..dc6483f 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -27,7 +27,7 @@ shallow_depth=\n no_progress=\n test -t 1 || no_progress=--no-progress\n quiet=\n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-a|--a|--ap|--app|--appe|--appen|--append)\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a4b6577..b7b2ef7 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -105,8 +105,9 @@ filter_tag_name=\n filter_subdir=\n orig_namespace=refs/original/\n force=\n-while case \"$#\" in 0) usage;; esac\n+while :\n do\n+\ttest \"$#\" = 0 && usage\n \tcase \"$1\" in\n \t--)\n \t\tshift\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex b79c6b6..364cf93 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -61,7 +61,7 @@ stop_httpd () {\n \ttest -f \"$fqgitdir/pid\" && kill `cat \"$fqgitdir/pid\"`\n }\n \n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t--stop|stop)\ndiff --git a/git-ls-remote.sh b/git-ls-remote.sh\nindex b7e5d04..e6e9760 100755\n--- a/git-ls-remote.sh\n+++ b/git-ls-remote.sh\n@@ -13,7 +13,7 @@ die () {\n }\n \n exec=\n-while case \"$#\" in 0) break;; esac\n+while test \"$#\" != 0\n do\n   case \"$1\" in\n   -h|--h|--he|--hea|--head|--heads)\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 3a01db0..e4116a8 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -122,7 +122,7 @@ merge_name () {\n case \"$#\" in 0) usage ;; esac\n \n have_message=\n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\\\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 47a8055..a0e44f7 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -268,7 +268,7 @@ merge_file () {\n     cleanup_temp_files\n }\n \n-while case $# in 0) break ;; esac\n+while test $# != 0\n do\n     case \"$1\" in\n \t-t|--tool*)\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 5e96d1f..c3f05f5 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -16,7 +16,7 @@ test -z \"$(git ls-files -u)\" ||\n \tdie \"You are in the middle of a conflicted merge.\"\n \n strategy_args= no_summary= no_commit= squash=\n-while case \"$#,$1\" in 0) break ;; *,-*) ;; *) break ;; esac\n+while :\n do\n \tcase \"$1\" in\n \t-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\\\n@@ -46,8 +46,8 @@ do\n \t-h|--h|--he|--hel|--help)\n \t\tusage\n \t\t;;\n-\t-*)\n-\t\t# Pass thru anything that is meant for fetch.\n+\t*)\n+\t\t# Pass thru anything that may be meant for fetch.\n \t\tbreak\n \t\t;;\n \tesac\ndiff --git a/git-quiltimport.sh b/git-quiltimport.sh\nindex 9de54d1..1ad9291 100755\n--- a/git-quiltimport.sh\n+++ b/git-quiltimport.sh\n@@ -5,7 +5,7 @@ SUBDIRECTORY_ON=Yes\n \n dry_run=\"\"\n quilt_author=\"\"\n-while case \"$#\" in 0) break;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t--au=*|--aut=*|--auth=*|--autho=*|--author=*)\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex abc2b1c..2fa53fd 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -317,7 +317,7 @@ do_rest () {\n \tdone\n }\n \n-while case $# in 0) break ;; esac\n+while test $# != 0\n do\n \tcase \"$1\" in\n \t--continue)\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex c9942f2..9779d34 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -122,15 +122,14 @@ finish_rb_merge () {\n \n is_interactive () {\n \ttest -f \"$dotest\"/interactive ||\n-\twhile case $#,\"$1\" in 0,|*,-i|*,--interactive) break ;; esac\n-\tdo\n+\twhile :; do case $#,\"$1\" in 0,|*,-i|*,--interactive) break ;; esac\n \t\tshift\n \tdone && test -n \"$1\"\n }\n \n is_interactive \"$@\" && exec git-rebase--interactive \"$@\"\n \n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t--continue)\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 156c5e8..77126cd 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -9,7 +9,7 @@ SUBDIRECTORY_OK='Yes'\n \n no_update_info= all_into_one= remove_redundant=\n local= quiet= no_reuse= extra=\n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \t-n)\tno_update_info=t ;;\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3320998..a7180ad 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -251,7 +251,7 @@ modules_list()\n \tdone\n }\n \n-while case \"$#\" in 0) break ;; esac\n+while test \"$#\" != 0\n do\n \tcase \"$1\" in\n \tadd)\n-- \n1.5.3.1.96.g4568\n"},{"id":"53858","messageId":"7vhcllc9bz.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"85myvdktb3.fsf@lola.goethe.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-23T22:20:48Z","receivedAt":"2007-09-23T22:20:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> A lot of shell scripts contained stuff starting with\n>\n> \twhile case \"$#\" in 0) break ;; esac\n>\n> and similar.  I consider breaking out of the condition instead of the\n> body od the loop ugly, and the implied \"true\" value of the\n> non-matching case is not really obvious to humans at first glance.  It\n> happens not to be obvious to some BSD shells, either, but that's\n> because they are not POSIX-compliant.  In most cases, this has been\n> replaced by a straight condition using \"test\".  \"case\" has the\n> advantage of being faster than \"test\" on vintage shells where \"test\"\n> is not a builtin.  Since none of them is likely to run the git\n> scripts, anyway, the added readability should be worth the change.\n>\n> A few loops have had their termination condition expressed\n> differently.\n>\n> Signed-off-by: David Kastrup <dak@gnu.org>\n> ---\n>\n> Ok, this is not really what we have been talking about except in one\n> case, but I think it is actually more of an improvement.\n\nGaah, didn't I say I do NOT think it is an improvement?\n\n> I consider breaking out of the condition instead of the\n> body od the loop ugly,\n\nWell, as we all know that we disagree on this point, stating\nwhat you consider one-sidedly here is quite inappropriate.\n\n> and the implied \"true\" value of the\n> non-matching case is not really obvious to humans at first\n> glance.\n\nIt is more like \"if you do not know shell\".\n\nIn other words, I am somewhat disgusted with the first part of\nyour proposed commit log message, although I like what the patch\ndoes ;-).\n\n> -while case \"$#\" in 0) break ;; esac\n> +while test \"$#\" != 0\n>  do\n>      case \"$1\" in\n>      -a)\n\nAnd let's not quote \"$#\".\n"},{"id":"53889","messageId":"20070924060521.GB10975@glandium.org","threadId":"9971","inReplyTo":"85myvdktb3.fsf@lola.goethe.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-09-24T06:05:21Z","receivedAt":"2007-09-24T06:05:21Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n> -while case $# in 0) break ;; esac\n> +while test $# != 0\n\nWouldn't -ne be better ?\n\nMike\n"},{"id":"53891","messageId":"85ps08k2fj.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"7vhcllc9bz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T06:22:40Z","receivedAt":"2007-09-24T06:22:40Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Ok, this is not really what we have been talking about except in\n>> one case, but I think it is actually more of an improvement.\n>\n> Gaah, didn't I say I do NOT think it is an improvement?\n\nAh, but I am not presuming to speak for you in my commit message and\npostings.\n\n>> I consider breaking out of the condition instead of the\n>> body od the loop ugly,\n>\n> Well, as we all know that we disagree on this point, stating\n> what you consider one-sidedly here is quite inappropriate.\n\nHm.  If I create a patch after you basically said \"go ahead, I don't\nmind, but I consider it unimportant\", how am I going to put the\nmotivation for the patch in the commit message while expressing _your_\nopinion?  I thought that using \"I\" to make clear that it is my\npersonal view would be doing that.\n\nSo what am I supposed to write instead?\n\n\"There is no good reason for this patch, but we might as well do it.\"?\n\n>> and the implied \"true\" value of the non-matching case is not really\n>> obvious to humans at first glance.\n>\n> It is more like \"if you do not know shell\".\n\nIt is more to take in.  Believe me, I do know shell.\n\n> In other words, I am somewhat disgusted with the first part of\n> your proposed commit log message, although I like what the patch\n> does ;-).\n\nCould you propose a commit message that would be acceptable to you,\nyet not make it appear like a mistake to actually commit the patch?\n\n>> -while case \"$#\" in 0) break ;; esac\n>> +while test \"$#\" != 0\n>>  do\n>>      case \"$1\" in\n>>      -a)\n>\n> And let's not quote \"$#\".\n\nI kept this as it was originally.  Some authors prefer to quote every\nshell variable as a rule in order to avoid stupid syntactic things\nhappening.  Of course, $# never needs quoting, but I did not want to\nchange the personal style of the respective authors.  I can make this\nconsistent if you want to.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53892","messageId":"85k5qgk295.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"20070924060521.GB10975@glandium.org","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T06:26:30Z","receivedAt":"2007-09-24T06:26:30Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n>> -while case $# in 0) break ;; esac\n>> +while test $# != 0\n>\n> Wouldn't -ne be better ?\n\nWhy?\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53893","messageId":"ee77f5c20709232330n7b47d9e9v38677678dbf197da@mail.gmail.com","threadId":"9971","inReplyTo":"85k5qgk295.fsf@lola.goethe.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-09-24T06:30:33Z","receivedAt":"2007-09-24T06:30:33Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n> Mike Hommey <mh@glandium.org> writes:\n>\n> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n> >> -while case $# in 0) break ;; esac\n> >> +while test $# != 0\n> >\n> > Wouldn't -ne be better ?\n>\n> Why?\n\nBecause -ne does a numeric comparison, != does a string comparison,\nand it's a numeric comparison happening, semantically speaking.\n\n\nDave.\n"},{"id":"53896","messageId":"86ejgowl5g.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"ee77f5c20709232330n7b47d9e9v38677678dbf197da@mail.gmail.com","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T07:57:31Z","receivedAt":"2007-09-24T07:57:31Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\"David Symonds\" <dsymonds@gmail.com> writes:\n\n> On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n>> Mike Hommey <mh@glandium.org> writes:\n>>\n>> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n>> >> -while case $# in 0) break ;; esac\n>> >> +while test $# != 0\n>> >\n>> > Wouldn't -ne be better ?\n>>\n>> Why?\n>\n> Because -ne does a numeric comparison, != does a string comparison,\n> and it's a numeric comparison happening, semantically speaking.\n\nI don't see the point in converting $# and 0 into numbers before\ncomparing them.  \"!=\" is quite more readable, and the old code also\ncompared the strings.\n\n-- \nDavid Kastrup\n"},{"id":"53897","messageId":"20070924080134.GA9112@artemis.corp","threadId":"9971","inReplyTo":"86ejgowl5g.fsf@lola.quinscape.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-24T08:01:34Z","receivedAt":"2007-09-24T08:01:34Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:\n> \"David Symonds\" <dsymonds@gmail.com> writes:\n> \n> > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n> >> Mike Hommey <mh@glandium.org> writes:\n> >>\n> >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n> >> >> -while case $# in 0) break ;; esac\n> >> >> +while test $# != 0\n> >> >\n> >> > Wouldn't -ne be better ?\n> >>\n> >> Why?\n> >\n> > Because -ne does a numeric comparison, != does a string comparison,\n> > and it's a numeric comparison happening, semantically speaking.\n> \n> I don't see the point in converting $# and 0 into numbers before\n> comparing them.  \"!=\" is quite more readable, and the old code also\n> compared the strings.\n\n  Fwiw $# already is a number. Hence test $# -ne 0 is definitely a\nbetter test.\n\n  $# != 0 would yield sth like (strcmp(sprintf(\"%d\", argc), \"0\"))\n  $# -ne 0 would yield sth like (argc != atoi(\"0\")).\n\n  Not that it matters much, but the latter looks better to me.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53898","messageId":"20070924080436.GB9112@artemis.corp","threadId":"9971","inReplyTo":"20070924080134.GA9112@artemis.corp","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-24T08:04:36Z","receivedAt":"2007-09-24T08:04:36Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Sep 24, 2007 at 08:01:34AM +0000, Pierre Habouzit wrote:\n> On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:\n> > \"David Symonds\" <dsymonds@gmail.com> writes:\n> > \n> > > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n> > >> Mike Hommey <mh@glandium.org> writes:\n> > >>\n> > >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n> > >> >> -while case $# in 0) break ;; esac\n> > >> >> +while test $# != 0\n> > >> >\n> > >> > Wouldn't -ne be better ?\n> > >>\n> > >> Why?\n> > >\n> > > Because -ne does a numeric comparison, != does a string comparison,\n> > > and it's a numeric comparison happening, semantically speaking.\n> > \n> > I don't see the point in converting $# and 0 into numbers before\n> > comparing them.  \"!=\" is quite more readable, and the old code also\n> > compared the strings.\n> \n>   Fwiw $# already is a number. Hence test $# -ne 0 is definitely a\n> better test.\n> \n>   $# != 0 would yield sth like (strcmp(sprintf(\"%d\", argc), \"0\"))\n>   $# -ne 0 would yield sth like (argc != atoi(\"0\")).\n\n  Of course this holds only for shell where test/[ is a builtin, which\nis the at least the case for zsh, bash, and dash (but not posh).\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53900","messageId":"867imgwjne.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"20070924080134.GA9112@artemis.corp","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T08:29:57Z","receivedAt":"2007-09-24T08:29:57Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:\n>> \"David Symonds\" <dsymonds@gmail.com> writes:\n>> \n>> > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n>> >> Mike Hommey <mh@glandium.org> writes:\n>> >>\n>> >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n>> >> >> -while case $# in 0) break ;; esac\n>> >> >> +while test $# != 0\n>> >> >\n>> >> > Wouldn't -ne be better ?\n>> >>\n>> >> Why?\n>> >\n>> > Because -ne does a numeric comparison, != does a string comparison,\n>> > and it's a numeric comparison happening, semantically speaking.\n>> \n>> I don't see the point in converting $# and 0 into numbers before\n>> comparing them.  \"!=\" is quite more readable, and the old code also\n>> compared the strings.\n>\n>   Fwiw $# already is a number.\n\nIt isn't.\n\n> Hence test $# -ne 0 is definitely a better test.\n\n/* TEST/[ builtin. */\nint\ntest_builtin (list)\n     WORD_LIST *list;\n{\n  char **argv;\n  int argc, result;\n\n  /* We let Matthew Bradburn and Kevin Braunsdorf's code do the\n     actual test command.  So turn the list of args into an array\n     of strings, since that is what their code wants. */\n  if (list == 0)\n    {\n      if (this_command_name[0] == '[' && !this_command_name[1])\n\t{\n\t  builtin_error (\"missing `]'\");\n\t  return (EX_BADUSAGE);\n\t}\n\n      return (EXECUTION_FAILURE);\n    }\n\n  argv = make_builtin_argv  (list, &argc);\n  result = test_command (argc, argv);\n  free ((char *)argv);\n\n  return (result);\n}\n\n>   $# != 0 would yield sth like (strcmp(sprintf(\"%d\", argc), \"0\"))\n>   $# -ne 0 would yield sth like (argc != atoi(\"0\")).\n>\n>   Not that it matters much, but the latter looks better to me.\n\nNot to me.  The code does not support your argument, and all $x\nexpansions certainly are strings, according to manual and usage.  I\nwill rework the patch this evening in order to get a commit message\nmore placable to Junio, and I will at his request remove all of the\n(redundant) quoting.  But removing quoting from $# does not turn it\ninto a number: it remains the same string '0'.  If someone else feels\nhe should replace all \"=\" and \"!=\" tests for \"numeric\" comparisons\nwith the unreadable numeric tests, he can go ahead proposing a\nseparate patch that should not just cover $#.\n\nYou can have bash declare numeric variables, but even they are strings\n(they just auto-evaluate on assignment):\n\ndeclare -i nonsense\nnonsense=\"3+$(echo 4)\"\necho \"$nonsense\"\n\ngives 7, even though everything has been \"strings\" here.\n\n-- \nDavid Kastrup\n"},{"id":"53904","messageId":"Pine.LNX.4.64.0709241128460.28395@racer.site","threadId":"9971","inReplyTo":"20070924080436.GB9112@artemis.corp","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-24T10:33:00Z","receivedAt":"2007-09-24T10:33:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 24 Sep 2007, Pierre Habouzit wrote:\n\n> On Mon, Sep 24, 2007 at 08:01:34AM +0000, Pierre Habouzit wrote:\n> > On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:\n> > > \"David Symonds\" <dsymonds@gmail.com> writes:\n> > > \n> > > > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:\n> > > >> Mike Hommey <mh@glandium.org> writes:\n> > > >>\n> > > >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:\n> > > >> >> -while case $# in 0) break ;; esac\n> > > >> >> +while test $# != 0\n> > > >> >\n> > > >> > Wouldn't -ne be better ?\n> > > >>\n> > > >> Why?\n> > > >\n> > > > Because -ne does a numeric comparison, != does a string comparison,\n> > > > and it's a numeric comparison happening, semantically speaking.\n> > > \n> > > I don't see the point in converting $# and 0 into numbers before\n> > > comparing them.  \"!=\" is quite more readable, and the old code also\n> > > compared the strings.\n> > \n> >   Fwiw $# already is a number. Hence test $# -ne 0 is definitely a\n> > better test.\n> > \n> >   $# != 0 would yield sth like (strcmp(sprintf(\"%d\", argc), \"0\"))\n> >   $# -ne 0 would yield sth like (argc != atoi(\"0\")).\n> \n>   Of course this holds only for shell where test/[ is a builtin, which\n> is the at least the case for zsh, bash, and dash (but not posh).\n\nThe reason we used \"case\" is that this has always been a builtin (has to \nbe, because it changes workflow).\n\nTherefore I am somewhat uneasy that the patch went in so easily, \nespecially given a message that flies in the face of our endeavours to \nmake git less dependent on any given shell (as long as it is not broken to \nbegin with).\n\nCiao,\nDscho\n"},{"id":"53910","messageId":"87ps08s3zt.fsf@catnip.gol.com","threadId":"9971","inReplyTo":"Pine.LNX.4.64.0709241128460.28395@racer.site","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2007-09-24T11:21:42Z","receivedAt":"2007-09-24T11:21:42Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>> >   $# != 0 would yield sth like (strcmp(sprintf(\"%d\", argc), \"0\"))\n>> >   $# -ne 0 would yield sth like (argc != atoi(\"0\")).\n>> \n>>   Of course this holds only for shell where test/[ is a builtin, which\n>> is the at least the case for zsh, bash, and dash (but not posh).\n>\n> The reason we used \"case\" is that this has always been a builtin (has to \n> be, because it changes workflow).\n>\n> Therefore I am somewhat uneasy that the patch went in so easily, \n> especially given a message that flies in the face of our endeavours to \n> make git less dependent on any given shell (as long as it is not broken to \n> begin with).\n\nThe comment \"... holds only for a shell where [ is a builtin\" doesn't\nmake any sense to me though:  \"-ne\" is a standard operator even if \"[\"\nisn't a builtin.  It's useful if you are testing numbers in potentially\nnon-canonical form, e.g., with leading zeroes or something, and AFAIK is\nquite portable.\n\n-Miles\n-- \nEverywhere is walking distance if you have the time.  -- Steven Wright\n"},{"id":"53911","messageId":"20070924113556.GI8111@void.codelabs.ru","threadId":"9971","inReplyTo":"87ps08s3zt.fsf@catnip.gol.com","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Eygene Ryabinkin","fromEmail":"rea-git@codelabs.ru","sentAt":"2007-09-24T11:35:56Z","receivedAt":"2007-09-24T11:35:56Z","isPatch":true,"sender":{"key":"rea-git@codelabs.ru","avatar":null},"body":"Miles,\n\nMon, Sep 24, 2007 at 08:21:42PM +0900, Miles Bader wrote:\n> > The reason we used \"case\" is that this has always been a builtin (has to \n> > be, because it changes workflow).\n> >\n> > Therefore I am somewhat uneasy that the patch went in so easily, \n> > especially given a message that flies in the face of our endeavours to \n> > make git less dependent on any given shell (as long as it is not broken to \n> > begin with).\n> \n> The comment \"... holds only for a shell where [ is a builtin\" doesn't\n> make any sense to me\n\nThe 'while case ...' construct does not invoke any external commands.\nThe 'while test ...' too, but only when 'test' is builtin.  When\n'test' is the external binary you get one additional fork/exec per\neach cycle.\n\nI believe that this trick comes from the old days where people were\ngenerally much more eager to save CPU cycles than now ;))  But if\nyou tried to run Cygwin on the moderately new machines (like PIII),\nyou still can notice that commands like 'man' are a bit slow just\nbecause they require a bunch of preprocessors (and processes in\npipe) to run.  So, \"save the process, kill the fork/exec\" ;))\n\n> though:  \"-ne\" is a standard operator even if \"[\"\n> isn't a builtin.  It's useful if you are testing numbers in potentially\n> non-canonical form, e.g., with leading zeroes or something, and AFAIK is\n> quite portable.\n\nIn general -- yes, it is portable and good for leading zeros.  But\nin the case of $#, there will hardly be leading zeros or something\nelse.  And using '!=' typically saves you one atoi() + some other\nchecks, since it typically translates to a bare strcmp().\n-- \nEygene\n"},{"id":"53912","messageId":"861wcouwbf.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"Pine.LNX.4.64.0709241128460.28395@racer.site","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T11:39:16Z","receivedAt":"2007-09-24T11:39:16Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> The reason we used \"case\" is that this has always been a builtin\n> (has to be, because it changes workflow).\n>\n> Therefore I am somewhat uneasy that the patch went in so easily,\n\nIt didn't yet.\n\n> especially given a message that flies in the face of our endeavours\n> to make git less dependent on any given shell (as long as it is not\n> broken to begin with).\n\n\"test\" is not actually a shell dependency since it is available as an\nexternal when not available as builtin.  And if you really want to\nprefer \"case\" over \"test\" because the latter is not a built-in in a\nsmall number of shells, then it should be done consistently everywhere\nand not just in code I touch.\n\n-- \nDavid Kastrup\n"},{"id":"53917","messageId":"87k5qgrxcu.fsf@catnip.gol.com","threadId":"9971","inReplyTo":"20070924113556.GI8111@void.codelabs.ru","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2007-09-24T13:45:05Z","receivedAt":"2007-09-24T13:45:05Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>> The comment \"... holds only for a shell where [ is a builtin\" doesn't\n>> make any sense to me\n>\n> The 'while case ...' construct does not invoke any external commands.\n> The 'while test ...' too, but only when 'test' is builtin.  When\n> 'test' is the external binary you get one additional fork/exec per\n> each cycle.\n\nIn practice that's not an issue though -- every reasonable shell has\ntest as a builtin these days, so the \"works when test is not a builtin\"\ncriteria is really important only for robustness.\n\n> I believe that this trick comes from the old days where people were\n> generally much more eager to save CPU cycles than now ;))\n\nYes.  I still occasionally find myself using \"case\" where if+test might\nbe more natural, but I think it's basically an anachronism these days,\nand causes more harm by reducing readability than good.\n\n-Miles\n-- \nThe car has become... an article of dress without which we feel uncertain,\nunclad, and incomplete.  [Marshall McLuhan, Understanding Media, 1964]\n"},{"id":"53922","messageId":"86k5qgtbag.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"87k5qgrxcu.fsf@catnip.gol.com","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T13:58:47Z","receivedAt":"2007-09-24T13:58:47Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Miles Bader <miles@gnu.org> writes:\n\n> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>>> The comment \"... holds only for a shell where [ is a builtin\" doesn't\n>>> make any sense to me\n>>\n>> The 'while case ...' construct does not invoke any external commands.\n>> The 'while test ...' too, but only when 'test' is builtin.  When\n>> 'test' is the external binary you get one additional fork/exec per\n>> each cycle.\n>\n> In practice that's not an issue though -- every reasonable shell has\n> test as a builtin these days, so the \"works when test is not a builtin\"\n> criteria is really important only for robustness.\n\nSince the external test is pretty standardized, \"works when test is\none of various builtins\" is actually more important for robustness...\n\n-- \nDavid Kastrup\n"},{"id":"53920","messageId":"Pine.LNX.4.64.0709241502330.28395@racer.site","threadId":"9971","inReplyTo":"87k5qgrxcu.fsf@catnip.gol.com","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-24T14:04:32Z","receivedAt":"2007-09-24T14:04:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 24 Sep 2007, Miles Bader wrote:\n\n> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n> >> The comment \"... holds only for a shell where [ is a builtin\" doesn't\n> >> make any sense to me\n> >\n> > The 'while case ...' construct does not invoke any external commands.\n> > The 'while test ...' too, but only when 'test' is builtin.  When\n> > 'test' is the external binary you get one additional fork/exec per\n> > each cycle.\n> \n> In practice that's not an issue though -- every reasonable shell has \n> test as a builtin these days, so the \"works when test is not a builtin\" \n> criteria is really important only for robustness.\n\nAAAAAAAAAAAAAARRRRRGGGHHHHHHHHHHHH!\n\n_Exactly_ the same reasoning can be said about the old code: _every_ \nreasonable shell can grok the code that used to be there!\n\n<rhetoric-question>\n\tSo what exactly was your point again?\n</rhetoric-question>\n\nCiao,\nDscho\n"},{"id":"53923","messageId":"86fy14taqr.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"Pine.LNX.4.64.0709241502330.28395@racer.site","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T14:10:36Z","receivedAt":"2007-09-24T14:10:36Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Mon, 24 Sep 2007, Miles Bader wrote:\n>\n>> Eygene Ryabinkin <rea-git@codelabs.ru> writes:\n>> >> The comment \"... holds only for a shell where [ is a builtin\" doesn't\n>> >> make any sense to me\n>> >\n>> > The 'while case ...' construct does not invoke any external commands.\n>> > The 'while test ...' too, but only when 'test' is builtin.  When\n>> > 'test' is the external binary you get one additional fork/exec per\n>> > each cycle.\n>> \n>> In practice that's not an issue though -- every reasonable shell has \n>> test as a builtin these days, so the \"works when test is not a builtin\" \n>> criteria is really important only for robustness.\n>\n> AAAAAAAAAAAAAARRRRRGGGHHHHHHHHHHHH!\n>\n> _Exactly_ the same reasoning can be said about the old code: _every_ \n> reasonable shell can grok the code that used to be there!\n\nThere are no known shells that would not grok the proposed code, and\nthe BSD shells don't grok the \"code that used to be there\".\n\nSo what point is there in preserving compatibility with some mystical\nnon-specified shell while breaking compatibility with actually\nexisting shells?\n\nAnd by using a less human-readable idiom, to boot?\n\n-- \nDavid Kastrup\n"},{"id":"53924","messageId":"86bqbsta3g.fsf@lola.quinscape.zz","threadId":"9971","inReplyTo":"85ps08k2fj.fsf@lola.goethe.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-24T14:24:35Z","receivedAt":"2007-09-24T14:24:35Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Well, as we all know that we disagree on this point, stating what\n>> you consider one-sidedly here is quite inappropriate.\n>\n> Hm.  If I create a patch after you basically said \"go ahead, I don't\n> mind, but I consider it unimportant\", how am I going to put the\n> motivation for the patch in the commit message while expressing\n> _your_ opinion?  I thought that using \"I\" to make clear that it is\n> my personal view would be doing that.\n>\n> So what am I supposed to write instead?\n>\n> \"There is no good reason for this patch, but we might as well do\n> it.\"?\n\n[...]\n\n>> In other words, I am somewhat disgusted with the first part of\n>> your proposed commit log message, although I like what the patch\n>> does ;-).\n>\n> Could you propose a commit message that would be acceptable to you,\n> yet not make it appear like a mistake to actually commit the patch?\n>\n>>> -while case \"$#\" in 0) break ;; esac\n>>> +while test \"$#\" != 0\n>>>  do\n>>>      case \"$1\" in\n>>>      -a)\n>>\n>> And let's not quote \"$#\".\n>\n> I kept this as it was originally.  Some authors prefer to quote\n> every shell variable as a rule in order to avoid stupid syntactic\n> things happening.  Of course, $# never needs quoting, but I did not\n> want to change the personal style of the respective authors.  I can\n> make this consistent if you want to.\n\nIt seems like the window of opportunity to fix the objectable commit\nmessage has closed for me, as well as doing the work of removing the\n\"$#\" (which you did already): I find that the patch has already made\nit into upstream.\n\nI am somewhat taken aback that a commit message considered offensive\n(though I still have a problem understanding why and certainly did not\nintend this) has been committed into master without giving me a chance\nto amend it.\n\nUnfortunately, the ensuing discussion around the _technical_ merits is\nsomewhat lopsided since Dscho keeps me in his killfile, and so the\ncommit message in the repository is all he'll ever be able to see from\nme concerning this matter.\n\nWhich makes it more unfortunate that I have not been able to amend it.\n\nToo bad.\n\n-- \nDavid Kastrup\n"},{"id":"53925","messageId":"878x6wrtyu.fsf@catnip.gol.com","threadId":"9971","inReplyTo":"Pine.LNX.4.64.0709241502330.28395@racer.site","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2007-09-24T14:58:17Z","receivedAt":"2007-09-24T14:58:17Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>> In practice that's not an issue though -- every reasonable shell has \n>> test as a builtin these days, so the \"works when test is not a builtin\" \n>> criteria is really important only for robustness.\n>\n> AAAAAAAAAAAAAARRRRRGGGHHHHHHHHHHHH!\n>\n> _Exactly_ the same reasoning can be said about the old code: _every_ \n> reasonable shell can grok the code that used to be there!\n\nAs has been stated, that's not true.  Some \"real\" shells don't handle\nthe old code correctly (and the old code is less readable as well).\n\nAFAIK, the new code works all cases, it's merely very slightly slower in\nunusual circumstances.\n\n-Miles\n\n-- \n/\\ /\\\n(^.^)\n(\")\")\n*This is the cute kitty virus, please copy this into your sig so it can spread.\n"},{"id":"53928","messageId":"7vbqbsav56.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"Pine.LNX.4.64.0709241502330.28395@racer.site","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-24T16:24:53Z","receivedAt":"2007-09-24T16:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Mon, 24 Sep 2007, Miles Bader wrote:\n>> ...\n>> In practice that's not an issue though -- every reasonable shell has \n>> test as a builtin these days, so the \"works when test is not a builtin\" \n>> criteria is really important only for robustness.\n>\n> AAAAAAAAAAAAAARRRRRGGGHHHHHHHHHHHH!\n>\n> _Exactly_ the same reasoning can be said about the old code: _every_ \n> reasonable shell can grok the code that used to be there!\n>\n> <rhetoric-question>\n> \tSo what exactly was your point again?\n> </rhetoric-question>\n\nThe points are:\n\n (1) The code used to be there is known to cause trouble with a\n     deployed shell on FreeBSD of some vintage.  It may be true\n     that the shell is broken, but it does not matter much to\n     the end user on such systems if breakage is in shell or in\n     scripts --- the end result is that the user cannot benefit\n     from git, and we already happen to know the workaround,\n     which does not make the scripts less readable nor less\n     portable;\n\n (2) It's not like people who work on git scripts share the\n     exact same style and tradition.  While I do not personally\n     think there is much readability improvements between the\n     old code and the new code, if more people find the latter\n     easier to work with, it's better to switch to the new code\n     especially because there is no downside.\n\n     (2-a) Nobody finds the latter less readable nor impossible\n           to work with.  Even I am not saying that; I only said\n           I do not think it improves.\n\n     (2-b) git is not an educational project. It can be done\n           elsewhere in a UNIX history class, not here, to teach\n           people that \"case ... esac\" used to be much more\n           preferred over \"test\" because often the latter was\n           not built-in and slower.\n\nRegarding \"$# != 0\" vs \"$# -ne 0\", I agree with the patch by\nDavid.  If the variable were \"$something_else\", then it might\nhave been better to use the explicitly numeric form, but I think\nany seasoned shell people is much more used to see $# and $? (or\n$status after an earlier \"status=$?\") compared with numeric\nstring with \"=\" or \"!=\".\n"},{"id":"53949","messageId":"7vodfr8wts.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"86bqbsta3g.fsf@lola.quinscape.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-24T23:31:27Z","receivedAt":"2007-09-24T23:31:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> I am somewhat taken aback that a commit message considered offensive\n> (though I still have a problem understanding why and certainly did not\n> intend this) has been committed into master without giving me a chance\n> to amend it.\n\nHeh, that's simple.  I changed my mind ;-)\n\nWhen A and B test for preconditions, and C, D, and E are\noperations with error reports as their side effects, we can\nwrite our loop in these forms:\n\n (1) while A && B && C && D && E || false; do :; done\n (2) while A && B && C && D && E || break; do :; done\n (3) while A && B; do C && D && E || break; do :; done\n (4) while :; do A && B && C && D && E || break; done\n\nand all of them are equivalent.\n\nBut obviously the only sane version is (3).\n\nIf your complaint were against things like (1) and (2), I would\nhave completely agreed with you.  If you want \"effects\", you do\nso between do and done.  Although you can use break between do\nand done if you need to conditionally break out of the loop\nafter causing some effect there, between while and do is where\nyou are only supposed to decide if you want to break out of the\nloop without causing \"effects\".\n\nBut what you were complaining about was different.\n\nIf we were to ignore broken shells that do not return success\nfrom a case statement with no matching pattern, the following\ntwo are equivalent:\n\n\twhile case \"$sth\" in foo) break ;; esac; do ...; done\n\twhile case \"$sth\" in foo) false ;; esac; do ...; done\n\nTheir \"case\" are used to decide if you want to break out of the\nloop; the former is (1) being a bit more explicit, and (2) used\nto be a bit more efficient when false was not built-in.\n\nNow the latter reason is mostly historical and it is not a valid\nreason to choose the former over the latter anymore.  But that\ndoes not make it any more confusing than the latter to a person\nwho knows what \"break\" means in a loop.  An explicit 'break' is\nstill more, eh,... explicit ;-)\n\nBut the \"break\" never was the issue.  Return value of \"case\"\nwas.\n\nThe reason I took your patch and proposed commit log message\n(almost) as-is was because you rewrote \"case\" to \"test\".  That\nIS an improvement, especially in the presense of a shell in the\nfield that does not implement case statement correctly, and you\ntalk about that in the later part of the commit log message.\n\nThe only \"offending\" part was \"I consider...ugly\", which is your\nopinion but I think you as the patch author deserve to express\nthat.  I do not think it would not have helped the FreeBSD shell\na bit if you removed that \"ugliness\" by merely replacing \"break\"\nwith \"false\", so I think the comment was not just offending but\nirrelevant, though.\n\nAll the rest of your commit message is correct.  The spec of\n\"case\" might not be obvious to everybody that it ought to return\nsuccess when no pattern matched.  And I found your wording to\nfold the bug decription of some BSD shells there amusing ;-)\n"},{"id":"53963","messageId":"85hcljgtlr.fsf@lola.goethe.zz","threadId":"9971","inReplyTo":"7vodfr8wts.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-25T06:13:52Z","receivedAt":"2007-09-25T06:13:52Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> I am somewhat taken aback that a commit message considered offensive\n>> (though I still have a problem understanding why and certainly did not\n>> intend this) has been committed into master without giving me a chance\n>> to amend it.\n>\n> Heh, that's simple.  I changed my mind ;-)\n>\n> When A and B test for preconditions, and C, D, and E are\n> operations with error reports as their side effects, we can\n> write our loop in these forms:\n>\n>  (1) while A && B && C && D && E || false; do :; done\n>  (2) while A && B && C && D && E || break; do :; done\n>  (3) while A && B; do C && D && E || break; do :; done\n>  (4) while :; do A && B && C && D && E || break; done\n>\n> and all of them are equivalent.\n>\n> But obviously the only sane version is (3).\n\nUh, it is the only version with a syntax error.\n\n> If your complaint were against things like (1) and (2), I would have\n> completely agreed with you.  If you want \"effects\", you do so\n> between do and done.  Although you can use break between do and done\n> if you need to conditionally break out of the loop after causing\n> some effect there, between while and do is where you are only\n> supposed to decide if you want to break out of the loop without\n> causing \"effects\".\n>\n> But what you were complaining about was different.\n\nBasically\n\nwhile A && B || break; do C && D && E || break; done\n\n> If we were to ignore broken shells that do not return success\n> from a case statement with no matching pattern, the following\n> two are equivalent:\n>\n> \twhile case \"$sth\" in foo) break ;; esac; do ...; done\n> \twhile case \"$sth\" in foo) false ;; esac; do ...; done\n>\n> Their \"case\" are used to decide if you want to break out of the\n> loop; the former is (1) being a bit more explicit, and (2) used\n> to be a bit more efficient when false was not built-in.\n\nAs a completely irrelevant side note: the autoconf documentation\nmentions that \"false\" is more portable than \"true\" since calling it\nreturns a non-zero exit status even when it is not installed or\nbuilt-in.\n\n> Now the latter reason is mostly historical and it is not a valid\n> reason to choose the former over the latter anymore.  But that does\n> not make it any more confusing than the latter to a person who knows\n> what \"break\" means in a loop.  An explicit 'break' is still more,\n> eh,... explicit ;-)\n>\n> But the \"break\" never was the issue.  Return value of \"case\" was.\n\nI guess this has been a misunderstanding: for me, personally, the\nbreak was the issue: I don't like breaking out of a condition, since\nbreaking for me is an action.  I just used the fact that the BSD\nshells happen not to grok the constructs (and actually through a\nsomewhat similar confusion between condition and action) to leverage\nmy dislike of this construct and propose a patch.\n\n> The reason I took your patch and proposed commit log message\n> (almost) as-is was because you rewrote \"case\" to \"test\".\n\nUhm, ok.  It was a case of realizing \"hm, this does not really look\nmuch nicer\" before I chose to switch to \"test\".  In fact, there is one\ncase statement remaining which I rewrote in the previously discussed\nmanner, and it did not strike me as being much prettier.  So maybe I\nsomewhat misjudged the core of my offended sense of aesthetics, but\nthe impetus of the discussion still carried into the commit message.\n\nAlea iacta est (\"The SHA-1 has been established\").\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"53965","messageId":"7v4phj6yxb.fsf@gitster.siamese.dyndns.org","threadId":"9971","inReplyTo":"85hcljgtlr.fsf@lola.goethe.zz","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-25T06:29:04Z","receivedAt":"2007-09-25T06:29:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> As a completely irrelevant side note: the autoconf documentation\n> mentions that \"false\" is more portable than \"true\" since calling it\n> returns a non-zero exit status even when it is not installed or\n> built-in.\n\nAh, I like that ;-)  It is obvious when you think about it, and\nit is so true but in a very twisted way...\n"},{"id":"53979","messageId":"Pine.LNX.4.64.0709251132110.28395@racer.site","threadId":"9971","inReplyTo":"7v4phj6yxb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T10:33:42Z","receivedAt":"2007-09-25T10:33:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 24 Sep 2007, Junio C Hamano wrote:\n\n> David Kastrup <dak@gnu.org> writes:\n> \n> > As a completely irrelevant side note: the autoconf documentation \n> > mentions that \"false\" is more portable than \"true\" since calling it \n> > returns a non-zero exit status even when it is not installed or \n> > built-in.\n> \n> Ah, I like that ;-)  It is obvious when you think about it, and it is so \n> true but in a very twisted way...\n\nBut would you not have to redirect stderr to /dev/null, then?\n\nIn the same vein, we could replace \"true\" by \"! false\".\n\nThat's such a good idea that I'll go and make a patch.\n\nCiao,\nDscho\n"},{"id":"53980","messageId":"46F8E71C.1070409@qumranet.com","threadId":"9971","inReplyTo":"7v4phj6yxb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Supplant the \"while case ... break ;; esac\" idiom","fromName":"Avi Kivity","fromEmail":"avi@qumranet.com","sentAt":"2007-09-25T10:46:52Z","receivedAt":"2007-09-25T10:46:52Z","isPatch":true,"sender":{"key":"avi@qumranet.com","avatar":null},"body":"Junio C Hamano wrote:\n> David Kastrup <dak@gnu.org> writes:\n>\n>   \n>> As a completely irrelevant side note: the autoconf documentation\n>> mentions that \"false\" is more portable than \"true\" since calling it\n>> returns a non-zero exit status even when it is not installed or\n>> built-in.\n>>     \n>\n> Ah, I like that ;-)  It is obvious when you think about it, and\n> it is so true but in a very twisted way...\n>\n>   \n\nYou mean, it is not false but in a twisted way, don't you?\n\n\n-- \nDo not meddle in the internals of kernels, for they are subtle and quick to panic.\n"}]}