{"thread":{"id":"29468","subject":"[PATCH 0/3] completion: trivial cleanups","startedAt":"2012-01-29T23:41:16Z","lastAt":"2012-01-30T18:21:00Z","messageCount":28,"participants":["Felipe Contreras","Jonathan Nieder","Junio C Hamano","Thomas Rast","Frans Klaver"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"183266","messageId":"1327880479-25275-1-git-send-email-felipe.contreras@gmail.com","threadId":"29468","inReplyTo":null,"subject":"[PATCH 0/3] completion: trivial cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-29T23:41:16Z","receivedAt":"2012-01-29T23:41:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"And an improvement for zsh.\n\nFelipe Contreras (3):\n  completion: be nicer with zsh\n  completion: remove old code\n  completion: remove unused code\n\n contrib/completion/git-completion.bash |   47 +++++---------------------------\n 1 files changed, 7 insertions(+), 40 deletions(-)\n\n-- \n1.7.8.3\n"},{"id":"183267","messageId":"1327880479-25275-2-git-send-email-felipe.contreras@gmail.com","threadId":"29468","inReplyTo":"1327880479-25275-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/3] completion: be nicer with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-29T23:41:17Z","receivedAt":"2012-01-29T23:41:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"And yet another bug in zsh[1] causes a mismatch; zsh seems to have\nproblem emulating wordspliting, but only when the ':' command is\ninvolved.\n\nLet's avoid it. This has the advantage that the code is now actually\nunderstandable (at least to me), while before it looked like voodoo.\n\nI found this issue because __git_compute_porcelain_commands listed all\ncommands (not only porcelain).\n\n[1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24296\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 1496c6d..7051c7a 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -676,7 +676,8 @@ __git_merge_strategies=\n # is needed.\n __git_compute_merge_strategies ()\n {\n-\t: ${__git_merge_strategies:=$(__git_list_merge_strategies)}\n+\ttest \"$__git_merge_strategies\" && return\n+\t__git_merge_strategies=$(__git_list_merge_strategies 2> /dev/null)\n }\n \n __git_complete_revlist_file ()\n@@ -854,7 +855,8 @@ __git_list_all_commands ()\n __git_all_commands=\n __git_compute_all_commands ()\n {\n-\t: ${__git_all_commands:=$(__git_list_all_commands)}\n+\ttest \"$__git_all_commands\" && return\n+\t__git_all_commands=$(__git_list_all_commands 2> /dev/null)\n }\n \n __git_list_porcelain_commands ()\n@@ -947,7 +949,8 @@ __git_porcelain_commands=\n __git_compute_porcelain_commands ()\n {\n \t__git_compute_all_commands\n-\t: ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}\n+\ttest \"$__git_porcelain_commands\" && return\n+\t__git_porcelain_commands=$(__git_list_porcelain_commands 2> /dev/null)\n }\n \n __git_pretty_aliases ()\n-- \n1.7.8.3\n"},{"id":"183268","messageId":"1327880479-25275-3-git-send-email-felipe.contreras@gmail.com","threadId":"29468","inReplyTo":"1327880479-25275-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/3] completion: remove old code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-29T23:41:18Z","receivedAt":"2012-01-29T23:41:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We don't need to check for GIT_DIR/remotes, right? This was removed long\ntime ago.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |    8 +-------\n 1 files changed, 1 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7051c7a..f7278b5 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -643,13 +643,7 @@ __git_refs_remotes ()\n \n __git_remotes ()\n {\n-\tlocal i ngoff IFS=$'\\n' d=\"$(__gitdir)\"\n-\t__git_shopt -q nullglob || ngoff=1\n-\t__git_shopt -s nullglob\n-\tfor i in \"$d/remotes\"/*; do\n-\t\techo ${i#$d/remotes/}\n-\tdone\n-\t[ \"$ngoff\" ] && __git_shopt -u nullglob\n+\tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n \tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n \t\ti=\"${i#remote.}\"\n \t\techo \"${i/.url*/}\"\n-- \n1.7.8.3\n"},{"id":"183269","messageId":"1327880479-25275-4-git-send-email-felipe.contreras@gmail.com","threadId":"29468","inReplyTo":"1327880479-25275-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/3] completion: remove unused code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-29T23:41:19Z","receivedAt":"2012-01-29T23:41:19Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No need for thus rather complicated piece of code :)\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |   30 ------------------------------\n 1 files changed, 0 insertions(+), 30 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex f7278b5..59a4650 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2730,33 +2730,3 @@ if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \\\n \t|| complete -o default -o nospace -F _git git.exe\n fi\n-\n-if [[ -n ${ZSH_VERSION-} ]]; then\n-\t__git_shopt () {\n-\t\tlocal option\n-\t\tif [ $# -ne 2 ]; then\n-\t\t\techo \"USAGE: $0 (-q|-s|-u) <option>\" >&2\n-\t\t\treturn 1\n-\t\tfi\n-\t\tcase \"$2\" in\n-\t\tnullglob)\n-\t\t\toption=\"$2\"\n-\t\t\t;;\n-\t\t*)\n-\t\t\techo \"$0: invalid option: $2\" >&2\n-\t\t\treturn 1\n-\t\tesac\n-\t\tcase \"$1\" in\n-\t\t-q)\tsetopt | grep -q \"$option\" ;;\n-\t\t-u)\tunsetopt \"$option\" ;;\n-\t\t-s)\tsetopt \"$option\" ;;\n-\t\t*)\n-\t\t\techo \"$0: invalid flag: $1\" >&2\n-\t\t\treturn 1\n-\t\tesac\n-\t}\n-else\n-\t__git_shopt () {\n-\t\tshopt \"$@\"\n-\t}\n-fi\n-- \n1.7.8.3\n"},{"id":"183275","messageId":"20120130023642.GA14986@burratino","threadId":"29468","inReplyTo":"1327880479-25275-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-30T02:36:42Z","receivedAt":"2012-01-30T02:36:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nFelipe Contreras wrote:\n\n> We don't need to check for GIT_DIR/remotes, right? This was removed long\n> time ago.\n\nI don't follow.  fetch, push, and remote still look in .git/remotes\nlike they always did, last time I checked.\n\nPerhaps you mean that /usr/share/git-core/templates/ no longer\ncontains a remotes/ directory?  That's true but not particularly\nrelevant.  A more relevant detail would be that very few people _use_\nthe .git/remotes feature, though it is not obvious to me whether that\njustifies removing this code from the git-completion script that\nalready works.\n"},{"id":"183277","messageId":"20120130025014.GA15944@burratino","threadId":"29468","inReplyTo":"1327880479-25275-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-30T02:50:15Z","receivedAt":"2012-01-30T02:50:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Felipe Contreras wrote:\n\n> No need for thus rather complicated piece of code :)\n[...]\n>  contrib/completion/git-completion.bash |   30 ------------------------------\n>  1 files changed, 0 insertions(+), 30 deletions(-)\n[...]\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2730,33 +2730,3 @@ if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n[...]\n> -if [[ -n ${ZSH_VERSION-} ]]; then\n> -\t__git_shopt () {\n[...]\n> -else\n> -\t__git_shopt () {\n> -\t\tshopt \"$@\"\n> -\t}\n> -fi\n\nWhat codebase does this apply to?  My copy of git-completion.bash\ncontains a number of calls to __git_shopt, which will fail after this\nchange.\n\nBy the way, is there any reason you did not cc this series to Gábor or\nothers who also know the completion code well?  The patches are not\nmarked with RFC/ so I assume they are intended for direct application,\nwhich seems somewhat odd to me.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"183278","messageId":"CAMP44s1H6Db6Xq_iZseXppaTwpBCeu14ySgPfmoQnpELfywQ-Q@mail.gmail.com","threadId":"29468","inReplyTo":"20120130023642.GA14986@burratino","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T03:24:23Z","receivedAt":"2012-01-30T03:24:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 4:36 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Felipe Contreras wrote:\n>\n>> We don't need to check for GIT_DIR/remotes, right? This was removed long\n>> time ago.\n>\n> I don't follow.  fetch, push, and remote still look in .git/remotes\n> like they always did, last time I checked.\n>\n> Perhaps you mean that /usr/share/git-core/templates/ no longer\n> contains a remotes/ directory?  That's true but not particularly\n> relevant.  A more relevant detail would be that very few people _use_\n> the .git/remotes feature, though it is not obvious to me whether that\n> justifies removing this code from the git-completion script that\n> already works.\n\nThe problem is all the 'nullglob' stuff. It's *a lot* of code for this\nfeature that nobody uses.\n\nOK, maybe some people use it, but most likely they are using an old\nversion of git, and thus an old version of the completion script.\n\nAnyway, aren't there easier ways to get this? Perhaps first checking\nif the directory exists, to avoid wasting cycles.\n\nSomething like:\n  test -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n\n-- \nFelipe Contreras\n"},{"id":"183279","messageId":"20120130032739.GB10618@burratino","threadId":"29468","inReplyTo":"CAMP44s1H6Db6Xq_iZseXppaTwpBCeu14ySgPfmoQnpELfywQ-Q@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-30T03:27:39Z","receivedAt":"2012-01-30T03:27:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Felipe Contreras wrote:\n\n> Anyway, aren't there easier ways to get this? Perhaps first checking\n> if the directory exists, to avoid wasting cycles.\n>\n> Something like:\n>   test -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n\nYeah, that sounds like a good idea.  Could you send a patch that does\nthat?\n\nThanks,\nJonathan\n"},{"id":"183280","messageId":"20120130032913.GC10618@burratino","threadId":"29468","inReplyTo":"20120130025014.GA15944@burratino","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-30T03:29:13Z","receivedAt":"2012-01-30T03:29:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> What codebase does this apply to?  My copy of git-completion.bash\n> contains a number of calls to __git_shopt\n\nAh, now I get it.  This would have been easier to understand if\nsquashed in with patch 2/3.\n\nAnd it certainly looks like a good change, yes. :)\n\nThanks for explaining,\nJonathan\n"},{"id":"183281","messageId":"CAMP44s1bZeednbHfqXANZR5zVVvGwjRpuV5TFmnh212FD7E-Vg@mail.gmail.com","threadId":"29468","inReplyTo":"20120130025014.GA15944@burratino","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T03:30:49Z","receivedAt":"2012-01-30T03:30:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 4:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Felipe Contreras wrote:\n>\n>> No need for thus rather complicated piece of code :)\n> [...]\n>>  contrib/completion/git-completion.bash |   30 ------------------------------\n>>  1 files changed, 0 insertions(+), 30 deletions(-)\n> [...]\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -2730,33 +2730,3 @@ if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n> [...]\n>> -if [[ -n ${ZSH_VERSION-} ]]; then\n>> -     __git_shopt () {\n> [...]\n>> -else\n>> -     __git_shopt () {\n>> -             shopt \"$@\"\n>> -     }\n>> -fi\n>\n> What codebase does this apply to?  My copy of git-completion.bash\n> contains a number of calls to __git_shopt, which will fail after this\n> change.\n\nThe latest and greatest of course:\n\nhttp://git.kernel.org/?p=git/git.git;a=blob;f=contrib/completion/git-completion.bash\n\nIt's only used in __git_remotes.\n\n> By the way, is there any reason you did not cc this series to Gábor or\n> others who also know the completion code well?  The patches are not\n> marked with RFC/ so I assume they are intended for direct application,\n> which seems somewhat odd to me.\n\nNo reason. I hope they read the mailing list, otherwise I'll resend\nand CC them. A get_maintainers script, or something like that would\nmake things easier.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"183283","messageId":"7vd3a1erwf.fsf@alter.siamese.dyndns.org","threadId":"29468","inReplyTo":"CAMP44s1H6Db6Xq_iZseXppaTwpBCeu14ySgPfmoQnpELfywQ-Q@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-30T04:27:44Z","receivedAt":"2012-01-30T04:27:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> OK, maybe some people use it, but most likely they are using an old\n> version of git, and thus an old version of the completion script.\n\nPlease adjust your attitude about backward compatibility to match the\nstandard used for other parts of Git.\n\nMost likely they are using repositories that they started using with an\nold version, but at the same time, most likely they are happily using more\nmodern version exactly because the rest of Git still support it, except\nfor the completion script after _this_ patch breaks the support.\n"},{"id":"183284","messageId":"7v8vkperli.fsf@alter.siamese.dyndns.org","threadId":"29468","inReplyTo":"1327880479-25275-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-30T04:34:17Z","receivedAt":"2012-01-30T04:34:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Let's avoid it. This has the advantage that the code is now actually\n> understandable (at least to me), while before it looked like voodoo.\n\nI am somewhat hesitant to accept a patch to shell scripts on the basis\nthat the patch author does not understand the existing constructs that\nare standard parts of shell idioms.\n\nAvoiding zsh's bug that cannot use conditional assignment on the no-op\ncolon command (if the bug is really that; it is somewhat hard to imagine\nif the bug exists only for colon command, though) *is* by itself a good\njustification for this change, even though the resulting code is harder to\nread for people who are used to read shell scripts.\n"},{"id":"183285","messageId":"7v4nvdeo23.fsf@alter.siamese.dyndns.org","threadId":"29468","inReplyTo":"7v8vkperli.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-30T05:50:44Z","receivedAt":"2012-01-30T05:50:44Z","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> Avoiding zsh's bug that cannot use conditional assignment on the no-op\n> colon command (if the bug is really that; it is somewhat hard to imagine\n> if the bug exists only for colon command, though) *is* by itself a good\n> justification for this change, even though the resulting code is harder to\n> read for people who are used to read shell scripts.\n\nJust from my curiosity, I am wondering what zsh does when given these:\n\n\tbar () { echo \"frotz nitfol xyzzy\" }\n\n\tunset foo; : ${foo:=$(bar)}; echo \"<$?,$foo>\"\n        unset foo; true ${foo:=$(bar)}; echo \"<$?,$foo>\"\n        unset foo; echo >/dev/null ${foo:=$(bar)}; echo \"<$?,$foo>\"\n\nThe first one is exactly your \"And yet another bug in zsh[1] causes a\nmismatch; zsh seems to have problem emulating wordspliting, but only when\nthe ':' command is involved.\", so we already know it \"seems to have\nproblem emulating word-splitting\" (by the way, can we replace that with\nexact description of faulty symptom? e.g. \"does not split words at $IFS\"\nmight be what you meant but still when we are assigning the result to a\nsingle variable, it is unclear how that matters).\n\nNote that I am not suggesting to rewrite the existing \": ${var:=val}\" with\n\"echo ${var:val} >/dev/null\" at all. Even if \"echo >/dev/null\" makes it\nwork as expected, your rewrite to protect it with an explicit conditional\ne.g. \"test -n ${foo:-} || foo=$(bar)\" would be a lot better than funny\nconstruct like \"echo >/dev/null ${foo:=$(bar)\", because it is not an\nestablished shell idiom to use default assignment with anything but \":\".\n\nThanks.\n"},{"id":"183288","messageId":"871uqh3a8s.fsf@thomas.inf.ethz.ch","threadId":"29468","inReplyTo":"CAMP44s1bZeednbHfqXANZR5zVVvGwjRpuV5TFmnh212FD7E-Vg@mail.gmail.com","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-01-30T07:44:35Z","receivedAt":"2012-01-30T07:44:35Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> No reason. I hope they read the mailing list, otherwise I'll resend\n> and CC them. A get_maintainers script, or something like that would\n> make things easier.\n\nI simply use\n\n  git shortlog -sn --no-merges v1.7.0.. -- contrib/completion/\n\n(In many parts the revision limiter can be omitted without losing much,\nbut e.g. here this drops Shawn who hasn't worked on it since 2009.)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"183291","messageId":"25ea208e-353d-48f7-a849-143689fb2be6@email.android.com","threadId":"29468","inReplyTo":"871uqh3a8s.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Junio C Hamano","fromEmail":"jch2355@gmail.com","sentAt":"2012-01-30T08:22:12Z","receivedAt":"2012-01-30T08:22:12Z","isPatch":true,"sender":{"key":"jch2355@gmail.com","avatar":null},"body":"\n\nThomas Rast <trast@inf.ethz.ch> wrote:\n\n>Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> No reason. I hope they read the mailing list, otherwise I'll resend\n>> and CC them. A get_maintainers script, or something like that would\n>> make things easier.\n>\n>I simply use\n>\n>  git shortlog -sn --no-merges v1.7.0.. -- contrib/completion/\n>\n>(In many parts the revision limiter can be omitted without losing much,\n>but e.g. here this drops Shawn who hasn't worked on it since 2009.)\n\nOr \"--since=1.year\", which you can keep using forever without adjusting.\n"},{"id":"183297","messageId":"CAMP44s2PsSj=mTZMtkteHnycqEXgO7YQeJzSuH9T734pFQiJMQ@mail.gmail.com","threadId":"29468","inReplyTo":"7v4nvdeo23.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T10:30:18Z","receivedAt":"2012-01-30T10:30:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 7:50 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Avoiding zsh's bug that cannot use conditional assignment on the no-op\n>> colon command (if the bug is really that; it is somewhat hard to imagine\n>> if the bug exists only for colon command, though) *is* by itself a good\n>> justification for this change, even though the resulting code is harder to\n>> read for people who are used to read shell scripts.\n>\n> Just from my curiosity, I am wondering what zsh does when given these:\n>\n>        bar () { echo \"frotz nitfol xyzzy\" }\n>\n>        unset foo; : ${foo:=$(bar)}; echo \"<$?,$foo>\"\n>        unset foo; true ${foo:=$(bar)}; echo \"<$?,$foo>\"\n>        unset foo; echo >/dev/null ${foo:=$(bar)}; echo \"<$?,$foo>\"\n\n<0,frotz nitfol xyzzy>\n<0,frotz nitfol xyzzy>\n<0,frotz nitfol xyzzy>\n\nAnd that's _without_ bash emulation.\n\nBTW. That code didn't work for me in bash (though it did in zsh), I\nhad to add a semicolon:\n\n bar () { echo \"frotz nitfol xyzzy\" ;}\n\n> The first one is exactly your \"And yet another bug in zsh[1] causes a\n> mismatch; zsh seems to have problem emulating wordspliting, but only when\n> the ':' command is involved.\", so we already know it \"seems to have\n> problem emulating word-splitting\" (by the way, can we replace that with\n> exact description of faulty symptom? e.g. \"does not split words at $IFS\"\n> might be what you meant but still when we are assigning the result to a\n> single variable, it is unclear how that matters).\n\nThat's not the problem, the problem is that this doesn't work in zsh:\n\narray=\"a b c\"\nfor i in $array; do\n echo $i\ndone\n\nThe result is \"a b c\". Unless sh emulation is on. This is the correct\nway in zsh:\n\narray=\"a b c\"\nfor i in ${=array}; do\n echo $i\ndone\n\nBut this behavior can be controlled with SH_WORD_SPLIT.\n\nAnyway, as I said, the problem is that the ':' have some problems, and\nsh emulation seems to be turned off inside such command, or at least\nSH_WORD_SPLIT was reset in my tests.\n\n-- \nFelipe Contreras\n"},{"id":"183298","messageId":"CAMP44s0rp1EwruAwMpntcUzKS=Pbe44t7Eq0OcHdH8WF7OoUhQ@mail.gmail.com","threadId":"29468","inReplyTo":"7v8vkperli.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T10:35:31Z","receivedAt":"2012-01-30T10:35:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 6:34 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Let's avoid it. This has the advantage that the code is now actually\n>> understandable (at least to me), while before it looked like voodoo.\n>\n> I am somewhat hesitant to accept a patch to shell scripts on the basis\n> that the patch author does not understand the existing constructs that\n> are standard parts of shell idioms.\n\nI have been writing shell scripts for years[1], and I have *never* had\nan encounter with ':'. vim's sh syntax doesn't seem to be prepared for\nit, and zsh's sh emulation has problems only when ':' is involved, so\nI still think ':' is quite obscure.\n\nPlus, I haven't seen ${foo:=bar} that often.\n\nIn any case, there's no need for ad hominem arguments; there is a\nproblem when using zsh, that's a fact.\n\n[1] https://www.ohloh.net/accounts/felipec/positions/total\n\n-- \nFelipe Contreras\n"},{"id":"183299","messageId":"CAMP44s2ooo1uArhhtJkX3S9N=iE4MNJivMSvr3hsOkxFmJupFA@mail.gmail.com","threadId":"29468","inReplyTo":"25ea208e-353d-48f7-a849-143689fb2be6@email.android.com","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T10:38:35Z","receivedAt":"2012-01-30T10:38:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 10:22 AM, Junio C Hamano <jch2355@gmail.com> wrote:\n> Thomas Rast <trast@inf.ethz.ch> wrote:\n>>Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> No reason. I hope they read the mailing list, otherwise I'll resend\n>>> and CC them. A get_maintainers script, or something like that would\n>>> make things easier.\n>>\n>>I simply use\n>>\n>>  git shortlog -sn --no-merges v1.7.0.. -- contrib/completion/\n>>\n>>(In many parts the revision limiter can be omitted without losing much,\n>>but e.g. here this drops Shawn who hasn't worked on it since 2009.)\n>\n> Or \"--since=1.year\", which you can keep using forever without adjusting.\n\nPerhaps something like that can be stored in a script somewhere in\ngit's codebase so that people can set sendemail.cccmd to that.\n\n-- \nFelipe Contreras\n"},{"id":"183301","messageId":"CAMP44s2j+qotu8Fb-1qq9bqHqt+ZF877YzZFXHiMo7Z_BGzTMA@mail.gmail.com","threadId":"29468","inReplyTo":"7vd3a1erwf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T10:51:04Z","receivedAt":"2012-01-30T10:51:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> OK, maybe some people use it, but most likely they are using an old\n>> version of git, and thus an old version of the completion script.\n>\n> Please adjust your attitude about backward compatibility to match the\n> standard used for other parts of Git.\n\nWhat attitude? I am simply stating a fact. How much percentage of\npeople do you think still have .git/remotes around? How many people do\nyou think have clones more than 3 years old? And how many of these\npeople would complain if remotes were not properly completed for these\nrepos?\n\nI doubt anybody would have complained, but I guess we would never\nknow, because I already proposed a solution that would work for them\nand only uses a *single* line of code, unlike the current 40 ones.\n\nI don't see what is the problem with the attitude of sending a patch\nto remove code that most likely nobody cares about (neither you or I\nhave numbers on this), and then finding an alternative when people do\ncare about it.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"183305","messageId":"CAH6sp9Of2rT4ESMYj9kC2NPtapsN58X3A0FpHTTZO-kSqpb-2Q@mail.gmail.com","threadId":"29468","inReplyTo":"CAMP44s2j+qotu8Fb-1qq9bqHqt+ZF877YzZFXHiMo7Z_BGzTMA@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2012-01-30T11:19:21Z","receivedAt":"2012-01-30T11:19:21Z","isPatch":true,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"Hi,\n\nOn Mon, Jan 30, 2012 at 11:51 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Mon, Jan 30, 2012 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> OK, maybe some people use it, but most likely they are using an old\n>>> version of git, and thus an old version of the completion script.\n>>\n>> Please adjust your attitude about backward compatibility to match the\n>> standard used for other parts of Git.\n>\n> What attitude?\n\nThis attitude:\n\n> I am simply stating a fact. How much percentage of\n> people do you think still have .git/remotes around? How many people do\n> you think have clones more than 3 years old? And how many of these\n> people would complain if remotes were not properly completed for these\n> repos?\n>\n> I doubt anybody would have complained, but I guess we would never\n> know, because I already proposed a solution that would work for them\n> and only uses a *single* line of code, unlike the current 40 ones.\n>\n> I don't see what is the problem with the attitude of sending a patch\n> to remove code that most likely nobody cares about (neither you or I\n> have numbers on this), and then finding an alternative when people do\n> care about it.\n\nI don't think Junio actually meant an \"attitude\", but just your angle\nof approach (== attitude) on backwards compatibility.\n\nMaybe numbers for this could be generated from the next git user\nsurvey. If numbers justify this change, maybe this or something like\nit could be scheduled for a major release of git.\n\nCheers,\nFrans\n"},{"id":"183316","messageId":"CAMP44s3a05dZqOqpDFDnWQ_C03EODgeP1eRhko-Mc8OjGXj6FQ@mail.gmail.com","threadId":"29468","inReplyTo":"CAH6sp9Of2rT4ESMYj9kC2NPtapsN58X3A0FpHTTZO-kSqpb-2Q@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T11:55:33Z","receivedAt":"2012-01-30T11:55:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 1:19 PM, Frans Klaver <fransklaver@gmail.com> wrote:\n> On Mon, Jan 30, 2012 at 11:51 AM, Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n>> On Mon, Jan 30, 2012 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> OK, maybe some people use it, but most likely they are using an old\n>>>> version of git, and thus an old version of the completion script.\n>>>\n>>> Please adjust your attitude about backward compatibility to match the\n>>> standard used for other parts of Git.\n>>\n>> What attitude?\n>\n> This attitude:\n>\n>> I am simply stating a fact. How much percentage of\n>> people do you think still have .git/remotes around? How many people do\n>> you think have clones more than 3 years old? And how many of these\n>> people would complain if remotes were not properly completed for these\n>> repos?\n>>\n>> I doubt anybody would have complained, but I guess we would never\n>> know, because I already proposed a solution that would work for them\n>> and only uses a *single* line of code, unlike the current 40 ones.\n>>\n>> I don't see what is the problem with the attitude of sending a patch\n>> to remove code that most likely nobody cares about (neither you or I\n>> have numbers on this), and then finding an alternative when people do\n>> care about it.\n>\n> I don't think Junio actually meant an \"attitude\", but just your angle\n> of approach (== attitude) on backwards compatibility.\n\nWe are not talking about backwards compatibility; we are talking about\ncompatibility of remotes completion of the bash completion script of\nrepositories more than 3 years old with remotes that haven't been\nmigrated.\n\nThis barely resembles the git-foo -> 'git foo', which truly broke\nbackwards compatibility, and at the time I proposed many different\napproaches to deal with these type of problems, which seem to be\nfollowed now (although probably not because of my recommendations).\n\nBut this has nothing to do with _attitude_; I am merely stating fact.\nI have never expressed any opinion or attitude with respect to how\nbackwards compatibility should be handled in this thread, have I?\n\n> Maybe numbers for this could be generated from the next git user\n> survey. If numbers justify this change, maybe this or something like\n> it could be scheduled for a major release of git.\n\nMaybe, but I doubt this issue hardly deserves much discussion.\n\nNobody is proposing to break backwards compatibility--as you can see,\nI already proposed a simple solution that should work.\n\nAnd FTR, when I wrote 'We don't need to check for GIT_DIR/remotes,\nright? This was removed long time ago.\" I clearly wasn't sure if\n.git/remotes was still used or not, after Jonathan Nieder replied, I\nchecked the source code of remotes.c, and I found that it was still\nsupported, so I wrote the proposed alternative.\n\n-- \nFelipe Contreras\n"},{"id":"183318","messageId":"CAH6sp9PfVTTNL218syf-MS465M+sP4E8eVxuVCHZC0geE3ezfg@mail.gmail.com","threadId":"29468","inReplyTo":"CAMP44s3a05dZqOqpDFDnWQ_C03EODgeP1eRhko-Mc8OjGXj6FQ@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2012-01-30T12:21:13Z","receivedAt":"2012-01-30T12:21:13Z","isPatch":true,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"On Mon, Jan 30, 2012 at 12:55 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n\n> We are not talking about backwards compatibility; we are talking about\n> compatibility of remotes completion of the bash completion script of\n> repositories more than 3 years old with remotes that haven't been\n> migrated.\n\nWhat's not backward about that?\n\n\n> This barely resembles the git-foo -> 'git foo', which truly broke\n> backwards compatibility, and at the time I proposed many different\n> approaches to deal with these type of problems, which seem to be\n> followed now (although probably not because of my recommendations).\n>\n> But this has nothing to do with _attitude_; I am merely stating fact.\n> I have never expressed any opinion or attitude with respect to how\n> backwards compatibility should be handled in this thread, have I?\n\nAs far as I know you haven't explicitly said anything about that.\nThere may still be a possibility that the sentence Junio quoted in his\nreply could have implied a certain attitude.\n\n>> Maybe numbers for this could be generated from the next git user\n>> survey. If numbers justify this change, maybe this or something like\n>> it could be scheduled for a major release of git.\n>\n> Maybe, but I doubt this issue hardly deserves much discussion.\n\nI wouldn't know about that. Apparently not everybody is happy with\napplying it without further discussion.\n\nCheers,\nFrans\n"},{"id":"183319","messageId":"87pqe1nx9a.fsf@thomas.inf.ethz.ch","threadId":"29468","inReplyTo":"CAMP44s2ooo1uArhhtJkX3S9N=iE4MNJivMSvr3hsOkxFmJupFA@mail.gmail.com","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-01-30T13:19:29Z","receivedAt":"2012-01-30T13:19:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Mon, Jan 30, 2012 at 10:22 AM, Junio C Hamano <jch2355@gmail.com> wrote:\n>> Thomas Rast <trast@inf.ethz.ch> wrote:\n>>>Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> No reason. I hope they read the mailing list, otherwise I'll resend\n>>>> and CC them. A get_maintainers script, or something like that would\n>>>> make things easier.\n>>>\n>>>I simply use\n>>>\n>>>  git shortlog -sn --no-merges v1.7.0.. -- contrib/completion/\n>>>\n>>>(In many parts the revision limiter can be omitted without losing much,\n>>>but e.g. here this drops Shawn who hasn't worked on it since 2009.)\n>>\n>> Or \"--since=1.year\", which you can keep using forever without adjusting.\n>\n> Perhaps something like that can be stored in a script somewhere in\n> git's codebase so that people can set sendemail.cccmd to that.\n\nUmm, that seems rather AI-complete.  You should always compile the list\nby hand.\n\nFor example, the list in this case started\n\n      25  SZEDER Gábor\n       4  Michael J Gruber\n       3  Teemu Matilainen\n       3  Thomas Rast\n\nWould you Cc Michael, Teemu and me?  Probably not.  What if it started\n\n       5  SZEDER Gábor\n       4  Michael J Gruber\n       3  Teemu Matilainen\n       3  Thomas Rast\n\nAlso, something I didn't mention so far was that you may be patching\nsquarely into the code of one contributor, even if he only had a single\npatch in that area.  To catch this, you should blame the code you are\nfixing (you already checked the message of the commit to verify whether\nthe bug/feature was intentional, right?).  On top of that, the patch may\nhave involved a large number of people not listed in the Author field.\nAs a random example,\n\n  $ git shortlog -sn --no-merges v1.7.0..origin/next -- grep.[ch] builtin/grep.[ch]\n      15  René Scharfe\n       9  Junio C Hamano\n       8  Nguyễn Thái Ngọc Duy\n       5  Michał Kiedrowicz\n       4  Johannes Schindelin\n       3  Jeff King\n       3  Thomas Rast\n\nbut if you were to submit a patch that disputes the case made by\n53b8d931, you should probably cc René, Peff and me (see the Helped-by\nlines).\n\nOk, this got rather long-winded.  But I think the bottom line is, trying\nto put this in sendemail.cccmd is trying to script common sense.\n\n--\nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"183322","messageId":"CAMP44s2OQY3Pym5uqxvEE_yYa0xLboT=mjv+DFCDo8_xobGO3w@mail.gmail.com","threadId":"29468","inReplyTo":"87pqe1nx9a.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 3/3] completion: remove unused code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T13:51:48Z","receivedAt":"2012-01-30T13:51:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 3:19 PM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Mon, Jan 30, 2012 at 10:22 AM, Junio C Hamano <jch2355@gmail.com> wrote:\n>>> Thomas Rast <trast@inf.ethz.ch> wrote:\n>>>>Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>>\n>>>>> No reason. I hope they read the mailing list, otherwise I'll resend\n>>>>> and CC them. A get_maintainers script, or something like that would\n>>>>> make things easier.\n>>>>\n>>>>I simply use\n>>>>\n>>>>  git shortlog -sn --no-merges v1.7.0.. -- contrib/completion/\n>>>>\n>>>>(In many parts the revision limiter can be omitted without losing much,\n>>>>but e.g. here this drops Shawn who hasn't worked on it since 2009.)\n>>>\n>>> Or \"--since=1.year\", which you can keep using forever without adjusting.\n>>\n>> Perhaps something like that can be stored in a script somewhere in\n>> git's codebase so that people can set sendemail.cccmd to that.\n>\n> Umm, that seems rather AI-complete.  You should always compile the list\n> by hand.\n\nWhy? Take a look at the Linux kernel; having tons of contributors,\nmany still haven't learned the ropes, and looking at MAINTAIERS, plus\nthe output of 'git blame', for potentially dozens of patches is too\nburdensome, which is why they have 'scripts/get_maintainer.pl' that\ndoes a pretty good job of figuring out who to cc so you don't have to\nthink about it.\n\n> Ok, this got rather long-winded.  But I think the bottom line is, trying\n> to put this in sendemail.cccmd is trying to script common sense.\n\nIt's still better than nothing.\n\nI once wrote a much smarter script[1], but it never go into the tree.\n\nThe output I get is this:\n\"Shawn O. Pearce\" <spearce@spearce.org>>\n\"Jonathan Nieder\" <jrnieder@gmail.com>\n\"Mark Lodato\" <lodatom@gmail.com>\n\"Junio C Hamano\" <junkio@cox.net>\n\"Ted Pavlic\" <ted@tedpavlic.com>\n\nNote: seems like there's a bug with git blame -P:\n% g blame -p -L 2730,+33 contrib/completion/git-completion.bash | grep\nauthor-mail\n\nAnd if this script is such a bad idea; why do you think sendmail.cccmd exists?\n\nI think we should have a simple script that at least does something\nsensible, at least in contrib, but I hope we could even have a\nstandard git-cccmd that all projects could use.\n\nIt looks like my ruby script never had much of a chance getting\nanywhere, so would it be accepted in another format? perl? python?\nbash?\n\nCheers.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/130391\n\n-- \nFelipe Contreras\n"},{"id":"183323","messageId":"CAMP44s3vXSJaXiQK4X0kNOECzfLFsTo1YeMCtVZ0NWY-CHJ++A@mail.gmail.com","threadId":"29468","inReplyTo":"CAH6sp9PfVTTNL218syf-MS465M+sP4E8eVxuVCHZC0geE3ezfg@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T13:59:33Z","receivedAt":"2012-01-30T13:59:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 2:21 PM, Frans Klaver <fransklaver@gmail.com> wrote:\n> On Mon, Jan 30, 2012 at 12:55 PM, Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n>\n>> We are not talking about backwards compatibility; we are talking about\n>> compatibility of remotes completion of the bash completion script of\n>> repositories more than 3 years old with remotes that haven't been\n>> migrated.\n>\n> What's not backward about that?\n\nNot all backwards compatibility issues are the same.\n\n>> This barely resembles the git-foo -> 'git foo', which truly broke\n>> backwards compatibility, and at the time I proposed many different\n>> approaches to deal with these type of problems, which seem to be\n>> followed now (although probably not because of my recommendations).\n>>\n>> But this has nothing to do with _attitude_; I am merely stating fact.\n>> I have never expressed any opinion or attitude with respect to how\n>> backwards compatibility should be handled in this thread, have I?\n>\n> As far as I know you haven't explicitly said anything about that.\n> There may still be a possibility that the sentence Junio quoted in his\n> reply could have implied a certain attitude.\n\nI already asked, but I ask again; what would be that attitude? Not\ncaring about backwards compatibility? Then that implication would have\nbeen wrong.\n\nIf you look a few lines below, you would see a change that doesn't\nbreak backwards compatibility, which proves the previous implication\nwrong... Not to mention previous discussions.\n\n>>> Maybe numbers for this could be generated from the next git user\n>>> survey. If numbers justify this change, maybe this or something like\n>>> it could be scheduled for a major release of git.\n>>\n>> Maybe, but I doubt this issue hardly deserves much discussion.\n>\n> I wouldn't know about that. Apparently not everybody is happy with\n> applying it without further discussion.\n\nJonathan Nieder is happy with the 'ls -1 \"$d/remotes\"' change, and I\nhaven't seen anybody object it.\n\nEither way. I'm not going to discuss in this thread any more. I'll\nresend the patches, feel free to comment there.\n\n-- \nFelipe Contreras\n"},{"id":"183324","messageId":"20120130140204.GD10618@burratino","threadId":"29468","inReplyTo":"CAMP44s3vXSJaXiQK4X0kNOECzfLFsTo1YeMCtVZ0NWY-CHJ++A@mail.gmail.com","subject":"Re: [PATCH 2/3] completion: remove old code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-30T14:02:04Z","receivedAt":"2012-01-30T14:02:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Felipe Contreras wrote:\n\n> Either way. I'm not going to discuss in this thread any more. I'll\n> resend the patches, feel free to comment there.\n\nGood idea.  Just for the record, I'm not happy with any patch until\nI've seen the code. ;-)\n\nThanks,\nJonathan\n"},{"id":"183346","messageId":"7vpqe1cbds.fsf@alter.siamese.dyndns.org","threadId":"29468","inReplyTo":"CAMP44s0rp1EwruAwMpntcUzKS=Pbe44t7Eq0OcHdH8WF7OoUhQ@mail.gmail.com","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-30T18:07:27Z","receivedAt":"2012-01-30T18:07:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> In any case, there's no need for ad hominem arguments; there is a\n> problem when using zsh, that's a fact.\n\nThere was no ad-hominem argument at all.\n\nRead your two lines I quoted \"... the code is now actually understandable\n(at least to me), while before it looked like voodoo\", which was your\nwords.  What does it tell the reader?  The patch author (1) did not\nunderstand existing code (voodoo) and (2) the change is a good thing as a\nstyle/readability improvement.\n\nI was saying that I did not want to see that in the justification, because\n(2) is not true, while (1) may be.\n\nThe patch as-is is a good change that works around issues with zsh's POSIX\nemulation, and that is sufficient-enough justification. IOW, we are in\nagreement on the later half of your sentence.\n"},{"id":"183352","messageId":"CAMP44s1q1RuHNo5O2rMDF_YHefagaXXwxg+5Lc4DCF7mZYm20w@mail.gmail.com","threadId":"29468","inReplyTo":"7vpqe1cbds.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] completion: be nicer with zsh","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-30T18:21:00Z","receivedAt":"2012-01-30T18:21:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jan 30, 2012 at 8:07 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> In any case, there's no need for ad hominem arguments; there is a\n>> problem when using zsh, that's a fact.\n>\n> There was no ad-hominem argument at all.\n>\n> Read your two lines I quoted \"... the code is now actually understandable\n> (at least to me), while before it looked like voodoo\", which was your\n> words.  What does it tell the reader?  The patch author (1) did not\n> understand existing code (voodoo) and (2) the change is a good thing as a\n> style/readability improvement.\n\nI disagree. Another possibility is that the code actually looked like\nvoodoo (it was obfuscated). You might disagree, but the fact that one\nof the main editors (the most used?) doesn't even recognize the syntax\nof this code I think is pretty telling.\n\n> I was saying that I did not want to see that in the justification, because\n> (2) is not true, while (1) may be.\n\nThat's not true: (2) might be true; at least it's debatable.\n\n> The patch as-is is a good change that works around issues with zsh's POSIX\n> emulation, and that is sufficient-enough justification. IOW, we are in\n> agreement on the later half of your sentence.\n\nSo, I shall just remove that part of the explanation?\n\n-- \nFelipe Contreras\n"}]}