{"thread":{"id":"34010","subject":"[PATCH 0/6] Minor prompt, completion cleanups","startedAt":"2013-06-02T14:03:36Z","lastAt":"2013-06-09T20:56:20Z","messageCount":33,"participants":["Ramkumar Ramachandra","Felipe Contreras","Thomas Rast","Junio C Hamano","SZEDER Gábor","Jeff King","Peter Krefting"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"219160","messageId":"1370181822-23450-1-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":null,"subject":"[PATCH 0/6] Minor prompt, completion cleanups","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:36Z","receivedAt":"2013-06-02T14:03:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Another lazy Sunday afternoon. All of these are trivial.\n\nThanks.\n\nRamkumar Ramachandra (6):\n prompt: don't scream continuation state\n completion: add common options for rev-parse\n completion: add common options for blame\n completion: correct completion for format-patch\n completion: clarify difftool completion\n completion: clarify ls-tree, archive, show completion\n\n contrib/completion/git-completion.bash | 37 +++++++++++++++++++++++++---------\n contrib/completion/git-prompt.sh | 18 ++++++++---------\n 2 files changed, 36 insertions(+), 19 deletions(-)\n\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219161","messageId":"1370181822-23450-2-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] prompt: don't scream continuation state","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:37Z","receivedAt":"2013-06-02T14:03:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, when performing any operation that saves the state and\nexpects the user the continue (like rebase, bisect, am), the prompt\nscreams:\n\n artagnon|completion|REBASE-i 2/2:~/src/git$\n\nLowercase the words, so we get a more pleasant\n\n artagnon|completion|rebase-i 2/2:~/src/git$\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-prompt.sh | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex eaf5c36..f99d1f2 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -270,29 +270,29 @@ __git_ps1 ()\n \t\t\tstep=$(cat \"$g/rebase-merge/msgnum\")\n \t\t\ttotal=$(cat \"$g/rebase-merge/end\")\n \t\t\tif [ -f \"$g/rebase-merge/interactive\" ]; then\n-\t\t\t\tr=\"|REBASE-i\"\n+\t\t\t\tr=\"|rebase-i\"\n \t\t\telse\n-\t\t\t\tr=\"|REBASE-m\"\n+\t\t\t\tr=\"|rebase-m\"\n \t\t\tfi\n \t\telse\n \t\t\tif [ -d \"$g/rebase-apply\" ]; then\n \t\t\t\tstep=$(cat \"$g/rebase-apply/next\")\n \t\t\t\ttotal=$(cat \"$g/rebase-apply/last\")\n \t\t\t\tif [ -f \"$g/rebase-apply/rebasing\" ]; then\n-\t\t\t\t\tr=\"|REBASE\"\n+\t\t\t\t\tr=\"|rebase\"\n \t\t\t\telif [ -f \"$g/rebase-apply/applying\" ]; then\n-\t\t\t\t\tr=\"|AM\"\n+\t\t\t\t\tr=\"|am\"\n \t\t\t\telse\n-\t\t\t\t\tr=\"|AM/REBASE\"\n+\t\t\t\t\tr=\"|am/rebase\"\n \t\t\t\tfi\n \t\t\telif [ -f \"$g/MERGE_HEAD\" ]; then\n-\t\t\t\tr=\"|MERGING\"\n+\t\t\t\tr=\"|merge\"\n \t\t\telif [ -f \"$g/CHERRY_PICK_HEAD\" ]; then\n-\t\t\t\tr=\"|CHERRY-PICKING\"\n+\t\t\t\tr=\"|cherry-pick\"\n \t\t\telif [ -f \"$g/REVERT_HEAD\" ]; then\n-\t\t\t\tr=\"|REVERTING\"\n+\t\t\t\tr=\"|revert\"\n \t\t\telif [ -f \"$g/BISECT_LOG\" ]; then\n-\t\t\t\tr=\"|BISECTING\"\n+\t\t\t\tr=\"|bisect\"\n \t\t\tfi\n \n \t\t\tb=\"$(git symbolic-ref HEAD 2>/dev/null)\" || {\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219162","messageId":"1370181822-23450-3-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] completion: add common options for rev-parse","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:38Z","receivedAt":"2013-06-02T14:03:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add support for completing 'git rev-parse'. List only the options that\nare likely to be used on the command-line, as opposed to scripts.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-completion.bash | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 1c35eef..139f48e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2299,6 +2299,17 @@ _git_reset ()\n \t__gitcomp_nl \"$(__git_refs)\"\n }\n \n+_git_rev_parse ()\n+{\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp \"--show-toplevel --is-inside-work-tree --symbolic-full-name --verify\"\n+\t\treturn\n+\t\t;;\n+\tesac\n+\t__gitcomp_nl \"$(__git_refs)\"\n+}\n+\n _git_revert ()\n {\n \tcase \"$cur\" in\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219163","messageId":"1370181822-23450-4-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] completion: add common options for blame","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:39Z","receivedAt":"2013-06-02T14:03:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add support for completing 'git blame'. List only the common short\noptions.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-completion.bash | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 139f48e..a003b81 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1034,6 +1034,17 @@ _git_bisect ()\n \tesac\n }\n \n+_git_blame ()\n+{\n+\t__git_has_doubledash && return\n+\n+\tcase \"$cur\" in\n+\t-*)\n+\t\t__gitcomp \"-M -C -L -s -w\"\n+\tesac\n+\t__git_complete_revlist_file\n+}\n+\n _git_branch ()\n {\n \tlocal i c=1 only_local_ref=\"n\" has_r=\"n\"\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219164","messageId":"1370181822-23450-5-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] completion: correct completion for format-patch","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:40Z","receivedAt":"2013-06-02T14:03:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, the completion for 'git format-patch' uses\n__git_complete_revlist. Although this is technically correct, and you\ncan\n\n $ git format-patch master contrib\n\nwhere master is a ref and contrib is a pathspec, just like in 'git log',\nthe usage is unidiomatic and undocumented. 'git format-patch' is used\nwithout pathspec filtering most of the time, and it makes sense to\nprovide sensible completions using __git_refs.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex a003b81..f46964d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1347,7 +1347,7 @@ _git_format_patch ()\n \t\treturn\n \t\t;;\n \tesac\n-\t__git_complete_revlist\n+\t__gitcomp_nl \"$(__git_refs)\"\n }\n \n _git_fsck ()\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219166","messageId":"1370181822-23450-6-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] completion: clarify difftool completion","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:41Z","receivedAt":"2013-06-02T14:03:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"'git difftool' is clearly a frontend to 'git diff' and is used in\nexactly the same way, but it uses a misleading completion function name\n__git_complete_file (aliased to to __git_complete_revlist_file). Change\nit to use __git_complete_revlist_file, just like 'git diff'. No\nfunctional changes.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex f46964d..8d70c30 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1305,7 +1305,7 @@ _git_difftool ()\n \t\treturn\n \t\t;;\n \tesac\n-\t__git_complete_file\n+\t__git_complete_revlist_file\n }\n \n __git_fetch_options=\"\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219165","messageId":"1370181822-23450-7-git-send-email-artagnon@gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T14:03:42Z","receivedAt":"2013-06-02T14:03:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, the 'git ls-tree', 'git archive', and 'git show' completions\nuse __git_complete_file (aliased to __git_complete_revlist_file).\n\nIn the case of 'git ls-tree' and 'git archive', they necessarily require\na tree-ish argument (and optionally a pathspec filter, or \"file\nargument\"):\n\n $ git ls-tree hot-branch git.c\n $ git archive HEAD~4 git.c\n\nSo, __git_complete_file is a misleading name.\n\nIn the case of 'git show', it can take a pathspec and default the\nrevision to HEAD like:\n\n $ git show git.c\n\n(which is useful if git.c was modified in HEAD)\n\nHowever, this usage is not idiomatic at all. The more common usage is\nlike:\n\n $ git show HEAD~1\n $ git show origin/pu:git.c\n\nSo, __git_complete_file is again a poor name.\n\nReplace these three instances of __git_complete_file with\n__git_complete_revlist_file, without making any functional changes.\n\nRemove __git_complete_file, as it has no other callers.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/completion/git-completion.bash | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 8d70c30..84d1548 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -592,11 +592,6 @@ __git_complete_diff_index_file ()\n \tesac\n }\n \n-__git_complete_file ()\n-{\n-\t__git_complete_revlist_file\n-}\n-\n __git_complete_revlist ()\n {\n \t__git_complete_revlist_file\n@@ -1007,7 +1002,7 @@ _git_archive ()\n \t\treturn\n \t\t;;\n \tesac\n-\t__git_complete_file\n+\t__git_complete_revlist_file\n }\n \n _git_bisect ()\n@@ -1476,7 +1471,7 @@ _git_ls_remote ()\n \n _git_ls_tree ()\n {\n-\t__git_complete_file\n+\t__git_complete_revlist_file\n }\n \n # Options that go well for log, shortlog and gitk\n@@ -2382,7 +2377,7 @@ _git_show ()\n \t\treturn\n \t\t;;\n \tesac\n-\t__git_complete_file\n+\t__git_complete_revlist_file\n }\n \n _git_show_branch ()\n-- \n1.8.3.457.g2410d5e\n"},{"id":"219176","messageId":"CAMP44s16s3Uz6ybjYuqni9JdDX0XvG7F3z7F1cyNS2k=AqrkKQ@mail.gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] completion: correct completion for format-patch","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-02T17:20:28Z","receivedAt":"2013-06-02T17:20:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 2, 2013 at 9:03 AM, Ramkumar Ramachandra wrote:\n> Currently, the completion for 'git format-patch' uses\n> __git_complete_revlist. Although this is technically correct, and you\n> can\n>\n> $ git format-patch master contrib\n>\n> where master is a ref and contrib is a pathspec, just like in 'git log',\n> the usage is unidiomatic and undocumented. 'git format-patch' is used\n> without pathspec filtering most of the time, and it makes sense to\n> provide sensible completions using __git_refs.\n\nThis breaks 'git format-patch master..'.\n\nMoreover, this is a perfectly fine usage of 'git format-patch':\n\n% git format-patch --full-diff master..fc/remote/hg-next --\ncontrib/remote-helpers/git-remote-bzr\n\nPlus, even even with your patch 'contrib' will be completed regardless\n(by default completion), wouldn't it?\n\nNAK.\n\n-- \nFelipe Contreras\n"},{"id":"219178","messageId":"CALkWK0=x3pTPwAkcj-FU6KwZcyxbxB4Ou0TpxKmrxUH7fG0UGA@mail.gmail.com","threadId":"34010","inReplyTo":"CAMP44s16s3Uz6ybjYuqni9JdDX0XvG7F3z7F1cyNS2k=AqrkKQ@mail.gmail.com","subject":"Re: [PATCH 4/6] completion: correct completion for format-patch","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-02T17:29:57Z","receivedAt":"2013-06-02T17:29:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> This breaks 'git format-patch master..'.\n\nOh, ouch.\n\n> Moreover, this is a perfectly fine usage of 'git format-patch':\n>\n> % git format-patch --full-diff master..fc/remote/hg-next --\n> contrib/remote-helpers/git-remote-bzr\n\nNever mind then. Drop this patch.\n"},{"id":"219215","messageId":"87r4gjzi3d.fsf@linux-k42r.v.cablecom.net","threadId":"34010","inReplyTo":"1370181822-23450-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-03T08:58:14Z","receivedAt":"2013-06-03T08:58:14Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Currently, when performing any operation that saves the state and\n> expects the user the continue (like rebase, bisect, am), the prompt\n> screams:\n>\n> artagnon|completion|REBASE-i 2/2:~/src/git$\n>\n> Lowercase the words, so we get a more pleasant\n>\n> artagnon|completion|rebase-i 2/2:~/src/git$\n\nSo I'm not sure whether this falls under bikeshedding or actual\nfeatures, but I like the screaming. When I first saw these displays, I\nthought that the point was that this display is more important than all\nthe others, so it should be louder and easy to distinguish from the\nbranch name (common ones, anyway).\n\nDo you have other ways of distinguishing the branch and the state?\nColors? I'm a bit too lazy to check. Perhaps it could be made to only\nuse caps if not in colored mode?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"219216","messageId":"87mwr7zhv5.fsf@linux-k42r.v.cablecom.net","threadId":"34010","inReplyTo":"1370181822-23450-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-03T09:03:10Z","receivedAt":"2013-06-03T09:03:10Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Add support for completing 'git blame'. List only the common short\n> options.\n[...]\n> +_git_blame ()\n> +{\n> +\t__git_has_doubledash && return\n> +\n> +\tcase \"$cur\" in\n> +\t-*)\n> +\t\t__gitcomp \"-M -C -L -s -w\"\n> +\tesac\n> +\t__git_complete_revlist_file\n> +}\n\nIs this the first time we introduce completion (I guess you could call\nit \"help\") for short options? I only did a quick search for /-. -/ but\nit certainly seems that way.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"219217","messageId":"CALkWK0kR2fCQcO9XurzjOGF19vaC67p5kMpkJL=Vbu1Ovku18w@mail.gmail.com","threadId":"34010","inReplyTo":"87mwr7zhv5.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-03T09:32:00Z","receivedAt":"2013-06-03T09:32:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> Is this the first time we introduce completion (I guess you could call\n> it \"help\") for short options? I only did a quick search for /-. -/ but\n> it certainly seems that way.\n\nYeah. We generally prefer the long-form equivalents while doing\ncompletions, but these blame options do not have equivalent\nlong-forms.\n"},{"id":"219218","messageId":"CALkWK0=LzUe0k7rWcrbiiTZNOOGg+=SsQB3u2F2ysjb5AJ=qZw@mail.gmail.com","threadId":"34010","inReplyTo":"87r4gjzi3d.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-03T09:47:27Z","receivedAt":"2013-06-03T09:47:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> Do you have other ways of distinguishing the branch and the state?\n> Colors? I'm a bit too lazy to check. Perhaps it could be made to only\n> use caps if not in colored mode?\n\nCurrently, no. See git-prompt.sh:401, 403, 409; we don't have a\nseparate color for $r. I didn't introduce a color in this series\nbecause it would conflict with rr/zsh-color-prompt which is in pu: we\ncan introduce it after that graduates. I was thinking yellow, since\nthat's not taken?\n\nYou really should use colors. I don't think it's worth the extra\nugliness to scream in the no-color case.\n\nAs such, the prompt is a fine bikeshedding target. I even introduced\nGIT_PS1_SEPARATOR, because some people were unhappy with me stripping\none whitespace (yes, one). If we go down the configurability road,\nwe'll either end up with a ton of environment variables, or be made to\nwrite a generalized custom formatter which splices together tons of\narguments using color (super painful). While I do agree that it is a\nmatter of taste, we have to make a few compromises for the sake of\nsanity.\n"},{"id":"219250","messageId":"7vr4gjglbi.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"87r4gjzi3d.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T17:23:29Z","receivedAt":"2013-06-03T17:23:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> Currently, when performing any operation that saves the state and\n>> expects the user the continue (like rebase, bisect, am), the prompt\n>> screams:\n>>\n>> artagnon|completion|REBASE-i 2/2:~/src/git$\n>>\n>> Lowercase the words, so we get a more pleasant\n>>\n>> artagnon|completion|rebase-i 2/2:~/src/git$\n>\n> So I'm not sure whether this falls under bikeshedding or actual\n> features, but I like the screaming.\n\nSounds like bikeshedding.\n\nMy personal opinion is that \"detached in the middle of something\" is\nnot a normal state, and it deserves to be in bold as a reminder.\nLet's leave it as-is.\n \n"},{"id":"219251","messageId":"7vmwr7gl93.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"87mwr7zhv5.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T17:24:56Z","receivedAt":"2013-06-03T17:24:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> Add support for completing 'git blame'. List only the common short\n>> options.\n> [...]\n>> +_git_blame ()\n>> +{\n>> +\t__git_has_doubledash && return\n>> +\n>> +\tcase \"$cur\" in\n>> +\t-*)\n>> +\t\t__gitcomp \"-M -C -L -s -w\"\n>> +\tesac\n>> +\t__git_complete_revlist_file\n>> +}\n>\n> Is this the first time we introduce completion (I guess you could call\n> it \"help\") for short options? I only did a quick search for /-. -/ but\n> it certainly seems that way.\n\nYeah, I do not think this is a good idea.\n"},{"id":"219253","messageId":"7vip1vgl17.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"1370181822-23450-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] completion: clarify difftool completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T17:29:40Z","receivedAt":"2013-06-03T17:29:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> 'git difftool' is clearly a frontend to 'git diff' and is used in\n> exactly the same way, but it uses a misleading completion function name\n> __git_complete_file (aliased to to __git_complete_revlist_file). Change\n> it to use __git_complete_revlist_file, just like 'git diff'. No\n> functional changes.\n\nMakes sense. The completion helper for both revname and pathname\nshould not be called __git_complete_file, and this is a step in the\nright direction.\n\nIf we have a situation where we _know_ we only want to complete\npathname and never revname, we may want to keep __git_complete_file\nfunction. For example, after seeing \"--\" on the command line, we\nmay want to use __git_complete_file (that does not look at revs,\ninstead of __git_complete_revlist_file function.\n\nWill apply.\n\nThanks.\n\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> contrib/completion/git-completion.bash | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index f46964d..8d70c30 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1305,7 +1305,7 @@ _git_difftool ()\n> \t\treturn\n> \t\t;;\n> \tesac\n> -\t__git_complete_file\n> +\t__git_complete_revlist_file\n> }\n> \n> __git_fetch_options=\"\n"},{"id":"219254","messageId":"7vehcjgkuq.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"1370181822-23450-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T17:33:33Z","receivedAt":"2013-06-03T17:33:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Currently, the 'git ls-tree', 'git archive', and 'git show' completions\n> use __git_complete_file (aliased to __git_complete_revlist_file).\n>\n> In the case of 'git ls-tree' and 'git archive', they necessarily require\n> a tree-ish argument (and optionally a pathspec filter, or \"file\n> argument\"):\n>\n> $ git ls-tree hot-branch git.c\n> $ git archive HEAD~4 git.c\n>\n> So, __git_complete_file is a misleading name.\n>\n> In the case of 'git show', it can take a pathspec and default the\n> revision to HEAD like:\n>\n> $ git show git.c\n>\n> (which is useful if git.c was modified in HEAD)\n>\n> However, this usage is not idiomatic at all. The more common usage is\n> like:\n>\n> $ git show HEAD~1\n> $ git show origin/pu:git.c\n>\n> So, __git_complete_file is again a poor name.\n>\n> Replace these three instances of __git_complete_file with\n> __git_complete_revlist_file, without making any functional changes.\n>\n> Remove __git_complete_file, as it has no other callers.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n\nI think this is the same as 5/6 and better explained in a single\npatch, as the rationale is the same: these commands can all take the\nusual revs and then paths, so using misnamed complete_FILE helper is\nwrong.\n\nMind if I squashed them together?\n\n\n> contrib/completion/git-completion.bash | 11 +++--------\n> 1 file changed, 3 insertions(+), 8 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 8d70c30..84d1548 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -592,11 +592,6 @@ __git_complete_diff_index_file ()\n> \tesac\n> }\n> \n> -__git_complete_file ()\n> -{\n> -\t__git_complete_revlist_file\n> -}\n> -\n> __git_complete_revlist ()\n> {\n> \t__git_complete_revlist_file\n> @@ -1007,7 +1002,7 @@ _git_archive ()\n> \t\treturn\n> \t\t;;\n> \tesac\n> -\t__git_complete_file\n> +\t__git_complete_revlist_file\n> }\n> \n> _git_bisect ()\n> @@ -1476,7 +1471,7 @@ _git_ls_remote ()\n> \n> _git_ls_tree ()\n> {\n> -\t__git_complete_file\n> +\t__git_complete_revlist_file\n> }\n> \n> # Options that go well for log, shortlog and gitk\n> @@ -2382,7 +2377,7 @@ _git_show ()\n> \t\treturn\n> \t\t;;\n> \tesac\n> -\t__git_complete_file\n> +\t__git_complete_revlist_file\n> }\n> \n> _git_show_branch ()\n"},{"id":"219261","messageId":"7vobbnf54o.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"1370181822-23450-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T17:58:31Z","receivedAt":"2013-06-03T17:58:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Currently, the 'git ls-tree', 'git archive', and 'git show' completions\n> use __git_complete_file (aliased to __git_complete_revlist_file).\n>\n> In the case of 'git ls-tree' and 'git archive', they necessarily require\n> a tree-ish argument (and optionally a pathspec filter, or \"file\n> argument\"):\n>\n> $ git ls-tree hot-branch git.c\n> $ git archive HEAD~4 git.c\n>\n> So, __git_complete_file is a misleading name.\n>\n> In the case of 'git show', it can take a pathspec and default the\n> revision to HEAD like:\n>\n> $ git show git.c\n>\n> (which is useful if git.c was modified in HEAD)\n>\n> However, this usage is not idiomatic at all.\n\nMore idiomatic is after seeing \"git show --stat\" doing \"git show dir/\".\n\nBut I do not think it really matters.\n\nJust like \"diff\" and \"difftool\", \"show\" optionally takes revs and\npaths, and because the current completion code does not do anything\nfancy like \"we have seen enough revs and there will appear no revs\nhence we only complete paths from here on\", complete_revlist_file is\nthe right helper to use for all of them.\n"},{"id":"219266","messageId":"20130603180701.GJ2192@goldbirke","threadId":"34010","inReplyTo":"CALkWK0kR2fCQcO9XurzjOGF19vaC67p5kMpkJL=Vbu1Ovku18w@mail.gmail.com","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-03T18:07:01Z","receivedAt":"2013-06-03T18:07:01Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jun 03, 2013 at 03:02:00PM +0530, Ramkumar Ramachandra wrote:\n> Thomas Rast wrote:\n> > Is this the first time we introduce completion (I guess you could call\n> > it \"help\") for short options? I only did a quick search for /-. -/ but\n> > it certainly seems that way.\n> \n> Yeah. We generally prefer the long-form equivalents while doing\n> completions, but these blame options do not have equivalent\n> long-forms.\n\nBut providing short options for completion is pointless. Those who\nknow git blame's short options will just type them right away, because\nit requires less key presses than using completion, and those who\ndon't know these options will only see -C, -M, etc. and won't have any\nclues what it stands for.\n\n\nBest,\nGábor\n"},{"id":"219269","messageId":"7vzjv7dowz.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"20130603180701.GJ2192@goldbirke","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T18:34:04Z","receivedAt":"2013-06-03T18:34:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> But providing short options for completion is pointless. Those who\n> know git blame's short options will just type them right away, because\n> it requires less key presses than using completion, and those who\n> don't know these options will only see -C, -M, etc. and won't have any\n> clues what it stands for.\n\n\nIt is not necessarily true that \"they won't have any clue\"; one\ncould argue that \"-C -M etc\" can serve as reminder to people who\nonce learned but forgot momentarily.\n\nMy conclusion is the same, though; \"git blame -h\" is for those\npeople.\n"},{"id":"219281","messageId":"20130603192555.GK2192@goldbirke","threadId":"34010","inReplyTo":"1370181822-23450-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-03T19:25:55Z","receivedAt":"2013-06-03T19:25:55Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Jun 02, 2013 at 07:33:42PM +0530, Ramkumar Ramachandra wrote:\n> Currently, the 'git ls-tree', 'git archive', and 'git show' completions\n> use __git_complete_file (aliased to __git_complete_revlist_file).\n> \n> In the case of 'git ls-tree' and 'git archive', they necessarily require\n> a tree-ish argument (and optionally a pathspec filter, or \"file\n> argument\"):\n> \n> $ git ls-tree hot-branch git.c\n> $ git archive HEAD~4 git.c\n> \n> So, __git_complete_file is a misleading name.\n> \n> In the case of 'git show', it can take a pathspec and default the\n> revision to HEAD like:\n> \n> $ git show git.c\n> \n> (which is useful if git.c was modified in HEAD)\n> \n> However, this usage is not idiomatic at all. The more common usage is\n> like:\n> \n> $ git show HEAD~1\n> $ git show origin/pu:git.c\n> \n> So, __git_complete_file is again a poor name.\n> \n> Replace these three instances of __git_complete_file with\n> __git_complete_revlist_file, without making any functional changes.\n> \n> Remove __git_complete_file, as it has no other callers.\n> \n\nWell, people out there might have completion scriplets for their\naliases or custom git commands which use __git_complete_file().\nRemoving this function would break those scripts.\n\nArguably the name of __git_complete_file() could describe better what\nthe function does, or what it did, i.e. it used to provide completion\nfor the master:Doc notation. But that's only the name. Since\nboth git ls-tree and git archive understand this notation, calling the\nhelper for master:Doc in their completion functions is not\nmisleading at all.\n\nNow, __git_complete_revlist_file() provides completion both for this\nmaster:Doc notation and for revision ranges, i.e. for\nmaster..n and master...n. However, since neither git\nls-tree nor git archive accept revision ranges, calling\n__git_complete_revlist_file() in their completion function would be\nmisleading.\n\ngit show is special, as it understands both the master:Doc\nnotation and revision ranges, and even the combination of the two, so\ncalling __git_complete_revlist_file() there would indeed be better.\n\n\nGábor\n"},{"id":"219302","messageId":"20130603211556.GB23224@sigill.intra.peff.net","threadId":"34010","inReplyTo":"CALkWK0=LzUe0k7rWcrbiiTZNOOGg+=SsQB3u2F2ysjb5AJ=qZw@mail.gmail.com","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-03T21:15:56Z","receivedAt":"2013-06-03T21:15:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 03, 2013 at 03:17:27PM +0530, Ramkumar Ramachandra wrote:\n\n> Thomas Rast wrote:\n> > Do you have other ways of distinguishing the branch and the state?\n> > Colors? I'm a bit too lazy to check. Perhaps it could be made to only\n> > use caps if not in colored mode?\n> \n> Currently, no. See git-prompt.sh:401, 403, 409; we don't have a\n> separate color for $r. I didn't introduce a color in this series\n> because it would conflict with rr/zsh-color-prompt which is in pu: we\n> can introduce it after that graduates. I was thinking yellow, since\n> that's not taken?\n> \n> You really should use colors. I don't think it's worth the extra\n> ugliness to scream in the no-color case.\n> \n> As such, the prompt is a fine bikeshedding target. I even introduced\n> GIT_PS1_SEPARATOR, because some people were unhappy with me stripping\n> one whitespace (yes, one). If we go down the configurability road,\n> we'll either end up with a ton of environment variables, or be made to\n> write a generalized custom formatter which splices together tons of\n> arguments using color (super painful). While I do agree that it is a\n> matter of taste, we have to make a few compromises for the sake of\n> sanity.\n\nIt seems silly to argue about output formats when we are writing a\nprompt in a convenient Turing-complete scripting language already.\nWhat about something like:\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex eaf5c36..1da3833 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -404,6 +404,8 @@ __git_ps1 ()\n \t\t\tfi\n \t\t\tgitstring=$(printf -- \"$printf_format\" \"$gitstring\")\n \t\t\tPS1=\"$ps1pc_start$gitstring$ps1pc_end\"\n+\t\telif test \"$(type -t __git_ps1_printer)\" = \"function\"; then\n+\t\t\t__git_ps1_printer\n \t\telse\n \t\t\t# NO color option unless in PROMPT_COMMAND mode\n \t\t\tprintf -- \"$printf_format\" \"$c${b##refs/heads/}${f:+ $f}$r$p\"\n\nand then you can define your own custom function if you like, which will\ninherit all of the variables from __git_ps1. E.g.:\n\n __git_ps1_printer() {\n echo \"${r,,}\"\n }\n\nyields a lower-case \"rebase-i\". Of course there are many other variables\nyou want to put in your prompt, too. The only problem I see is that the\ncurrent set of variables is not suitable as a user-visible interface.\nThe names are not good (the branch should be \"$branch\" rather than \"$b\",\nthe operation-in-progress should be \"$operation\" or similar rather than\n\"$r\", etc), and the formats are not as convenient (\"$r\" contains the\nfull \"|REBASE-i 6/10\", whereas it should be broken down so the printing\nfunction can easily use a case statement).\n\nAnd of course the variables would need documentation.\n\n-Peff\n"},{"id":"219317","messageId":"CALkWK0=5XecaMpPPQgvJZTwR2QUUWVqbZK5p7af9R1HjA0U=OQ@mail.gmail.com","threadId":"34010","inReplyTo":"20130603211556.GB23224@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-04T03:44:23Z","receivedAt":"2013-06-04T03:44:23Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jeff King wrote:\n> It seems silly to argue about output formats when we are writing a\n> prompt in a convenient Turing-complete scripting language already.\n> What about something like:\n\nCould you have a look at __git_ps1_colorize_gitstring from\nrr/zsh-color-prompt in pu? In the general case, wouldn't the user\nneed to re-implement this entire function (with so many variables) in\nher ~/.zshrc? Isn't it horribly painful for too little gain?\n"},{"id":"219318","messageId":"CALkWK0nMWPvVBXEyM5TwCC+0LGvLGbUVXPBhewY8X4==nHELSQ@mail.gmail.com","threadId":"34010","inReplyTo":"7vehcjgkuq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-04T03:49:50Z","receivedAt":"2013-06-04T03:49:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I think this is the same as 5/6 and better explained in a single\n> patch, as the rationale is the same: these commands can all take the\n> usual revs and then paths, so using misnamed complete_FILE helper is\n> wrong.\n>\n> Mind if I squashed them together?\n\nI'm okay with what you've queued in pu (sans some points raised by\nSZEDER; looking into that), but you can squash them together if you\nlike.\n"},{"id":"219319","messageId":"20130604043836.GA3931@sigill.intra.peff.net","threadId":"34010","inReplyTo":"CALkWK0=5XecaMpPPQgvJZTwR2QUUWVqbZK5p7af9R1HjA0U=OQ@mail.gmail.com","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-04T04:38:37Z","receivedAt":"2013-06-04T04:38:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 04, 2013 at 09:14:23AM +0530, Ramkumar Ramachandra wrote:\n\n> Jeff King wrote:\n> > It seems silly to argue about output formats when we are writing a\n> > prompt in a convenient Turing-complete scripting language already.\n> > What about something like:\n> \n> Could you have a look at __git_ps1_colorize_gitstring from\n> rr/zsh-color-prompt in pu? In the general case, wouldn't the user\n> need to re-implement this entire function (with so many variables) in\n> her ~/.zshrc? Isn't it horribly painful for too little gain?\n\nIt is a lot of ugly code, but then, complexity and flexibility are a\ntradeoff. You would not _have_ to use a custom function at all. But if\nwe make it an option, and leave the existing code as the default\nfunction, then only those who want it have to use it.\n\nEven better, we can hit a middle ground by abstracting away some of the\ncomplexity. You could probably write colorize() function to handle\ncolorizing a particular string (and handle bash/zsh magic), and then\npull the whole \"which color is the branch\" function into a\ncolorize_branch(), and so forth.\n\nI haven't looked that closely, so there may be hidden troubles awaiting,\nbut I don't see why you couldn't ultimately let the user write something\nlike:\n\n __git_ps1_printer() {\n echo ' ('\n test \"$bare\" = t && echo \"BARE:\"; # or \"bare:\" :)\n color_branch \"$(short_ref $branch)\" \"$detached\"\n test \"$wt_changes\" = t && colorize red \"*\"\n ...\n echo ')'\n }\n\nAnd if that is too much, you can abstract bits of it out. The point is\nthat if we can expose the lowest-level details, we can compose functions\naround them at whatever level is appropriate, and callers can mix and\nmatch for the balance of complexity and flexibility they want.\n\nPerhaps it is overkill, but I also think it may make the code itself\nmore readable. For example, I put a \"...\" above because the next line\nwould handle this:\n\n if [ -n \"$i\" ]; then\n gitstring=\"$gitstring\\[$ok_color\\]$i\"\n fi\n\nI have no clue what that does, or what the global variable \"$i\"\ncontains. Reading through __git_ps1, it looks like it will be \"+\" for\nindex changes. Being able to write:\n\n test \"$index_changes\" = t && colorize green \"+\"\n\nseems much more readable to me. And we could probably compose a:\n\n colorize_diff_state() {\n test \"$wt_changes\" = t && colorize red \"*\"\n test \"$index_changes\" = t && colorize green \"+\"\n }\n\nfor people who just want to take the default colors and characters.\n\n-Peff\n"},{"id":"219323","messageId":"7v8v2qa02b.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"20130604043836.GA3931@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] prompt: don't scream continuation state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-04T05:59:08Z","receivedAt":"2013-06-04T05:59:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Even better, we can hit a middle ground by abstracting away some of the\n> complexity.\n\nThe latter half of __git_ps1 is already fairly nice; w/i/s/u/c/p and\nfriends can serve as the basis of such an abstraction, even though r\ndoes want to be separated further.\n\nI tried the GIT_PS1_SHOW* variables several times, but every time I\ndid so, I eventually had to give up because I simply couldn't read\nand/or remember what */+#/$/% line noises meant. If we had a custom\n\"collect these pieces and form the final presentation\" mechanism, I\ncould easily replace these with mnemonic letters that can be more\neasily memorable.\n"},{"id":"219324","messageId":"7v4nde9zxl.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"CALkWK0nMWPvVBXEyM5TwCC+0LGvLGbUVXPBhewY8X4==nHELSQ@mail.gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-04T06:01:58Z","receivedAt":"2013-06-04T06:01:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> I think this is the same as 5/6 and better explained in a single\n>> patch, as the rationale is the same: these commands can all take the\n>> usual revs and then paths, so using misnamed complete_FILE helper is\n>> wrong.\n>>\n>> Mind if I squashed them together?\n>\n> I'm okay with what you've queued in pu (sans some points raised by\n> SZEDER; looking into that), but you can squash them together if you\n> like.\n\nIn any case, I think it is sensible not to remove the _file\nhelper. We may want to fix it not to expand revname and do only the\npathname, but that is a separate issue from using _revlist_file for\nthe commands you updated completion for in these two patches.\n"},{"id":"219501","messageId":"alpine.DEB.2.00.1306061057010.14650@ds9.cixit.se","threadId":"34010","inReplyTo":"CALkWK0kR2fCQcO9XurzjOGF19vaC67p5kMpkJL=Vbu1Ovku18w@mail.gmail.com","subject":"Re: [PATCH 3/6] completion: add common options for blame","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2013-06-06T09:58:58Z","receivedAt":"2013-06-06T09:58:58Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Ramkumar Ramachandra:\n\n> Yeah. We generally prefer the long-form equivalents while doing \n> completions, but these blame options do not have equivalent \n> long-forms.\n\nPerhaps that is the real bug, then. -M and -C already have long names \nfor diff (and its friends), perhaps blame should have the same long \noption names for them?\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"219640","messageId":"CALkWK0khxsp7pLb_ojHqYhT1SQnAAX6DLi7eSBTn_CQ1rXAJkA@mail.gmail.com","threadId":"34010","inReplyTo":"1370181822-23450-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/6] completion: add common options for rev-parse","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-07T15:33:28Z","receivedAt":"2013-06-07T15:33:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Add support for completing 'git rev-parse'. List only the options that\n> are likely to be used on the command-line, as opposed to scripts.\n\nCan we get this patch? I use git rev-list quite a lot, and want completion.\n"},{"id":"219652","messageId":"CALkWK0mbVgJHRoNXNkc6n7==-H+caNqvzrzUwspS_Eq2sMfJbg@mail.gmail.com","threadId":"34010","inReplyTo":"20130603192555.GK2192@goldbirke","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-07T17:21:53Z","receivedAt":"2013-06-07T17:21:53Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"SZEDER Gábor wrote:\n> Well, people out there might have completion scriplets for their\n> aliases or custom git commands which use __git_complete_file().\n> Removing this function would break those scripts.\n\nWhat is the advantage of using __git_complete_file() over\n__git_complete_revlist_file()? Isn't it just a misleading alias?\n\n> Arguably the name of __git_complete_file() could describe better what\n> the function does, or what it did, i.e. it used to provide completion\n> for the master:Doc notation. But that's only the name. Since\n> both git ls-tree and git archive understand this notation, calling the\n> helper for master:Doc in their completion functions is not\n> misleading at all.\n\nBut __git_complete_revlist_file() provides all this and more, no?\n\n> Now, __git_complete_revlist_file() provides completion both for this\n> master:Doc notation and for revision ranges, i.e. for\n> master..n and master...n. However, since neither git\n> ls-tree nor git archive accept revision ranges, calling\n> __git_complete_revlist_file() in their completion function would be\n> misleading.\n\nYeah, they accept tree-ish'es. Isn't __git_complete_file() still a\nhorrible name?\n\n $ git ls-tree HEAD:Documentation\n\nWhat file? There's already a __git_complete_index_file(), which is\nproperly named and used by the ls-files family. If anything, we\nshould write a new __git_complete_treeish() function that does what\n__git_complete_revlist_file() does, except that it doesn't complete\nrevision ranges, right? Frankly, I don't know if it's worth the\nadditional trouble: we do spurious completions all over the place, and\nwe haven't clamped down on any of that.\n\n $ git log HEAD:Doc\n\nNote how log doesn't even error out.\n\n> git show is special, as it understands both the master:Doc\n> notation and revision ranges, and even the combination of the two, so\n> calling __git_complete_revlist_file() there would indeed be better.\n\nIt just accepts any revspec with pathspec filtering, like many many\nother commands.\n"},{"id":"219664","messageId":"7vd2rxu5ru.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"CALkWK0mbVgJHRoNXNkc6n7==-H+caNqvzrzUwspS_Eq2sMfJbg@mail.gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-07T18:36:53Z","receivedAt":"2013-06-07T18:36:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> SZEDER Gábor wrote:\n>> Well, people out there might have completion scriplets for their\n>> aliases or custom git commands which use __git_complete_file().\n>> Removing this function would break those scripts.\n>\n> What is the advantage of using __git_complete_file() over\n> __git_complete_revlist_file()? Isn't it just a misleading alias?\n\nNone.\n\nBut \"Those who have been using it can update their script\" is not an\nanswer you give when dealing with a regression.\n\nI think the right thing is to add __git_complete_file() back in;\nperhaps strip support of A..B/A...B range notations from it, but\nthat is an independent change.\n"},{"id":"219666","messageId":"20130607184527.GB25731@goldbirke","threadId":"34010","inReplyTo":"CALkWK0mbVgJHRoNXNkc6n7==-H+caNqvzrzUwspS_Eq2sMfJbg@mail.gmail.com","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-07T18:45:27Z","receivedAt":"2013-06-07T18:45:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Jun 07, 2013 at 10:51:53PM +0530, Ramkumar Ramachandra wrote:\n> SZEDER Gábor wrote:\n> > Well, people out there might have completion scriplets for their\n> > aliases or custom git commands which use __git_complete_file().\n> > Removing this function would break those scripts.\n> \n> What is the advantage of using __git_complete_file() over\n> __git_complete_revlist_file()?\n\nThat it doesn't imply that the command takes refs in the form of\nref1..ref2.\n\n> Isn't it just a misleading alias?\n\nNo. It's an implementation detail that __git_complete_file() became\nan alias to __git_complete_revlist_file() to avoid unnecessary code\nduplication.\n\nAnd it was a concious decision to keep __git_complete_file() (and\n__git_complete_revlist()) around in order not to break completion\nscriplets for users' alieses and custom git commands which might call\nit.\n\n> > Arguably the name of __git_complete_file() could describe better what\n> > the function does, or what it did, i.e. it used to provide completion\n> > for the master:Doc notation. But that's only the name. Since\n> > both git ls-tree and git archive understand this notation, calling the\n> > helper for master:Doc in their completion functions is not\n> > misleading at all.\n> \n> But __git_complete_revlist_file() provides all this and more, no?\n\nIndeed, and this \"more\" is exactly why it is misleading to call\n__git_complete_revlist_file() directly for git ls-tree and git\narchive.\n\n> > Now, __git_complete_revlist_file() provides completion both for this\n> > master:Doc notation and for revision ranges, i.e. for\n> > master..n and master...n. However, since neither git\n> > ls-tree nor git archive accept revision ranges, calling\n> > __git_complete_revlist_file() in their completion function would be\n> > misleading.\n> \n> Yeah, they accept tree-ish'es. Isn't __git_complete_file() still a\n> horrible name?\n\nWe can't go back in time to correct it, unfortunately.\n\n> If anything, we\n> should write a new __git_complete_treeish() function that does what\n> __git_complete_revlist_file() does, except that it doesn't complete\n> revision ranges, right? Frankly, I don't know if it's worth the\n> additional trouble\n\nI agree that it isn't worth it, and that is exactly why\n__git_complete_revlist() and __git_complete_file() were unified in\n__git_complete_revlist_file().\n\n> $ git log HEAD:Doc\n> \n> Note how log doesn't even error out.\n\nBut note how git log master..HEAD:Documentation/ errors out.\n\n> > git show is special, as it understands both the master:Doc\n> > notation and revision ranges, and even the combination of the two, so\n> > calling __git_complete_revlist_file() there would indeed be better.\n> \n> It just accepts any revspec with pathspec filtering, like many many\n> other commands.\n\nWhich many many other commands do accept ref1..ref2:file?\n\n\nGábor\n"},{"id":"220166","messageId":"7vbo7fovez.fsf@alter.siamese.dyndns.org","threadId":"34010","inReplyTo":"20130603192555.GK2192@goldbirke","subject":"Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T20:56:20Z","receivedAt":"2013-06-09T20:56:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> Now, __git_complete_revlist_file() provides completion both for this\n> master:Doc notation and for revision ranges, i.e. for\n> master..n and master...n. However, since neither git\n> ls-tree nor git archive accept revision ranges, calling\n> __git_complete_revlist_file() in their completion function would be\n> misleading.\n\nohh, I missed this part, and you are right.\n\n> git show is special, as it understands both the master:Doc\n> notation and revision ranges, and even the combination of the two, so\n> calling __git_complete_revlist_file() there would indeed be better.\n"}]}