From: Martin von Zweigbergk <hidden> Date: 2016-06-15 22:50:15
Quite a few configuration variables have been added since 226b343
(completion: add missing configuration variables to _git_config(),
2009-05-03). Add these variables to the Bash completion script.
Signed-off-by: Martin von Zweigbergk <redacted>
---
The color.grep.external option has been deleted. Should it be deleted
from here or do we want to help users run e.g.
'git config --unset color.grep.external'? Same goes for
add.ignore-errors.
I didn't find any references to 'diff.renameLimit.' even in 98171a0
(bash completion: Sync config variables with their man pages,
2008-12-15) in which it was introduced in the completions script. I
hope it was safe to remove it.
Some variables are documented with camelCase but read in all
lowercase in the code. Not worth updating the code just for that, is
it?
I hope none of the added variables are deprecated. After having a
quick look in git-config(1), I think they should not be.
contrib/completion/git-completion.bash | 67 +++++++++++++++++++++++++++++++-
1 files changed, 65 insertions(+), 2 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:50:15
On Wed, Dec 15, 2010 at 07:46:53AM +0100, Martin von Zweigbergk wrote:
The color.grep.external option has been deleted. Should it be deleted
from here or do we want to help users run e.g.
'git config --unset color.grep.external'? Same goes for
add.ignore-errors.
IMHO, they should go away. People who have them can figure out how to
delete them, but it is more important not to advertise them to people
who are adding variables.
As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.
I didn't find any references to 'diff.renameLimit.' even in 98171a0
(bash completion: Sync config variables with their man pages,
2008-12-15) in which it was introduced in the completions script. I
hope it was safe to remove it.
Yeah, I don't think it has ever existed.
Some variables are documented with camelCase but read in all
lowercase in the code. Not worth updating the code just for that, is
it?
All variables are case-insensitive. The config parser down-cases them,
so all code should treat tham as all-lowercase. However, we tend to
document them as camelCase for readability.
The completion code should match case-insensitively, too. It doesn't
seem to now, but I suspect it is not a problem in practice because the
first camelCase word is often enough to get a match, and is lowercase
itself.
I hope none of the added variables are deprecated. After having a
quick look in git-config(1), I think they should not be.
From: Martin von Zweigbergk <hidden> Date: 2016-06-15 22:50:15
On Wed, 15 Dec 2010, Jeff King wrote:
On Wed, Dec 15, 2010 at 07:46:53AM +0100, Martin von Zweigbergk wrote:
quoted
The color.grep.external option has been deleted. Should it be deleted
from here or do we want to help users run e.g.
'git config --unset color.grep.external'? Same goes for
add.ignore-errors.
IMHO, they should go away. People who have them can figure out how to
delete them, but it is more important not to advertise them to people
who are adding variables.
Sounds good to me. If no one disagrees, I'll send an updated patch in
a day or two, removing both color.grep.external and add.ignore-errors.
As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.
Good point. I'll put it on my todo.
quoted
Some variables are documented with camelCase but read in all
lowercase in the code. Not worth updating the code just for that, is
it?
All variables are case-insensitive.
That I knew...
The config parser down-cases them,
so all code should treat tham as all-lowercase.
... but not that. Thanks. I could/should have figured that out if I
had noticed that *all* of them were lowercase in the code :-P.
We have color.diff.branch coming soon (I think it is in 'next' now).
Strictly speaking, that note is for Junio to think of when he merges,
right? But adding it early is pretty harmless and if that relieves him
of some work, I would be happy to add it in the next submission of
this patch. Is that better?
Thinking a bit more, maybe what you are suggesting is that I base the
next revision of this patch on the branch that adds that variable?
/Martin
We have color.diff.branch coming soon (I think it is in 'next' now).
Strictly speaking, that note is for Junio to think of when he merges,
right? But adding it early is pretty harmless and if that relieves him
of some work, I would be happy to add it in the next submission of
this patch. Is that better?
Thinking a bit more, maybe what you are suggesting is that I base the
next revision of this patch on the branch that adds that variable?
The "correct" thing to do from a topic branch standpoint is to submit
this patch without it as its own topic, submit a patch with just
color.diff.branch on top of the other topic, and then the merge
resolution will include both sets.
In this case, it might be OK to just start shipping color.diff.branch in
the completion list. It doesn't hurt anything to have the extra
completion before the feature is in, and the feature seems very likely
to make it in soon.
But I'll let Junio decide how meticulous about history he wants to be.
:)
-Peff
From: SZEDER Gábor <hidden> Date: 2016-06-15 22:50:15
Hi,
On Wed, Dec 15, 2010 at 08:44:45PM +0100, Martin von Zweigbergk wrote:
On Wed, 15 Dec 2010, Jeff King wrote:
quoted
As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.
Good point. I'll put it on my todo.
It already works that way since 0065236 (bash completion: complete
variable names for "git config" with options, 2009-05-08).
Best,
Gábor
From: Martin von Zweigbergk <hidden> Date: 2016-06-15 22:50:15
On Thu, 16 Dec 2010, SZEDER G?bor wrote:
Hi,
On Wed, Dec 15, 2010 at 08:44:45PM +0100, Martin von Zweigbergk wrote:
quoted
On Wed, 15 Dec 2010, Jeff King wrote:
quoted
As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.
Good point. I'll put it on my todo.
It already works that way since 0065236 (bash completion: complete
variable names for "git config" with options, 2009-05-08).
Nice. Thanks. Removed from my todo :-). Then I don't see any reason at
all to keep color.grep.external and add.ignore-errors.
/Martin
From: Jeff King <hidden> Date: 2016-06-15 22:50:15
On Thu, Dec 16, 2010 at 08:14:54AM +0100, Martin von Zweigbergk wrote:
quoted
On Wed, Dec 15, 2010 at 08:44:45PM +0100, Martin von Zweigbergk wrote:
quoted
On Wed, 15 Dec 2010, Jeff King wrote:
quoted
As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.
Good point. I'll put it on my todo.
It already works that way since 0065236 (bash completion: complete
variable names for "git config" with options, 2009-05-08).
Nice. Thanks. Removed from my todo :-). Then I don't see any reason at
all to keep color.grep.external and add.ignore-errors.
From: Martin von Zweigbergk <hidden> Date: 2016-06-15 22:50:17
Quite a few configuration variables have been added since 226b343
(completion: add missing configuration variables to _git_config(),
2009-05-03). Add these variables to the Bash completion script.
Also remove the obsolete 'add.ignore-errors' and
'color.grep.external', as well as 'diff.renameLimit.', which never
existed and rename the misspelled 'sendemail.aliasesfiletype'.
Signed-off-by: Martin von Zweigbergk <redacted>
---
Changes since previous version:
Removed 'add.ignore-errors' and 'color.grep.external'.
Added 'fetch.recurseSubmodules', which has entered master since last
round.
(Still based on master.)
contrib/completion/git-completion.bash | 70 ++++++++++++++++++++++++++++++--
1 files changed, 66 insertions(+), 4 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:50:17
On Mon, Dec 20, 2010 at 04:18:25PM +0100, Martin von Zweigbergk wrote:
Quite a few configuration variables have been added since 226b343
(completion: add missing configuration variables to _git_config(),
2009-05-03). Add these variables to the Bash completion script.
Also remove the obsolete 'add.ignore-errors' and
'color.grep.external', as well as 'diff.renameLimit.', which never
existed and rename the misspelled 'sendemail.aliasesfiletype'.
Signed-off-by: Martin von Zweigbergk <redacted>
---
Changes since previous version:
Removed 'add.ignore-errors' and 'color.grep.external'.
Added 'fetch.recurseSubmodules', which has entered master since last
round.
(Still based on master.)
Based on our prior discussion, I think this can go in, and all of the
other issues can be left for future patches if somebody wants to address
them.
Acked-by: Jeff King <redacted>
-Peff