Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Junio C Hamano wrote: > But it turns out that in the context of these functions it refers to > "what users consider paths in objects stored in the object database" > (as opposed to working tree paths). That is what ls-tree would take > (i.e. and :). And I do not offhand think > of a better name myself to strongly oppose to using the word FILE to > refer to what it does. __git_complete_treeish(). Write a new patch with a proper comment saying why it is aliased to __git_complete_revlist_file(). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Ramkumar Ramachandra writes: > Junio C Hamano wrote: >> Revert 77c1305 and 3c3b46b > > The core of the argument seems to be that > __git_complete_revlist_file() is a misleading name for the completion > done by archive and ls-tree, but __git_complete_file() is somehow a > less misleading name? Irrespective of what the valid completions of > the various commands are, I want to know which completion function > will be _used_ when I'm reading the script. And that is > __git_complete_revlist_file(). > > To me, it looks like mega-bikeshedding; a huge amount of time and > effort going behind a non-functional variable rename (and the best > part? the rename isn't getting us a "better" name; just a "historical" > name). But whatever. To me the most important part is that we have two separate functions that are used consistently by how the completion is to be done for their users. The complete-file variant can then lose the A..B range completion, and then be given a better name than FILE to express what it does if somebody can find one. When it happens, the same better name should be used to rename complete-revlist-FILE by replacing the "FILE" part. I initially thought that FILE referred to the pathspecs (i.e. the last part in "log "), and felt it was strange to call it FILE. Perhaps that (i.e. it not being pathspec) is what you find it misnamed). But it turns out that in the context of these functions it refers to "what users consider paths in objects stored in the object database" (as opposed to working tree paths). That is what ls-tree would take (i.e. and :). And I do not offhand think of a better name myself to strongly oppose to using the word FILE to refer to what it does. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Junio C Hamano wrote: > Revert 77c1305 and 3c3b46b The core of the argument seems to be that __git_complete_revlist_file() is a misleading name for the completion done by archive and ls-tree, but __git_complete_file() is somehow a less misleading name? Irrespective of what the valid completions of the various commands are, I want to know which completion function will be _used_ when I'm reading the script. And that is __git_complete_revlist_file(). To me, it looks like mega-bikeshedding; a huge amount of time and effort going behind a non-functional variable rename (and the best part? the rename isn't getting us a "better" name; just a "historical" name). But whatever. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: >> There still remain two users of __git_complete_file, completions for >> "archive" and "ls-tree". As these commands do not take range >> notation, and "git show" no longer uses __git_complete_file, the >> implementation of can be updated not to complete ranges, but that is > > "the implementation of" what? A word missing perhaps? it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Hi, On Sun, Jun 09, 2013 at 02:20:33PM -0700, Junio C Hamano wrote: > Regarding that rr/complete-difftool topic, let's revert the tip 2 > commits (the "ls-tree, archive and show" one, and the follow-up > resurrection of __git_complete_file) with this: > > Revert 77c1305 and 3c3b46b > > As explained by SZEDER Gábor in $gmane/226272, git_complete_file > helper is about completing , taking at the tips > of refs and also understanding : notation, and > changing "archive" and "ls-tree" to use git_complete_revlist_file > that in addition is meant to expand A..B range notation was a > mistake. > > Signed-off-by: Junio C Hamano > > and then queue this on top of d8517cc6670d (completion: difftool > takes both revs and files, 2013-06-02), so that the attached and > d8517cc6670d will be the only two commits that graduate to 'master' > from this topic. That's fine with me, except ... > -- >8 -- > From: Ramkumar Ramachandra > Date: Sun, 2 Jun 2013 19:33:42 +0530 > Subject: [PATCH] completion: show can take both revlist and paths > > The 'git show' completion uses __git_complete_file (aliased to > __git_complete_revlist_file), because accepts : as > well as . But the command also accepts range of commits > in A..B notation, so using __git_complete_revlist_file is more > appropriate. > > There still remain two users of __git_complete_file, completions for > "archive" and "ls-tree". As these commands do not take range > notation, and "git show" no longer uses __git_complete_file, the > implementation of can be updated not to complete ranges, but that is "the implementation of" what? A word missing perhaps? Or "the implementation can be updated"? Best, Gábor > a separate topic. > > Signed-off-by: Ramkumar Ramachandra > Signed-off-by: Junio C Hamano > --- > contrib/completion/git-completion.bash | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index 1b4b0f9..b9dfc3b 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2360,7 +2360,7 @@ _git_show () > return > ;; > esac > - __git_complete_file > + __git_complete_revlist_file > } > > _git_show_branch () > -- > 1.8.3-477-gc2fede3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: >> It is somewhat annoying that "git diff gi" stops at expanding >> it to "git diff git" and then upon another "git diff git" >> offers tags whose names begin with "git" (e.g. gitgui-0.10.0) but >> the pathname git.c is not included in the choices. My wish was to >> take the union in such a fairly limited case. I tend to agree with >> you that "git diff " that expands to all refs and pathnames >> would be useless, but so is expansion to all pathnames (or refnames >> for that matter). > > ... or trying to complete a branch name starting with a 'v', and then > getting all the vx.y.z tags. > > If you know you want git.c, then you can force filename completion > either by entering "--" before hitting tab... Yes, that is exactly why I said "the current completion code already works better than reasonably well, at least for me" in the concluding part of my message. Regarding that rr/complete-difftool topic, let's revert the tip 2 commits (the "ls-tree, archive and show" one, and the follow-up resurrection of __git_complete_file) with this: Revert 77c1305 and 3c3b46b As explained by SZEDER Gábor in $gmane/226272, git_complete_file helper is about completing , taking at the tips of refs and also understanding : notation, and changing "archive" and "ls-tree" to use git_complete_revlist_file that in addition is meant to expand A..B range notation was a mistake. Signed-off-by: Junio C Hamano and then queue this on top of d8517cc6670d (completion: difftool takes both revs and files, 2013-06-02), so that the attached and d8517cc6670d will be the only two commits that graduate to 'master' from this topic. -- >8 -- From: Ramkumar Ramachandra Date: Sun, 2 Jun 2013 19:33:42 +0530 Subject: [PATCH] completion: show can take both revlist and paths The 'git show' completion uses __git_complete_file (aliased to __git_complete_revlist_file), because accepts : as well as . But the command also accepts range of commits in A..B notation, so using __git_complete_revlist_file is more appropriate. There still remain two users of __git_complete_file, completions for "archive" and "ls-tree". As these commands do not take range notation, and "git show" no longer uses __git_complete_file, the implementation of can be updated not to complete ranges, but that is a separate topic. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1b4b0f9..b9dfc3b 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2360,7 +2360,7 @@ _git_show () return ;; esac - __git_complete_file + __git_complete_revlist_file } _git_show_branch () -- 1.8.3-477-gc2fede3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Ramkumar Ramachandra writes: > Yes, please merge. The commit message looks good as well: it doesn't > say anything about waiting for 2.0. The commit message doesn't, but the doc does. I'll resend without the 2.0 mention. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Junio C Hamano wrote: > * mm/color-auto-default (2013-05-15) 2 commits > - make color.ui default to 'auto' > - config: refactor management of color.ui's default value > > Flip the default for color.ui to 'auto', which is what many > tutorials recommend new users to do. The updated code claims the > switch happened at Git 2.0 in the past tense, but we might want to > expedite it, as this change is not all that important to deserve a > major version bump. > > I'd vote for merging this without waiting for 2.0. Comments? Yes, please merge. The commit message looks good as well: it doesn't say anything about waiting for 2.0. > Waiting for a reroll. The one in pu looks fine to me. What am I missing? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 02:53:02PM -0700, Junio C Hamano wrote: > SZEDER Gábor writes: > > > On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: > >> Thanks for a pointer. I think what I was suggesting was slightly > >> different in that I was hoping to see a single helper that knows to > >> complete to object names (possibly including trees/blobs with the > >> treeish:path notation), ranges, and pathnames (not treeish:path > >> notation) until it sees a "--" and then complete only to pathnames. > > > > We already got that except the completion of pathnames before "--", > > and I don't know how that could be done properly for commands taking > > both refs and paths. > > ... > > git diff git.c > > git diff master git.c > > git diff master next git.c > > It is somewhat annoying that "git diff gi" stops at expanding > it to "git diff git" and then upon another "git diff git" > offers tags whose names begin with "git" (e.g. gitgui-0.10.0) but > the pathname git.c is not included in the choices. My wish was to > take the union in such a fairly limited case. I tend to agree with > you that "git diff " that expands to all refs and pathnames > would be useless, but so is expansion to all pathnames (or refnames > for that matter). ... or trying to complete a branch name starting with a 'v', and then getting all the vx.y.z tags. If you know you want git.c, then you can force filename completion either by entering "--" before hitting tab or by using the Alt-/ Bash (readline?) keybinding, otherwise you'll get refs. I think this is more than adequate, as it brings the best of both worlds: you can quickly and easily get both ref-only and file-only completion. Training your fingers to type "--" is perhaps better, just in case we'll ever do tracked-file-aware filename completion for e.g. 'git log -- g' in the future. Best, Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: > On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: >> Thanks for a pointer. I think what I was suggesting was slightly >> different in that I was hoping to see a single helper that knows to >> complete to object names (possibly including trees/blobs with the >> treeish:path notation), ranges, and pathnames (not treeish:path >> notation) until it sees a "--" and then complete only to pathnames. > > We already got that except the completion of pathnames before "--", > and I don't know how that could be done properly for commands taking > both refs and paths. > ... > git diff git.c > git diff master git.c > git diff master next git.c It is somewhat annoying that "git diff gi" stops at expanding it to "git diff git" and then upon another "git diff git" offers tags whose names begin with "git" (e.g. gitgui-0.10.0) but the pathname git.c is not included in the choices. My wish was to take the union in such a fairly limited case. I tend to agree with you that "git diff " that expands to all refs and pathnames would be useless, but so is expansion to all pathnames (or refnames for that matter). In any case, I wouldn't insist on AI perfection in the first place ;-). As long as it works reasonably well, I am happy (and I think the current completion code already works better than reasonably well, at least for me). Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: > Thanks for a pointer. I think what I was suggesting was slightly > different in that I was hoping to see a single helper that knows to > complete to object names (possibly including trees/blobs with the > treeish:path notation), ranges, and pathnames (not treeish:path > notation) until it sees a "--" and then complete only to pathnames. We already got that except the completion of pathnames before "--", and I don't know how that could be done properly for commands taking both refs and paths. Just consider git diff git.c git diff master git.c git diff master next git.c We can't guess whether the user wants refs or paths when he first hits tab after 'git diff ', not even after 'git diff master '. I definitely don't want to see refs and paths all mixed up. As for the _single_ helper: I think it has some value that we have different helper functions and we can indicate whether a certain git command can take a ref or ref:path or ref1...ref2 or even ref1..ref2:path by calling the appropriate helper function (however badly it might have been named), even if all these functions happen to boil down to a single implementation. Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Sat, Jun 08, 2013 at 01:17:28AM +0530, Ramkumar Ramachandra wrote: > SZEDER Gábor wrote: > > because nowadays __git_complete_file() is a wrapper around > > __git_complete_revlist_file(). > > What? It was never anything different from a poorly-named alias for > __git_complete_revlist_file(). Again: __git_complete_file() has been there since the completion script was first included in git. > You have already agreed that > __git_complete_file() is a horrible name, so why not deprecate it? I am not against deprecating it, and by "it" I mean the function name. > Whom is this confusion benefiting, and how is it any "cleaner"? It's clearer because of the reasons I gave in my other email in the discussion of the patch. Plus it would avoid commits on master with incorrect commit messages. > If > you're arguing about expanding __git_complete_file(), don't do that: > just write a new function and give it a proper name. I am not arguing about that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor wrote: > because nowadays __git_complete_file() is a wrapper around > __git_complete_revlist_file(). What? It was never anything different from a poorly-named alias for __git_complete_revlist_file(). You have already agreed that __git_complete_file() is a horrible name, so why not deprecate it? Whom is this confusion benefiting, and how is it any "cleaner"? If you're arguing about expanding __git_complete_file(), don't do that: just write a new function and give it a proper name. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: >> Now I do not recall suggesting it, and you (and I today after 2 >> years) may disagree with the rationale, but at least we can read >> what was the "intended" meaning, I think. > > See > > http://thread.gmane.org/gmane.comp.version-control.git/167728/focus=168838 > > I still agree with the rationale,... Thanks for a pointer. I think what I was suggesting was slightly different in that I was hoping to see a single helper that knows to complete to object names (possibly including trees/blobs with the treeish:path notation), ranges, and pathnames (not treeish:path notation) until it sees a "--" and then complete only to pathnames. It can be improved by teaching the unified one that some command like "log" can never take treeish:path but only committishes, some command take individual object names but never ranges, and/or details like that. But I still agree that "git log HEAD:dir" that completes to a blob or a tree object name is not an issue (because what was before cannot possibly name anything "git log" would appreciate), even though it might not be ideal. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Thu, Jun 06, 2013 at 06:05:44PM -0700, Junio C Hamano wrote: > SZEDER Gábor writes: > > > On Thu, Jun 06, 2013 at 03:41:08PM -0700, Junio C Hamano wrote: > >> * rr/complete-difftool (2013-06-03) 2 commits > >> (merged to 'next' on 2013-06-04 at 01c7611) > >> + completion: clarify ls-tree, archive, show completion > >> + completion: difftool takes both revs and files > >> > >> Update command line completion (in contrib/) to use a better named > >> completion helper function for commands that take revisions and > >> paths. > >> > >> Will merge to 'master'. > > > > This should not be merged to master as is; the one at the top because > > of the reasons given in $gmane/226272, the one at the bottom because > > of the misleading commit message (__git_complete_file() always > > completed refs first as part of the ref:file notation, so it worked > > just fine except for the ref1...ref2 notation; the real reason for > > calling __git_complete_revlist_file() for difftool is to make clear > > that difftool takes ref1...ref2:file, too). > > Oops. > > It is too late to amend the log messages now, but at least a follow-up > patch can fix the breakage by adding __git_complete_file() back. Would > you mind doing that? Is it in master already? Am I missing something? Wouldn't it be cleaner to revert those two patches from next and apply this instead? -- >8 -- From: SZEDER Gábor Subject: [PATCH] completion: be explicit about revlist completion for difftool and show The completion functions for 'git difftool' and 'git show' call __git_complete_file() to support completion of the 'ref:path' notation. However, these two commands also understand the 'ref1..ref2:path' notation, the completion of which we happen to support accidentaly, because nowadays __git_complete_file() is a wrapper around __git_complete_revlist_file(). Let's be explicit about it and call __git_complete_revlist_file() directly. Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 56c52c66..fd9a1d5f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1211,7 +1211,7 @@ _git_difftool () return ;; esac - __git_complete_file + __git_complete_revlist_file } __git_fetch_options=" @@ -2277,7 +2277,7 @@ _git_show () return ;; esac - __git_complete_file + __git_complete_revlist_file } _git_show_branch () -- 1.8.0.220.g4d14ece -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 11:41:07AM -0700, Junio C Hamano wrote: > "git log -Gcomplete_file -p contrib/completion" found this one: > > commit 1d66ec587e7d903afdf12a81718772a9eadc15a1 > Author: SZEDER Gábor > Date: Thu Mar 10 19:12:29 2011 +0100 > > bash: complete 'git diff ...branc' (snip) > Suggested-by: Junio C Hamano > Signed-off-by: SZEDER Gábor > Signed-off-by: Junio C Hamano > > Now I do not recall suggesting it, and you (and I today after 2 > years) may disagree with the rationale, but at least we can read > what was the "intended" meaning, I think. See http://thread.gmane.org/gmane.comp.version-control.git/167728/focus=168838 I still agree with the rationale, considering that the new __git_complete_revlist_file() function for completing ref1...ref2:path is a nearly straight copy-paste from the then __git_complete_revlist() and __git_complete_file() including that 12 line long sed script. Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Junio C Hamano wrote: > "git log -Gcomplete_file -p contrib/completion" found this one: > > Now I do not recall suggesting it, and you (and I today after 2 > years) may disagree with the rationale, but at least we can read > what was the "intended" meaning, I think. Having spent so much time documenting pickaxe, I should atleast learn to use it more often :\ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 11:00:14PM +0530, Ramkumar Ramachandra wrote: > SZEDER Gábor wrote: > > the one at the top because > > of the reasons given in $gmane/226272 > > Sorry about the delay: I went to sleep for a couple of days :P > > > the one at the bottom because > > of the misleading commit message (__git_complete_file() always > > completed refs first as part of the ref:file notation, so it worked > > just fine except for the ref1...ref2 notation; the real reason for > > calling __git_complete_revlist_file() for difftool is to make clear > > that difftool takes ref1...ref2:file, too). > > How am I (or anyone else) supposed to know the "intended" meaning > __git_complete_file()? The implementation is just an alias to > __git_complete_revlist_file(), so I looked at the name and guessed > that it was supposed to complete files; now you tell me that it was > intended to complete any revspec except revision ranges (what does > that have to do with "file" again?). I suppose digging through the > history would've told me, but I really didn't bother for such a > trivial non-functional change. Yeah, I suppose it's always wise to do a bit of history digging before you go on to remove a function you don't know what it is doing, even though a simple git log -Sfuncname perhaps doesn't even qualifies for "digging" ;) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Ramkumar Ramachandra writes:
> SZEDER Gábor wrote:
>> the one at the top because
>> of the reasons given in $gmane/226272
>
> Sorry about the delay: I went to sleep for a couple of days :P
>
>> the one at the bottom because
>> of the misleading commit message (__git_complete_file() always
>> completed refs first as part of the ref:file notation, so it worked
>> just fine except for the ref1...ref2 notation; the real reason for
>> calling __git_complete_revlist_file() for difftool is to make clear
>> that difftool takes ref1...ref2:file, too).
>
> How am I (or anyone else) supposed to know the "intended" meaning
> __git_complete_file()?
"git log -Gcomplete_file -p contrib/completion" found this one:
commit 1d66ec587e7d903afdf12a81718772a9eadc15a1
Author: SZEDER Gábor
Date: Thu Mar 10 19:12:29 2011 +0100
bash: complete 'git diff ...branc'
While doing a final sanity check before merging a topic Bsomething, it
is a good idea to review what damage Bsomething branch would make, by
running:
$ git diff ...Bsomething
Unfortunately, our completion script for 'git diff' doesn't offer
anything after '...'. This is because 'git diff's completion function
invokes __git_complete_file() for non-option arguments to complete the
':' extended SHA-1 notation, but this helper function
doesn't support refs after '...' or '..'. Completion of refs after
'...' or '..' is supported by the __git_complete_revlist() helper
function, but that doesn't support ':'.
To support both '...' and ':' notations for 'git
diff', this patch, instead of adding yet another helper function,
joins __git_complete_file() and __git_complete_revlist() into the new
common function __git_complete_revlist_file(). The old helper
functions __git_complete_file() and __git_complete_revlist() are
changed to be a direct wrapper around the new
__git_complete_revlist_file(), because they might be used in
user-supplied completion scripts and we don't want to break them.
This change will cause some wrong suggestions for other commands which
use __git_complete_file() ('git diff' and friends) or
__git_complete_revlist() ('git log' and friends), e.g. 'git diff
...master:Doc' and 'git log master:Doc' will complete the
path to 'Documentation/', although neither commands make any sense.
However, both of these were actively wrong to begin with as soon as
the user entered the ':', so there is no real harm done.
Suggested-by: Junio C Hamano
Signed-off-by: SZEDER Gábor
Signed-off-by: Junio C Hamano
Now I do not recall suggesting it, and you (and I today after 2
years) may disagree with the rationale, but at least we can read
what was the "intended" meaning, I think.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor wrote: > the one at the top because > of the reasons given in $gmane/226272 Sorry about the delay: I went to sleep for a couple of days :P > the one at the bottom because > of the misleading commit message (__git_complete_file() always > completed refs first as part of the ref:file notation, so it worked > just fine except for the ref1...ref2 notation; the real reason for > calling __git_complete_revlist_file() for difftool is to make clear > that difftool takes ref1...ref2:file, too). How am I (or anyone else) supposed to know the "intended" meaning __git_complete_file()? The implementation is just an alias to __git_complete_revlist_file(), so I looked at the name and guessed that it was supposed to complete files; now you tell me that it was intended to complete any revspec except revision ranges (what does that have to do with "file" again?). I suppose digging through the history would've told me, but I really didn't bother for such a trivial non-functional change. If you ask me, you should clamp down on spurious completions everywhere uniformly, if that is what you want (I personally have no interest in the topic). I see no reason to keep a badly named function around. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Forgot to cc; sorry about that. Junio C Hamano writes: > SZEDER Gábor writes: > >> On Thu, Jun 06, 2013 at 03:41:08PM -0700, Junio C Hamano wrote: >>> * rr/complete-difftool (2013-06-03) 2 commits >>> (merged to 'next' on 2013-06-04 at 01c7611) >>> + completion: clarify ls-tree, archive, show completion >>> + completion: difftool takes both revs and files >>> >>> Update command line completion (in contrib/) to use a better named >>> completion helper function for commands that take revisions and >>> paths. >>> >>> Will merge to 'master'. >> >> This should not be merged to master as is; the one at the top because >> of the reasons given in $gmane/226272, the one at the bottom because >> of the misleading commit message (__git_complete_file() always >> completed refs first as part of the ref:file notation, so it worked >> just fine except for the ref1...ref2 notation; the real reason for >> calling __git_complete_revlist_file() for difftool is to make clear >> that difftool takes ref1...ref2:file, too). > > Oops. > > It is too late to amend the log messages now, but at least a follow-up > patch can fix the breakage by adding __git_complete_file() back. Would > you mind doing that? > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: > On Thu, Jun 06, 2013 at 03:41:08PM -0700, Junio C Hamano wrote: >> * rr/complete-difftool (2013-06-03) 2 commits >> (merged to 'next' on 2013-06-04 at 01c7611) >> + completion: clarify ls-tree, archive, show completion >> + completion: difftool takes both revs and files >> >> Update command line completion (in contrib/) to use a better named >> completion helper function for commands that take revisions and >> paths. >> >> Will merge to 'master'. > > This should not be merged to master as is; the one at the top because > of the reasons given in $gmane/226272, the one at the bottom because > of the misleading commit message (__git_complete_file() always > completed refs first as part of the ref:file notation, so it worked > just fine except for the ref1...ref2 notation; the real reason for > calling __git_complete_revlist_file() for difftool is to make clear > that difftool takes ref1...ref2:file, too). Oops. It is too late to amend the log messages now, but at least a follow-up patch can fix the breakage by adding __git_complete_file() back. Would you mind doing that? Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Thu, Jun 06, 2013 at 03:41:08PM -0700, Junio C Hamano wrote: > * rr/complete-difftool (2013-06-03) 2 commits > (merged to 'next' on 2013-06-04 at 01c7611) > + completion: clarify ls-tree, archive, show completion > + completion: difftool takes both revs and files > > Update command line completion (in contrib/) to use a better named > completion helper function for commands that take revisions and > paths. > > Will merge to 'master'. This should not be merged to master as is; the one at the top because of the reasons given in $gmane/226272, the one at the bottom because of the misleading commit message (__git_complete_file() always completed refs first as part of the ref:file notation, so it worked just fine except for the ref1...ref2 notation; the real reason for calling __git_complete_revlist_file() for difftool is to make clear that difftool takes ref1...ref2:file, too). Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
