Bash prompt "namespace" issue
Hi, Commit 59d3924fb (prompt: fix missing file errors in zsh) added the helper function eread() to git-prompt.sh. That function should be in the git "namespace", i.e. prefixed with __git_, otherwise it might collide with a function of the same name in the user's environment. It's already in master and I don't have the means to send a patch fixing this ATM, sorry. 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: [PATCH v2] completion: Add '--edit-todo' to rebase
Quoting Thomas Braun :
Signed-off-by: Thomas Braun
---
John Keeping hat am 13. Juli 2015 um 15:11 geschrieben:
git-rebase.sh contains:
if test "$action" = "edit-todo" && test "$type" != "interactive"
then
die "$(gettext "The --edit-todo action can only be used during
interactive
rebase.")"
fi
I wonder if it's worth doing a similar check here, which presumably
means testing if "$dir"/interactive exists.
Good point. Thanks for the hint.
Perhaps the subject line could say "completion: offer '--edit-todo'
during interactive rebase" to be a bit more specific.
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..b03050e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,7 +1668,11 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
+ if [ -d "$dir"/interactive ]; then
This doesn't work for me, I think it looks for the right file at the
wrong place. During an interactive rebase I have no
'.git/interactive' file but a '.git/rebase-merge/interactive', so I
never get '--edit-todo'.
After some playing around and a cursory look at the source it seems to
me that I have '.git/rebase-apply' during a "regular" rebase and
'.git/rebase-merge' during an interactive rebase, and git-rebase.sh
checks the presence of the 'interactive' file only in
'.git/rebase-merge'. It's not clear to me yet whether it's possible
to have a '.git/rebase-merge' without the file 'interactive' in it.
If it is possible, then I'd like to know with which commands and under
what circumstances. If it isn't, then we wouldn't have to look for
the file at all, because checking the presence of the directory would
be enough.
Best,
Gábor
+ __gitcomp "--continue --skip --abort --edit-todo"
+ else
+ __gitcomp "--continue --skip --abort"
+ fi
return
fi
__git_complete_strategy && return
--
2.4.5.windows.1
--
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: [PATCH v2] completion: Add '--edit-todo' to rebase
Quoting John Keeping :
On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:
Quoting Thomas Braun :
Signed-off-by: Thomas Braun
---
John Keeping hat am 13. Juli 2015 um 15:11
geschrieben:
git-rebase.sh contains:
if test "$action" = "edit-todo" && test "$type" != "interactive"
then
die "$(gettext "The --edit-todo action can only be used during
interactive
rebase.")"
fi
I wonder if it's worth doing a similar check here, which presumably
means testing if "$dir"/interactive exists.
Good point. Thanks for the hint.
Perhaps the subject line could say "completion: offer '--edit-todo'
during interactive rebase" to be a bit more specific.
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..b03050e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,7 +1668,11 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
+ if [ -d "$dir"/interactive ]; then
This doesn't work for me, I think it looks for the right file at the
wrong place. During an interactive rebase I have no
'.git/interactive' file but a '.git/rebase-merge/interactive', so I
never get '--edit-todo'.
Just noticed another issue here: it looks for a directory, though it
should look for a file.
After some playing around and a cursory look at the source it seems to
me that I have '.git/rebase-apply' during a "regular" rebase and
'.git/rebase-merge' during an interactive rebase, and git-rebase.sh
checks the presence of the 'interactive' file only in
'.git/rebase-merge'. It's not clear to me yet whether it's possible
to have a '.git/rebase-merge' without the file 'interactive' in it.
If it is possible, then I'd like to know with which commands and under
what circumstances. If it isn't, then we wouldn't have to look for
the file at all, because checking the presence of the directory would
be enough.
"git rebase --merge" will use ".git/rebase-merge" without creating the
"interactive" flag.
Oh, right, thanks. I should have remembered, I wrote the test of the
prompt script for that case...
(On a related note: is it possible to have a '.git/rebase-apply'
directory, but neither 'rebasing' or 'applying' files within? The
prompt script has a long if-elif chain with such a branch, and I
remember wondering how I could trigger it for testing.)
Anyway, so this could be something like (modulo likely whitespace damage):
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 07c34ef913..fac01d6985 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1667,7 +1667,10 @@ _git_push ()
_git_rebase ()
{
local dir="$(__gitdir)"
- if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+ if [ -f "$dir"/rebase-merge/interactive ]; then
+ __gitcomp "--continue --skip --abort --edit-todo"
+ return
+ elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
__gitcomp "--continue --skip --abort"
return
fi
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: [PATCH v2] completion: Add '--edit-todo' to rebase
Quoting Thomas Braun :
Am 31.07.2015 um 12:16 schrieb SZEDER Gábor:
Anyway, so this could be something like (modulo likely whitespace damage):
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 07c34ef913..fac01d6985 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1667,7 +1667,10 @@ _git_push ()
_git_rebase ()
{
local dir="$(__gitdir)"
-if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+if [ -f "$dir"/rebase-merge/interactive ]; then
+__gitcomp "--continue --skip --abort --edit-todo"
+return
+elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
__gitcomp "--continue --skip --abort"
return
fi
This looks much better than my attempt. Thanks.
How is the protocol now? Do I reroll and add
Helped-By: John Keeping
Completely-Overhauled-And-Properly-Implemented: SZEDER Gábor
Ugh :) I'm quite happy with Helped-by, if you do a proper reroll
after trying it out to see that it indeed does what it should.
Thanks,
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
[PATCHv3 1/2] config: add '--names-only' option to list only variable names
'git config' can only show values or name-value pairs, so if a shell
script needs the names of set config variables it has to run 'git config
--list' or '--get-regexp' and parse the output to separate config
variable names from their values. However, such a parsing can't cope
with multi-line values. Though 'git config' can produce null-terminated
output for newline-safe parsing, that's of no use in such a case, becase
shells can't cope with null characters.
Even our own bash completion script suffers from these issues.
Help the completion script, and shell scripts in general, by introducing
the '--names-only' option to modify the output of '--list' and
'--get-regexp' to list only the names of config variables, so they don't
have to perform error-prone post processing to separate variable names
from their values anymore.
Signed-off-by: SZEDER Gábor
---
Documentation/git-config.txt | 10 +++---
builtin/config.c | 14 --
contrib/completion/git-completion.bash | 1 +
t/t1300-repo-config.sh | 22 ++
4 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..ba76097c06 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -14,13 +14,13 @@ SYNOPSIS
'git config' [] [type] --replace-all name value [value_regex]
'git config' [] [type] [-z|--null] --get name [value_regex]
'git config' [] [type] [-z|--null] --get-all name [value_regex]
-'git config' [] [type] [-z|--null] --get-regexp name_regex
[value_regex]
+'git config' [] [type] [-z|--null] [--names-only] --get-regexp
name_regex [value_regex]
'git config' [] [type] [-z|--null] --get-urlmatch name URL
'git config' [] --unset name [value_regex]
'git config' [] --unset-all name [value_regex]
'git config' [] --rename-section old_name new_name
'git config' [] --remove-section name
-'git config' [] [-z|--null] -l | --list
+'git config' [] [-z|--null] [--names-only] -l | --list
'git config' [] --get-color name [default]
'git config' [] --get-colorbool name [stdout-is-tty]
'git config' [] -e | --edit
@@ -159,7 +159,7 @@ See also <>.
-l::
--list::
- List all variables set in config file.
+ List all variables set in config file, along with their values.
--bool::
'git config' will ensure that the output is "true" or "false"
@@ -190,6 +190,10 @@ See also <>.
output without getting confused e.g. by values that
contain line breaks.
+--names-only::
+ Output only the names of config variables for `--list` or
+ `--get-regexp`.
+
--get-colorbool name [stdout-is-tty]::
Find the color setting for `name` (e.g. `color.diff`) and output
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..307980ab50 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int omit_values;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -78,6 +79,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "path", &types, N_("value is a path (file or directory
name)"), TYPE_PATH),
OPT_GROUP(N_("Other")),
OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
+ OPT_BOOL(0, "names-only", &omit_values, N_("names only")),
OPT_BOOL(0, "includes", &respect_includes, N_("respect include
directives on lookup")),
OPT_END(),
};
@@ -91,7 +93,7 @@ static void check_argc(int argc, int min, int max) {
static int show_all_config(const char *key_, const char *value_, void *cb)
{
- if (value_)
+ if (!omit_values && value_)
printf("%s%c%s%c", key_, delim, value_, term);
else
printf("%s%c", key_, term);
@@ -117,6 +119,10 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
strbuf_addstr(buf, key_);
must_print_delim = 1;
}
+ if (omit_values) {
+ strbuf_addch(buf, term);
+ return 0;
+ }
if (types == TYPE_INT)
sprintf(value, "%"PRId64,
git_config_int64(key_, value_ ? value_ : ""));
@@ -549,7 +555,11 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
default:
usage_with_options(builtin_config_usage,
builtin_config
[PATCHv3 0/2] 'git config --names-only' to help the completion script
This is a reroll of 'sg/config-name-only'. * Instead of the two new listing options of the previous round add one new option '--names-only' to modify the output of '--list' and '--get-regexp' options, as suggested in previous discussions. * Reorganized the commit messages: don't go into details of the completion bug in the first patch modifying builtin/config.c, talk about that in the second patch. SZEDER Gábor (2): config: add '--names-only' option to list only variable names completion: list variable names reliably with 'git config --names-only' Documentation/git-config.txt | 10 +++--- builtin/config.c | 14 -- contrib/completion/git-completion.bash | 16 t/t1300-repo-config.sh | 22 ++ 4 files changed, 45 insertions(+), 17 deletions(-) -- 2.5.0.245.gff6622b -- 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
[PATCHv3 2/2] completion: list variable names reliably with 'git config --names-only'
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output
to strip the values and keep only the variable names. It does so by
looking for lines containing '.' and '=' and outputting everything
before the '=', which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
lines in its output are simply stripped after the first space, so
subsequent lines don't even have to contain '.' and '=' to fool the
completion script.
Use the new '--names-only' option added in the previous commit to list
config variable names reliably in both cases, without error-prone post
processing.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6eaab141e2..7200828fc4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -744,9 +744,8 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section="$1" i IFS=$'\n'
- for i in $(git --git-dir="$(__gitdir)" config --get-regexp
"^$section\..*" 2>/dev/null); do
- i="${i#$section.}"
- echo "${i/ */}"
+ for i in $(git --git-dir="$(__gitdir)" config --names-only --get-regexp
"^$section\..*" 2>/dev/null); do
+ echo "${i#$section.}"
done
}
@@ -1774,15 +1773,7 @@ __git_config_get_set_variables ()
c=$((--c))
done
- git --git-dir="$(__gitdir)" config $config_file --list 2>/dev/null |
- while read -r line
- do
- case "$line" in
- *.*=*)
- echo "${line/=*/}"
- ;;
- esac
- done
+ git --git-dir="$(__gitdir)" config $config_file --names-only --list
2>/dev/null
}
_git_config ()
--
2.5.0.245.gff6622b
--
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: [PATCHv3 1/2] config: add '--names-only' option to list only variable names
Quoting Junio C Hamano :
SZEDER Gábor writes:
'git config' can only show values or name-value pairs, so if a shell
script needs the names of set config variables it has to run 'git config
--list' or '--get-regexp' and parse the output to separate config
variable names from their values. However, such a parsing can't cope
with multi-line values. Though 'git config' can produce null-terminated
output for newline-safe parsing, that's of no use in such a case, becase
s/becase/because/;
OK.
shells can't cope with null characters.
Even our own bash completion script suffers from these issues.
Help the completion script, and shell scripts in general, by introducing
the '--names-only' option to modify the output of '--list' and
'--get-regexp' to list only the names of config variables, so they don't
have to perform error-prone post processing to separate variable names
from their values anymore.
I agree with Peff that "--names-only" has a subtle difference with
an existing and well known subcommand option and it would be a bit
irritating to remember which options is for which command.
OK.
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..307980ab50 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int omit_values;
static int use_key_regexp;
static int do_all;
static int do_not_match;
...
@@ -91,7 +93,7 @@ static void check_argc(int argc, int min, int max) {
static int show_all_config(const char *key_, const char *value_, void *cb)
{
- if (value_)
+ if (!omit_values && value_)
H. As we have "show_keys",
if (show_values && value_)
would be a lot more intuitive, no?
Well, the name 'omit_values' was suggested by Peff after the first
round. I'm happy to rename it to whatever you agree upon :)
@@ -117,6 +119,10 @@ static int format_config(struct strbuf *buf,
const char *key_, const char *value
strbuf_addstr(buf, key_);
must_print_delim = 1;
}
+ if (omit_values) {
+ strbuf_addch(buf, term);
+ return 0;
+ }
This hunk makes me wonder what the assignment to "must_print_delim"
is about. When the code is told to show only keys and not values,
it shouldn't even have to worry about key_delim, but that assignment
is done to control exactly that. It happens that you are lucky that
you can "return 0" early here so that the assignment does not have
any effect, but still conceptually the code structure is made ugly
by this patch.
How about restructuring the function like this? Perhaps even better
than a tri-state toggle would be.
(showing the result instead of the diff, because all the indentation
changes make the diff hard to read).
static int format_config(struct strbuf *buf, const char *key_, const
char *value_)
{
strbuf_init(buf, 0);
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) { // or show_values
int must_free_vptr = 0;
int must_add_delim = show_keys;
char value[256];
const char *vptr = value;
if (types == TYPE_INT)
sprintf(value, "%"PRId64,
git_config_int64(key_, value_ ? value_ : ""));
else if (types == TYPE_BOOL)
vptr = git_config_bool(key_, value_) ? "true"
: "false";
else if (types == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, &is_bool);
if (is_bool)
vptr = v ? "true" : "false";
else
sprintf(value, "%d", v);
} else if (types == TYPE_PATH) {
if (git_config_pathname(&vptr, key_, value_) < 0)
return -1;
must_free_vptr = 1;
} else if (value_) {
vptr = value_;
} else {
/* Just show the key name */
vptr = "";
must_add_delim = 0;
}
if (must_add_delim)
strbuf_addch(buf, key_delim);
strbuf_addstr(buf, vptr);
if (must_free_vptr)
free((char *)vptr);
}
strbuf_addch(buf, term);
return 0;
}
Isn't it more like the existing "show_keys" can be replaced/enhan
[PATCH] describe: make '--always' fallback work after '--exact-match' failed
'git describe [...] --always' should always show the unique abbreviated
object name as a fallback when the given commit cannot be described with
the given set of options, see da2478dbb0 (describe --always: fall back
to showing an abbreviated object name, 2008-03-02).
However, this is not the case when the combination '--exact-match
--always' is given and the commit cannot be described, because in such
cases 'git describe' errors out, as if '--always' were not given at all.
Respect '--always' and print the unique abbreviated object name instead
of erroring out when the commit cannot be described with '--exact-match
--always'.
Signed-off-by: SZEDER Gábor
---
builtin/describe.c | 2 +-
t/t6120-describe.sh | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index a36c829e57..ce36032b1f 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -266,7 +266,7 @@ static void describe(const char *arg, int last_one)
return;
}
- if (!max_candidates)
+ if (!always && !max_candidates)
die(_("no tag exactly matches '%s'"),
sha1_to_hex(cmit->object.sha1));
if (debug)
fprintf(stderr, _("searching to describe %s\n"), arg);
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index c0e5b2a627..57d50156bb 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -113,6 +113,8 @@ check_describe A-3-* --long HEAD^^2
check_describe c-7-* --tags
check_describe e-3-* --first-parent --tags
+check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
+
: >err.expect
check_describe A --all A^0
test_expect_success 'no warning was displayed for A' '
--
2.5.0.343.g6f38143
--
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
Minor builtin 'git am' side-effect
Hi,
The format of the files '.git/rebase-apply/{next,last}' changed slightly
with the recent builtin 'git am' conversion: while these files were
newline-terminated when written by the scripted version, the ones written
by the builtin are not.
This probably makes no difference for shell scripts looking at these
files, e.g. our __git_ps1() handles both just fine. However, it can
break C programs, when after strtol()ing the contents of the files they
get defensive and check for the terminating newline at *endptr (this is
how I noticed, as one of my pet projects did just that).
I'm not saying that the new behavior is bad and should be fixed; I merely
point it out and leave the rest for you to decide.
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
[PATCH] t3020: fix typo in test description
Signed-off-by: SZEDER Gábor --- t/t3020-ls-files-error-unmatch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh index ca01053bcc..124e73b8e6 100755 --- a/t/t3020-ls-files-error-unmatch.sh +++ b/t/t3020-ls-files-error-unmatch.sh @@ -22,7 +22,7 @@ test_expect_success \ 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match' test_expect_success \ -'git ls-files --error-unmatch should succeed eith matched paths.' \ +'git ls-files --error-unmatch should succeed with matched paths.' \ 'git ls-files --error-unmatch foo bar' test_done -- 2.5.0.415.g33d64ef -- 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
[PATCH] wt-status: move #include "pathspec.h" to the header
The declaration of 'struct wt_status' requires the declararion of 'struct
pathspec'.
Signed-off-by: SZEDER Gábor
---
wt-status.c | 1 -
wt-status.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/wt-status.c b/wt-status.c
index 717fd48d13..c327fe8128 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1,5 +1,4 @@
#include "cache.h"
-#include "pathspec.h"
#include "wt-status.h"
#include "object.h"
#include "dir.h"
diff --git a/wt-status.h b/wt-status.h
index e0a99f75c7..c9b3b744e9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -4,6 +4,7 @@
#include
#include "string-list.h"
#include "color.h"
+#include "pathspec.h"
enum color_wt_status {
WT_STATUS_HEADER = 0,
--
2.5.0.415.g33d64ef
--
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
[PATCH] config: restructure format_config() for better control flow
Commit 578625fa91 (config: add '--name-only' option to list only
variable names, 2015-08-10) modified format_config() such that it
returned from the middle of the function when showing only keys,
resulting in ugly code structure.
Reorganize the if statements and dealing with the key-value delimiter to
make the function easier to read.
Signed-off-by: SZEDER Gábor
---
> The topic is now in 'next'; I think I've locally fixed it up for
> these when I originally queued them a few days ago, so if there are
> any remaining issues, please throw incremental polishing patches.
OK, though it's not a major issue, I think this is still worth doing on
top.
builtin/config.c | 78 +++-
1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 631db458ec..810e104224 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -108,52 +108,48 @@ struct strbuf_list {
static int format_config(struct strbuf *buf, const char *key_, const char
*value_)
{
- int must_free_vptr = 0;
- int must_print_delim = 0;
- char value[256];
- const char *vptr = value;
-
strbuf_init(buf, 0);
- if (show_keys) {
+ if (show_keys)
strbuf_addstr(buf, key_);
- must_print_delim = 1;
- }
- if (omit_values) {
- strbuf_addch(buf, term);
- return 0;
- }
- if (types == TYPE_INT)
- sprintf(value, "%"PRId64,
- git_config_int64(key_, value_ ? value_ : ""));
- else if (types == TYPE_BOOL)
- vptr = git_config_bool(key_, value_) ? "true" : "false";
- else if (types == TYPE_BOOL_OR_INT) {
- int is_bool, v;
- v = git_config_bool_or_int(key_, value_, &is_bool);
- if (is_bool)
- vptr = v ? "true" : "false";
- else
- sprintf(value, "%d", v);
- } else if (types == TYPE_PATH) {
- if (git_config_pathname(&vptr, key_, value_) < 0)
- return -1;
- must_free_vptr = 1;
- } else if (value_) {
- vptr = value_;
- } else {
- /* Just show the key name */
- vptr = "";
- must_print_delim = 0;
- }
+ if (!omit_values) {
+ int must_free_vptr = 0;
+ int must_add_delim = show_keys;
+ char value[256];
+ const char *vptr = value;
- if (must_print_delim)
- strbuf_addch(buf, key_delim);
- strbuf_addstr(buf, vptr);
+ if (types == TYPE_INT)
+ sprintf(value, "%"PRId64,
+ git_config_int64(key_, value_ ? value_ : ""));
+ else if (types == TYPE_BOOL)
+ vptr = git_config_bool(key_, value_) ? "true" : "false";
+ else if (types == TYPE_BOOL_OR_INT) {
+ int is_bool, v;
+ v = git_config_bool_or_int(key_, value_, &is_bool);
+ if (is_bool)
+ vptr = v ? "true" : "false";
+ else
+ sprintf(value, "%d", v);
+ } else if (types == TYPE_PATH) {
+ if (git_config_pathname(&vptr, key_, value_) < 0)
+ return -1;
+ must_free_vptr = 1;
+ } else if (value_) {
+ vptr = value_;
+ } else {
+ /* Just show the key name */
+ vptr = "";
+ must_add_delim = 0;
+ }
+
+ if (must_add_delim)
+ strbuf_addch(buf, key_delim);
+ strbuf_addstr(buf, vptr);
+
+ if (must_free_vptr)
+ free((char *)vptr);
+ }
strbuf_addch(buf, term);
-
- if (must_free_vptr)
- free((char *)vptr);
return 0;
}
--
2.5.0.415.g33d64ef
--
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: [PATCH] wt-status: move #include "pathspec.h" to the header
Quoting Junio C Hamano : SZEDER Gábor writes: The declaration of 'struct wt_status' requires the declararion of 'struct pathspec'. I think this is fine. I am guessing that you are saying it is wrong to force wt-status.c to include pathspec.h before including wt-status.h; I am fine with that. This is a tangent, but the above is different from saying that with a single liner test.c that has #include "wt-status.h" your compilation "cc -c test.c" should succeed. Sort of, but not quite, rather a combination of both. My point is a builtin command, starting with #include "builtin.h" as it should, that wanted to use only struct wt_status_state and wt_status_get_state() from the wt-status machinery and has nothing to do with pathspecs, yet it's not sufficient to #include "wt-status.h", because the required pathspec.h is included in the .c file instead of the header itsef. (Actually, that's exactly how I noticed.) But for that goal, direct inclusion of to wt-status.h is also iffy. We include the system headers from git-compat-util.h because some platforms are picky about order of inclusion of system header files and definitions of feature test macros. Right now, the codebase is correct only because it is NOT our goal to guarantee that such a single-liner test.c file compiles. Instead, everybody is instructed to #include "git-compat-util.h" as the first thing, either directly or indirectly. So in that sense, we should also remove that inclusion from wt-status.h, I think. 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: [PATCH] describe: make '--always' fallback work after '--exact-match' failed
Quoting Junio C Hamano : SZEDER Gábor writes: 'git describe [...] --always' should always show the unique abbreviated object name as a fallback when the given commit cannot be described with the given set of options, see da2478dbb0 (describe --always: fall back to showing an abbreviated object name, 2008-03-02). However, this is not the case when the combination '--exact-match --always' is given and the commit cannot be described, because in such cases 'git describe' errors out, as if '--always' were not given at all. Respect '--always' and print the unique abbreviated object name instead of erroring out when the commit cannot be described with '--exact-match --always'. Signed-off-by: SZEDER Gábor Well, that can be argued both ways. Your patch introduces a regression, as "--exact-match" is an instruction to error out when no tag exactly matches, and you deliberately break that. This patch doesn't break '--exact-match', in fact doesn't modify it at all when it's on its own or combined with other options, but it makes '--exact-match --always' finally work. 'git describe' errors out by default if it can't describe the given commit. So if a user wants an exact match or an error otherwise, then he should not give '--always' at all, because that's the instruction to not error out but give the abbreviated object name instead. Why should '--exact-match' be any different from the other options that tell 'git describe' what to use for the description? Why should '--always' not work with '--exact-match', when it works in the other cases? Consider '--contains': it should find a tag that comes after the given commit or error out if such a tag doesn't exist. Now, in current git.git: $ git describe --contains master fatal: cannot describe 'ff86faf2fa02bc21933c9e1dcc75c8d81b3e104a' $ git describe --contains --always master ff86faf2fa Or the default behavior without any options: it should find a tag reachable from the given commit or error out, but what if we pass in a commit before the first tag? It recommends '--always': $ git describe e83c516 fatal: No tags can describe 'e83c5163316f89bfbde7d9ab23ca2e25604af290'. Try --always, or create some tags. $ git describe --always e83c516 e83c516331 My knee-jerk reaction is that the most sensible way forward is to make --exact-match and --always mutually incompatible. That would be wrong, it's perfectly valid to ask for an exactly matching tag or, if there is no such tag, the abbreviated object name as a fallback. -- 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: [PATCH 2/3] format_config: simplify buffer handling
Quoting Jeff King :
When formatting a config value into a strbuf, we may end
up stringifying it into a fixed-size buffer using sprintf,
and then copying that buffer into the strbuf. We can
eliminate the middle-man (and drop some calls to sprintf!)
by writing directly to the strbuf.
The reason it was written this way in the first place is
that we need to know before writing the value whether to
insert a delimiter. Instead of delaying the write of the
value, we speculatively write the delimiter, and roll it
back in the single case that cares.
Signed-off-by: Jeff King
---
I admit the rollback is a little gross.
Indeed it is, but I'm for it, as it gets rit of so much more
other grossness, i.e. the fixed-size buffer and vptr stuff
and the two must_do_this variables.
builtin/config.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 91aa56f..04befce 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -111,41 +111,35 @@ static int format_config(struct strbuf *buf,
const char *key_, const char *value
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) {
- int must_free_vptr = 0;
- int must_add_delim = show_keys;
- char value[256];
- const char *vptr = value;
+ if (show_keys)
+ strbuf_addch(buf, key_delim);
if (types == TYPE_INT)
- sprintf(value, "%"PRId64,
- git_config_int64(key_, value_ ? value_ : ""));
+ strbuf_addf(buf, "%"PRId64,
+ git_config_int64(key_, value_ ? value_ :
""));
else if (types == TYPE_BOOL)
- vptr = git_config_bool(key_, value_) ? "true" : "false";
+ strbuf_addstr(buf, git_config_bool(key_, value_) ?
+ "true" : "false");
else if (types == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, &is_bool);
if (is_bool)
- vptr = v ? "true" : "false";
+ strbuf_addstr(buf, v ? "true" : "false");
else
- sprintf(value, "%d", v);
+ strbuf_addf(buf, "%d", v);
} else if (types == TYPE_PATH) {
- if (git_config_pathname(&vptr, key_, value_) < 0)
+ const char *v;
+ if (git_config_pathname(&v, key_, value_) < 0)
return -1;
- must_free_vptr = 1;
+ strbuf_addstr(buf, v);
+ free((char *)v);
} else if (value_) {
- vptr = value_;
+ strbuf_addstr(buf, value_);
} else {
- /* Just show the key name */
- vptr = "";
- must_add_delim = 0;
+ /* Just show the key name; back out delimiter */
+ if (show_keys)
+ strbuf_setlen(buf, buf->len - 1);
}
-
- if (must_add_delim)
- strbuf_addch(buf, key_delim);
- strbuf_addstr(buf, vptr);
-
- if (must_free_vptr)
- free((char *)vptr);
}
strbuf_addch(buf, term);
return 0;
--
2.5.0.680.g69e7703
--
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
[PATCH] describe --contains: default to HEAD when no commit-ish is given
'git describe --contains' doesn't default to HEAD when no commit is
given, and it doesn't produce any output, not even an error:
~/src/git ((v2.5.0))$ ./git describe --contains
~/src/git ((v2.5.0))$ ./git describe --contains HEAD
v2.5.0^0
Unlike other 'git describe' options, the '--contains' code path is
implemented by calling 'name-rev' with a bunch of options plus all the
commit-ishes that were passed to 'git describe'. If no commit-ish was
present, then 'name-rev' got invoked with none, which then leads to the
behavior illustrated above.
Porcelain commands usually default to HEAD when no commit-ish is given,
and 'git describe' already does so in all other cases, so it should do
so with '--contains' as well.
Pass HEAD to 'name-rev' when no commit-ish is given on the command line
to make '--contains' behave consistently with other 'git describe'
options.
'git describe's short help already indicates that the commit-ish is
optional, but the synopsis in the man page doesn't, so update it
accordingly as well.
Signed-off-by: SZEDER Gábor
---
Documentation/git-describe.txt | 4 ++--
builtin/describe.c | 11 +++
t/t6120-describe.sh| 8
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e045fc73f8..c8f28c8c86 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -9,7 +9,7 @@ git-describe - Describe a commit using the most recent tag
reachable from it
SYNOPSIS
[verse]
-'git describe' [--all] [--tags] [--contains] [--abbrev=] ...
+'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]
'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]
DESCRIPTION
@@ -27,7 +27,7 @@ see the -a and -s options to linkgit:git-tag[1].
OPTIONS
---
...::
- Commit-ish object names to describe.
+ Commit-ish object names to describe. Defaults to HEAD if omitted.
--dirty[=]::
Describe the working tree.
diff --git a/builtin/describe.c b/builtin/describe.c
index ce36032b1f..0e31ac5cb9 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv, const char
*prefix)
if (pattern)
argv_array_pushf(&args, "--refs=refs/tags/%s",
pattern);
}
- while (*argv) {
- argv_array_push(&args, *argv);
- argv++;
- }
+ if (argc)
+ while (*argv) {
+ argv_array_push(&args, *argv);
+ argv++;
+ }
+ else
+ argv_array_push(&args, "HEAD");
return cmd_name_rev(args.argc, args.argv, prefix);
}
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 57d50156bb..bf52a0a1cc 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
+test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
+ echo "A^0" >expect &&
+ git checkout A &&
+ test_when_finished "git checkout -" &&
+ git describe --contains >actual &&
+ test_cmp expect actual
+'
+
: >err.expect
check_describe A --all A^0
test_expect_success 'no warning was displayed for A' '
--
2.5.0.416.g84b07b4
--
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: [RFC/PATCH] contrib: teach completion about git-worktree options and arguments
Quoting Junio C Hamano :
Eric Sunshine writes:
On Thu, Jul 23, 2015 at 4:49 PM, Eric Sunshine
wrote:
Complete subcommands 'add' and 'prune', as well as their respective
options --force, --detach, --dry-run, --verbose, and --expire. Also
complete 'refname' in "git worktree add [-b ]
".
Ping[1]?
[1]: http://article.gmane.org/gmane.comp.version-control.git/274526
Ping indeed?
Yeah, right, sorry. Non-subscribed occasional gmane-reader here amidst
job hunting, so thanks for the ping. And the re-ping...
Signed-off-by: Eric Sunshine
---
This is RFC since this is my first foray into the Git completion script,
and there are likely better and more idiomatic ways to accomplish this.
Using __git_find_on_cmdline() to find subcommands and case
"$subcommand,$cur" to limit the number of nested case statements is as
idiomatic as you can get in the completion script.
And I hear you, that " first, second" syntax makes it way
too complicated, especially since they can follow '-b '.
I wrote a completion function for 'git worktree' as well, turns out a week
or two before you posted this, but I never submitted it as it was way too
convoluted. Anyway, will send it in reply to this, just for reference.
contrib/completion/git-completion.bash | 32
1 file changed, 32 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..07c34ef 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2564,6 +2564,38 @@ _git_whatchanged ()
_git_log
}
+_git_worktree ()
+{
+ local subcommands='add prune'
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local c=2 n=0 refpos=2
A more descriptive variable name for 'n' would be great.
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+ else
+ case "$subcommand,$cur" in
+ add,--*)
+ __gitcomp "--force --detach"
We usually don't offer '--force', because that option must be
handled with care.
+ ;;
+ add,*)
+ while [ $c -lt $cword ]; do
+ case "${words[c]}" in
+ --*) ;;
+ -[bB]) ((refpos++)) ;;
+ *) ((n++)) ;;
+ esac
+ ((c++))
+ done
+ if [ $n -eq $refpos ]; then
I suppose here you wanted to calculate where (i.e. at which word on
the command line) we should offer refs and fall back to bash builtin
filename completion otherwise. It works well in the common cases,
but:
- it doesn't offer refs after -b or -B
- it gets fooled by options to the git command, e.g. 'git
--git-dir=.git worktree add ' offers refs instead of files,
'git --git-dir=.git worktree add ../some/path ' offers
refs, etc.
+ __gitcomp_nl "$(__git_refs)"
+ fi
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run --verbose --expire"
+ ;;
+ esac
+ fi
+}
+
__git_main ()
{
local i c=1 command __git_dir
--
2.5.0.rc3.407.g68aafd0
--
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
[PoC PATCH] completion: support 'git worktree'
Signed-off-by: SZEDER Gábor
---
> I wrote a completion function for 'git worktree' as well, turns out a week
> or two before you posted this, but I never submitted it as it was way too
> convoluted. Anyway, will send it in reply to this, just for reference.
And here it is.
>From the number of indentation levels and comment lines you can see why
I haven't submitted this patch yet :)
OTOH it offers refs for -b and -B, and there are only fairly narrow
corner cases when 'git --options' can fool it (but that's a general
issue with __git_find_on_cmdline(), I wouldn't go into that).
contrib/completion/git-completion.bash | 59 ++
1 file changed, 59 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648d7e..20a17e2c50 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2564,6 +2564,65 @@ _git_whatchanged ()
_git_log
}
+_git_worktree ()
+{
+ local subcommand subcommand_idx sc c=1
+ local subcommands="add prune"
+
+ while [ $c -lt $cword ] && [ -z "$subcommand" ]; do
+ for sc in $subcommands; do
+ if [ "$sc" = "${words[c]}" ]; then
+ subcommand=$sc
+ subcommand_idx=$c
+ break
+ fi
+ done
+ ((c++))
+ done
+
+ case "$subcommand,$cur" in
+ ,*)
+ __gitcomp "$subcommands"
+ ;;
+ add,--*)
+ __gitcomp "--detach"
+ ;;
+ add,*)
+ case "$prev" in
+ -b|-B)
+ __gitcomp_nl "$(__git_refs)"
+ ;;
+ -*) # $prev is an option without argument: have to complete
+ # the path for the new worktree, fall back to bash
+ # filename completion
+ ;;
+ *) # $prev is not an option, so it must be either the
+ # 'add' subcommand, an argument of an option (e.g.
+ # branch for -b|-B), or the path for the new worktree
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
+ # right after the 'add' subcommand, have to
+ # complete the path
+ :
+ else
+ case "${words[cword-2]}" in
+ -b|-B) # after '-b ', have to complete
+ # the path
+ ;;
+ *) # after the path, have to complete the
+ # branch to be checked out
+ __gitcomp_nl "$(__git_refs)"
+ ;;
+ esac
+ fi
+ ;;
+ esac
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run --verbose --expire"
+ ;;
+ esac
+}
+
__git_main ()
{
local i c=1 command __git_dir
--
2.5.0.418.gdd37a9b
--
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: [PATCH] am: terminate state files with a newline
Hi, Quoting Paul Tan : Did we ever explictly allow external programs to poke around the contents of the .git/rebase-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/ . Think of e.g. libgit2, JGit/EGit and all the other git implementations. They should be able to look everywhere in .git, shouldn't they? I don't think we will just "switch" the storage format of any parts of the repo. Whatever new formats may come (ref backends, index v5, pack v4), they will be an opt-in feature for a long time before becoming default, and there must be an even longer deprecation period before the old format gets phased out, if ever. 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: [PATCH] describe --contains: default to HEAD when no commit-ish is given
Quoting Junio C Hamano :
@@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv,
const char *prefix)
if (pattern)
argv_array_pushf(&args, "--refs=refs/tags/%s",
pattern);
}
- while (*argv) {
- argv_array_push(&args, *argv);
- argv++;
- }
+ if (argc)
"What would this code do to 'describe --all --contains'?" was my
knee-jerk reaction, but the options are all parsed by this code and
nothing is delegated to name-rev, so 'if (!argc)' here is truly the
lack of any revisions to be described, which is good.
Exactly. parse-opts removes all --options from argv as it processes
them, barfs at --unknown-options, so all what remains must be treated
as a commit-ish. And if nothing is left, well, then there was none
given.
+ while (*argv) {
+ argv_array_push(&args, *argv);
+ argv++;
+ }
+ else
+ argv_array_push(&args, "HEAD");
By the way, I usually prefer a fatter 'else' clause when everything
else is equal, i.e.
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD */
else {
while (*argv) {
...
}
}
because it is easy to miss tiny else-clause while reading code, but
it is harder to miss tiny then-clause. In this case, however, the
while loop can be replaced with argv_array_pushv() these days, so
perhaps
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD ... */
else
argv_array_pushv(&args, argv); /* or relay what we got */
or something?
Indeed, I didn't notice argv_array_pushv() being added, log tells me
it happened quite recently. I suppose with both branches becoming a
one-liner the order of them can remain what it was in the patch,
this sparing the negation from 'if (!argc)'.
v2 comes in a minute.
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
[PATCH v2] describe --contains: default to HEAD when no commit-ish is given
'git describe --contains' doesn't default to HEAD when no commit is
given, and it doesn't produce any output, not even an error:
~/src/git ((v2.5.0))$ ./git describe --contains
~/src/git ((v2.5.0))$ ./git describe --contains HEAD
v2.5.0^0
Unlike other 'git describe' options, the '--contains' code path is
implemented by calling 'name-rev' with a bunch of options plus all the
commit-ishes that were passed to 'git describe'. If no commit-ish was
present, then 'name-rev' got invoked with none, which then leads to the
behavior illustrated above.
Porcelain commands usually default to HEAD when no commit-ish is given,
and 'git describe' already does so in all other cases, so it should do
so with '--contains' as well.
Pass HEAD to 'name-rev' when no commit-ish is given on the command line
to make '--contains' behave consistently with other 'git describe'
options. While at it, use argv_array_pushv() instead of the loop to
pass commit-ishes to 'git name-rev'.
'git describe's short help already indicates that the commit-ish is
optional, but the synopsis in the man page doesn't, so update it
accordingly as well.
Signed-off-by: SZEDER Gábor
---
Documentation/git-describe.txt | 4 ++--
builtin/describe.c | 8
t/t6120-describe.sh| 8
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e045fc73f8..c8f28c8c86 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -9,7 +9,7 @@ git-describe - Describe a commit using the most recent tag
reachable from it
SYNOPSIS
[verse]
-'git describe' [--all] [--tags] [--contains] [--abbrev=] ...
+'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]
'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]
DESCRIPTION
@@ -27,7 +27,7 @@ see the -a and -s options to linkgit:git-tag[1].
OPTIONS
---
...::
- Commit-ish object names to describe.
+ Commit-ish object names to describe. Defaults to HEAD if omitted.
--dirty[=]::
Describe the working tree.
diff --git a/builtin/describe.c b/builtin/describe.c
index ce36032b1f..9dadfb58c8 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -443,10 +443,10 @@ int cmd_describe(int argc, const char **argv, const char
*prefix)
if (pattern)
argv_array_pushf(&args, "--refs=refs/tags/%s",
pattern);
}
- while (*argv) {
- argv_array_push(&args, *argv);
- argv++;
- }
+ if (argc)
+ argv_array_pushv(&args, argv);
+ else
+ argv_array_push(&args, "HEAD");
return cmd_name_rev(args.argc, args.argv, prefix);
}
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 57d50156bb..bf52a0a1cc 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
+test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
+ echo "A^0" >expect &&
+ git checkout A &&
+ test_when_finished "git checkout -" &&
+ git describe --contains >actual &&
+ test_cmp expect actual
+'
+
: >err.expect
check_describe A --all A^0
test_expect_success 'no warning was displayed for A' '
--
2.5.0.418.gdd37a9b
--
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: [PATCH] stash: Add stash.showFlag config variable
Hi,
I haven't made up my mind about this feature yet, but have a few
comments about its implementation.
> diff --git a/git-stash.sh b/git-stash.sh
> index 1d5ba7a..8432435 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -33,6 +33,12 @@ else
> reset_color=
> fi
>
> +if git config --get stash.showflag > /dev/null 2> /dev/null; then
> + show_flag=$(git config --get stash.showflag)
> +else
> + show_flag=--stat
> +fi
> +
Forking and executing processes are costly on some important platforms
we care about, so we should strive to avoid them whenever possible.
- This hunk runs the the exact same 'git config' command twice. Run it
only once, perhaps something like this:
show_flag=$(git config --get stash.showflag || echo --stat)
(I hope there are no obscure crazy 'echo' implemtations out there
that might barf on the unknown option '--stat'...)
- It runs 'git config' in the main code path, i.e. even for subcommands
other than 'show'. Run it only for 'git stash show'.
- This config setting is not relevant if there were options given on the
command line. Run it only if there are no options given, i.e. when
$FLAGS is empty.
> no_changes () {
> git diff-index --quiet --cached HEAD --ignore-submodules -- &&
> git diff-files --quiet --ignore-submodules &&
> @@ -305,7 +311,7 @@ show_stash () {
> ALLOW_UNKNOWN_FLAGS=t
> assert_stash_like "$@"
>
> - git diff ${FLAGS:---stat} $b_commit $w_commit
> + git diff ${FLAGS:-${show_flag}} $b_commit $w_commit
> }
>
> show_help () {
> --
> 2.5.0
--
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
[PATCH] completion: fix completion after 'git -C path'
The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word. As it is not aware of 'git -C's
mandatory path argument, if the '-C path' option is present, 'path' will
be the first such word and it will be mistaken for a git command. This
breaks the completion script in various ways:
- If 'path' happens to match one of the commands supported by the
completion script, then its options will be offered.
- If 'path' doesn't match a supported command and doesn't contain any
characters not allowed in Bash identifier names, then the completion
script does basically nothing and lets Bash to fall back to filename
completion.
- Otherwise, if 'path' contains such unallowed characters, then it
leads to a more or less ugly error in the middle of the command line.
The standard '/' directory separator is such a character, and it
happens to trigger one of the uglier errors:
$ git -C some/path sh.exe": declare: `_git_some/path': not a valid
identifier
error: invalid key: alias.some/path
Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line. Extend the relevant test with this
case and, while at it, with cases that needed similar treatment in the
past ('--git-dir', '-c', '--work-tree' and '--namespace').
Additionally, shut up standard error of the 'declare' commands looking
for the associated completion function and of the 'git config' query for
the aliased command, so if git learns a new option with a mandatory
argument in the future, then at least the command line will not be
utterly disrupted by those error messages.
Note, that this change merely fixes the breakage related to 'git -C
path', but the completion script will not take it into account as it
does '--git-dir path'.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 8
t/t9902-completion.sh | 7 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 482ca84b45..80dc717fe2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -763,7 +763,7 @@ __git_aliases ()
__git_aliased_command ()
{
local word cmdline=$(git --git-dir="$(__gitdir)" \
- config --get "alias.$1")
+ config --get "alias.$1" 2>/dev/null)
for word in $cmdline; do
case "$word" in
\!gitk|gitk)
@@ -2571,7 +2571,7 @@ __git_main ()
--git-dir) ((c++)) ; __git_dir="${words[c]}" ;;
--bare) __git_dir="." ;;
--help) command="help"; break ;;
- -c|--work-tree|--namespace) ((c++)) ;;
+ -c|-C|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command="$i"; break ;;
esac
@@ -2604,13 +2604,13 @@ __git_main ()
fi
local completion_func="_git_${command//-/_}"
- declare -f $completion_func >/dev/null && $completion_func && return
+ declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
&& return
local expansion=$(__git_aliased_command "$command")
if [ -n "$expansion" ]; then
words[1]=$expansion
completion_func="_git_${expansion//-/_}"
- declare -f $completion_func >/dev/null && $completion_func
+ declare -f $completion_func >/dev/null 2>/dev/null &&
$completion_func
fi
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2ba62fbc17..f5a669918d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -480,7 +480,12 @@ test_expect_success 'general options plus command' '
test_completion "git --namespace=foo check" "checkout " &&
test_completion "git --paginate check" "checkout " &&
test_completion "git --info-path check" "checkout " &&
- test_completion "git --no-replace-objects check" "checkout "
+ test_completion "git --no-replace-objects check" "checkout " &&
+ test_completion "git --git-dir some/path check" "checkout " &&
+ test_completion "git -c conf.var=value check" "checkout " &&
+ test_completion "git -C some/path check" "checkout " &&
+ test_completion "git --work-tree some/path check" "checkout " &&
+ test_completion "git --namespace name/space check" "checkout "
'
test_expect_success 'git --help completion' '
--
2.6.0.rc2.22.g7128296
--
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: [PATCH] completion: fix completion after 'git -C path'
Quoting Michael J Gruber :
SZEDER Gábor venit, vidit, dixit 05.10.2015 14:02:
The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word. As it is not aware of 'git -C's
mandatory path argument, if the '-C path' option is present, 'path' will
be the first such word and it will be mistaken for a git command. This
breaks the completion script in various ways:
- If 'path' happens to match one of the commands supported by the
completion script, then its options will be offered.
- If 'path' doesn't match a supported command and doesn't contain any
characters not allowed in Bash identifier names, then the completion
script does basically nothing and lets Bash to fall back to filename
completion.
- Otherwise, if 'path' contains such unallowed characters, then it
leads to a more or less ugly error in the middle of the command line.
The standard '/' directory separator is such a character, and it
happens to trigger one of the uglier errors:
$ git -C some/path sh.exe": declare: `_git_some/path':
not a valid identifier
error: invalid key: alias.some/path
Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line. Extend the relevant test with this
case and, while at it, with cases that needed similar treatment in the
past ('--git-dir', '-c', '--work-tree' and '--namespace').
Additionally, shut up standard error of the 'declare' commands looking
for the associated completion function and of the 'git config' query for
the aliased command, so if git learns a new option with a mandatory
argument in the future, then at least the command line will not be
utterly disrupted by those error messages.
Note, that this change merely fixes the breakage related to 'git -C
path', but the completion script will not take it into account as it
does '--git-dir path'.
I don't understand the "as it does" part. Do you mean that the
completion script does '--git-dir path', or that git does it?
When the user specifies '--git-dir path' on the command line the
completion script respects that (most of the time, I noticed a few
missing spots), i.e.
git --git-dir /somewhere/else/.git log
gives you all the refs from the specified repository, which is good.
However, that's not the case with '-C /somewhere/else', as it will lead to
the error mentioned in the commit message on current git, or will be
ignored with this patch.
In any case, "git -C path ..." is more like "cd path && git ...". That
is, completion should take it into account at least when determining
GIT_DIR (though -C does not specifiy the git-dir directly), and possibly
also for completion of untracked files. Otherwise, it's going by the
wrong repo (unless path is a subdir of cwd).
I agree, it should, but it doesn't. That will be the next step in some
future patches.
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: [PATCH v2 1/2] completion: Add sequencer function
Quoting Thomas Braun :
Signed-off-by: Thomas Braun
---
contrib/completion/git-completion.bash | 48
+++---
1 file changed, 33 insertions(+), 15 deletions(-)
I don't see the benefits of this change. This patch adds more than
twice as many lines as it removes, and patch 2/2 adds 8 new lines
although it could get away with only 5 without this function. To
offer sequencer options we currently go through a single if statement,
with this patch we'd go through a case statement, an if statement and
finally an &&.
Gábor
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bfc74e9..f6e5bf6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -851,15 +851,40 @@ __git_count_arguments ()
printf "%d" $c
}
+__git_complete_sequencer ()
+{
+ local dir="$(__gitdir)"
+
+ case "$1" in
+ am)
+ if [ -d "$dir"/rebase-apply ]; then
+ __gitcomp "--skip --continue --resolved --abort"
+ return 0
+ fi
+ ;;
+ cherry-pick)
+ if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
+ __gitcomp "--continue --quit --abort"
+ return 0
+ fi
+ ;;
+ rebase)
+ if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ];
then
+ __gitcomp "--continue --skip --abort"
+ return 0
+ fi
+ ;;
+ esac
+
+ return 1
+}
+
__git_whitespacelist="nowarn warn error error-all fix"
_git_am ()
{
- local dir="$(__gitdir)"
- if [ -d "$dir"/rebase-apply ]; then
- __gitcomp "--skip --continue --resolved --abort"
- return
- fi
+ __git_complete_sequencer "am" && return
+
case "$cur" in
--whitespace=*)
__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
@@ -1044,11 +1069,8 @@ _git_cherry ()
_git_cherry_pick ()
{
- local dir="$(__gitdir)"
- if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
- __gitcomp "--continue --quit --abort"
- return
- fi
+ __git_complete_sequencer "cherry-pick" && return
+
case "$cur" in
--*)
__gitcomp "--edit --no-commit --signoff --strategy= --mainline"
@@ -1666,11 +1688,7 @@ _git_push ()
_git_rebase ()
{
- local dir="$(__gitdir)"
- if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
- return
- fi
+ __git_complete_sequencer "rebase" && return
__git_complete_strategy && return
case "$cur" in
--whitespace=*)
--
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: [PATCH v2 1/2] completion: Add sequencer function
Quoting Junio C Hamano : SZEDER Gábor writes: I don't see the benefits of this change. This patch adds more than twice as many lines as it removes, and patch 2/2 adds 8 new lines although it could get away with only 5 without this function. To offer sequencer options we currently go through a single if statement, with this patch we'd go through a case statement, an if statement and finally an &&. Gábor Perhaps, especially given that I'd imagine we won't be adding 47 new commands that drive the sequencer in the near future ;-) I presume that you are OK with Thomas's original version, then? Yes, definitely. It's a shame all these sequencing commands have different sets of sequencer options. Perhaps something to clean up for, say, v3.0 :) 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
[PATCH] commit: cope with scissors lines in commit message
The diff and submodule shortlog appended to the commit message template
by 'git commit --verbose' are not stripped when the commit message
contains an indented scissors line.
When cleaning up a commit message with 'git commit --verbose' or
'--cleanup=scissors' the code is careful and triggers only on a pure
scissors line, i.e. a line containing nothing but a comment character, a
space, and the scissors cut. This is good, because people can embed
scissor lines in the commit message while using 'git commit --verbose',
and the text they write after their indented scissors line doesn't get
deleted.
While doing so, however, the cleanup function only looks at the first
line matching the scissors pattern and if it doesn't start at the
beginning of the line, then the function just returns without performing
any cleanup. This is bad, because a "real" scissors line added by 'git
commit --verbose' might follow, and in that case the diff and submodule
shortlog get included in the commit message.
Don't bail out if a scissors line doesn't start at the beginning of the
line, but keep looking for a non-indented scissors line to fix this.
Signed-off-by: SZEDER Gábor
---
t/t7502-commit.sh | 25 +
wt-status.c | 12
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 2e0d557243..77db3a31c3 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -239,6 +239,31 @@ EOF
'
+test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors
line in commit message)' '
+ echo >>negative &&
+ cat >text <8
+ # to be kept, too
+# >8
+to be removed
+# >8
+to be removed, too
+EOF
+
+ cat >expect <8
+ # to be kept, too
+EOF
+ git commit --cleanup=scissors -e -F text -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cleanup commit messages (strip option,-F)' '
echo >>negative &&
diff --git a/wt-status.c b/wt-status.c
index c56c78fb6f..e6d171a0cb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -822,13 +822,17 @@ conclude:
void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
{
- const char *p;
+ const char *p = buf->buf;
struct strbuf pattern = STRBUF_INIT;
strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line);
- p = strstr(buf->buf, pattern.buf);
- if (p && (p == buf->buf || p[-1] == '\n'))
- strbuf_setlen(buf, p - buf->buf);
+ while ((p = strstr(p, pattern.buf))) {
+ if (p == buf->buf || p[-1] == '\n') {
+ strbuf_setlen(buf, p - buf->buf);
+ break;
+ }
+ p++;
+ }
strbuf_release(&pattern);
}
--
2.4.2.423.gad3a03f
--
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
[PATCH] completion: teach 'scissors' mode to 'git commit --cleanup='
Signed-off-by: SZEDER Gábor
---
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 bfc74e9d57..a1098765f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1108,7 +1108,7 @@ _git_commit ()
case "$cur" in
--cleanup=*)
- __gitcomp "default strip verbatim whitespace
+ __gitcomp "default scissors strip verbatim whitespace
" "" "${cur##--cleanup=}"
return
;;
--
2.4.3.384.g605df7b
--
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: [PATCH] commit: cope with scissors lines in commit message
Quoting Junio C Hamano :
SZEDER Gábor writes:
The diff and submodule shortlog appended to the commit message template
by 'git commit --verbose' are not stripped when the commit message
contains an indented scissors line.
When cleaning up a commit message with 'git commit --verbose' or
'--cleanup=scissors' the code is careful and triggers only on a pure
scissors line, i.e. a line containing nothing but a comment character, a
space, and the scissors cut. This is good, because people can embed
scissor lines in the commit message while using 'git commit --verbose',
and the text they write after their indented scissors line doesn't get
deleted.
While doing so, however, the cleanup function only looks at the first
line matching the scissors pattern and if it doesn't start at the
beginning of the line, then the function just returns without performing
any cleanup. This is bad, because a "real" scissors line added by 'git
commit --verbose' might follow, and in that case the diff and submodule
shortlog get included in the commit message.
Yikes; this is not just "bad" but simply "wrong". Thanks for
noticing.
Great, the other day I scored a "Gaaah" from Peff, now a "Yikes" from
you... Doin' good! ;)
void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
{
- const char *p;
+ const char *p = buf->buf;
struct strbuf pattern = STRBUF_INIT;
strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line);
- p = strstr(buf->buf, pattern.buf);
- if (p && (p == buf->buf || p[-1] == '\n'))
- strbuf_setlen(buf, p - buf->buf);
+ while ((p = strstr(p, pattern.buf))) {
+ if (p == buf->buf || p[-1] == '\n') {
+ strbuf_setlen(buf, p - buf->buf);
+ break;
+ }
+ p++;
+ }
I however wonder if we should make strstr() do more work for us.
strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line);
if (starts_with(buf->buf, pattern.buf + 1))
strbuf_setlen(buf, 0);
else if ((p = strstr(buf->buf, pattern.buf)) != NULL)
strbuf_setlen(buf, p - buf->buf + 1);
strbuf_release(&pattern);
perhaps?
Though this has one more if statement, it doesn't add a loop and
eliminates the if (... || ...).
Good, will try to reroll in the evening.
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
[PATCH v2] commit: cope with scissors lines in commit message
The diff and submodule shortlog appended to the commit message template by 'git commit --verbose' are not stripped when the commit message contains an indented scissors line. When cleaning up a commit message with 'git commit --verbose' or '--cleanup=scissors' the code is careful and triggers only on a pure scissors line, i.e. a line containing nothing but a comment character, a space, and the scissors cut. This is good, because people can embed scissor lines in the commit message while using 'git commit --verbose', and the text they write after their indented scissors line doesn't get deleted. While doing so, however, the cleanup function only looks at the first line matching the scissors pattern and if it doesn't start at the beginning of the line, then the function just returns without performing any cleanup. This is wrong, because a "real" scissors line added by 'git commit --verbose' might follow, and in that case the diff and submodule shortlog get included in the commit message. Fix this by changing the scissors pattern to match only at the beginning of the line, yet be careful to catch scissors on the first line as well. Helped-by: Junio C Hamano Signed-off-by: SZEDER Gábor --- Changes besides incorporating Junio's suggestion and updating the commit message accordingly: * Instead of adding a new test, modify the existing one to check handling indented scissors lines. * Add a test to check scissors on the first line t/t7502-commit.sh | 24 +++- wt-status.c | 9 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 2e0d557243..b39e313ac2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' ' cat >text <8 +# to be kept, too # >8 to be removed +# >8 +to be removed, too +EOF + + cat >expect <8 +# to be kept, too EOF - echo "# to be kept" >expect && git commit --cleanup=scissors -e -F text -a && git cat-file -p HEAD |sed -e "1,/^\$/d">actual && test_cmp expect actual +' +test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' ' + + echo >>negative && + cat >text <8 +to be removed +EOF + git commit --cleanup=scissors -e -F text -a --allow-empty-message && + git cat-file -p HEAD |sed -e "1,/^\$/d">actual && + test_must_be_empty actual ' test_expect_success 'cleanup commit messages (strip option,-F)' ' diff --git a/wt-status.c b/wt-status.c index c56c78fb6f..eaed4fed32 100644 --- a/wt-status.c +++ b/wt-status.c @@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf) const char *p; struct strbuf pattern = STRBUF_INIT; - strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line); - p = strstr(buf->buf, pattern.buf); - if (p && (p == buf->buf || p[-1] == '\n')) - strbuf_setlen(buf, p - buf->buf); + strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line); + if (starts_with(buf->buf, pattern.buf + 1)) + strbuf_setlen(buf, 0); + else if ((p = strstr(buf->buf, pattern.buf))) + strbuf_setlen(buf, p - buf->buf + 1); strbuf_release(&pattern); } -- 2.4.3.384.g605df7b -- 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
[PATCH v3] commit: cope with scissors lines in commit message
The diff and submodule shortlog appended to the commit message template by 'git commit --verbose' are not stripped when the commit message contains an indented scissors line. When cleaning up a commit message with 'git commit --verbose' or '--cleanup=scissors' the code is careful and triggers only on a pure scissors line, i.e. a line containing nothing but a comment character, a space, and the scissors cut. This is good, because people can embed scissors lines in the commit message while using 'git commit --verbose', and the text they write after their indented scissors line doesn't get deleted. While doing so, however, the cleanup function only looks at the first line matching the scissors pattern and if it doesn't start at the beginning of the line, then the function just returns without performing any cleanup. This is wrong, because a "real" scissors line added by 'git commit --verbose' might follow, and in that case the diff and submodule shortlog get included in the commit message. Fix this by changing the scissors pattern to match only at the beginning of the line, yet be careful to catch scissors on the first line as well. Helped-by: Junio C Hamano Signed-off-by: SZEDER Gábor --- Fixed a typo in the commit message. t/t7502-commit.sh | 24 +++- wt-status.c | 9 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 2e0d557243..b39e313ac2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' ' cat >text <8 +# to be kept, too # >8 to be removed +# >8 +to be removed, too +EOF + + cat >expect <8 +# to be kept, too EOF - echo "# to be kept" >expect && git commit --cleanup=scissors -e -F text -a && git cat-file -p HEAD |sed -e "1,/^\$/d">actual && test_cmp expect actual +' +test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' ' + + echo >>negative && + cat >text <8 +to be removed +EOF + git commit --cleanup=scissors -e -F text -a --allow-empty-message && + git cat-file -p HEAD |sed -e "1,/^\$/d">actual && + test_must_be_empty actual ' test_expect_success 'cleanup commit messages (strip option,-F)' ' diff --git a/wt-status.c b/wt-status.c index c56c78fb6f..eaed4fed32 100644 --- a/wt-status.c +++ b/wt-status.c @@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf) const char *p; struct strbuf pattern = STRBUF_INIT; - strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line); - p = strstr(buf->buf, pattern.buf); - if (p && (p == buf->buf || p[-1] == '\n')) - strbuf_setlen(buf, p - buf->buf); + strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line); + if (starts_with(buf->buf, pattern.buf + 1)) + strbuf_setlen(buf, 0); + else if ((p = strstr(buf->buf, pattern.buf))) + strbuf_setlen(buf, p - buf->buf + 1); strbuf_release(&pattern); } -- 2.4.3.384.g605df7b -- 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
rebase -i might leave CHERRY_PICK_HEAD behind
Hi, When skipping an empty commit with 'git rebase --continue' a CHERRY_PICK_HEAD file might be left behind. What I did boils down to this: echo one >file git add file git commit -m first echo two >file git commit -a -m second echo three >file git commit -a -m third git rebase -i HEAD^^ # change todo list to edit the "second" commit echo three >file git commit -a --amend # this effectively makes the third commit an empty commit # and rebase will ask what to do: git rebase --continue The previous cherry-pick is now empty, possibly due to conflict resolution. If you wish to commit it anyway, use: git commit --allow-empty Otherwise, please use 'git reset' rebase in progress; onto 7335bbe7a5 You are currently rebasing branch 'master' on '7335bbe7a5'. nothing to commit, working directory clean Could not apply d19f82ac6d467247117fd734ed039b03ef923c86... third # I didn't want an empty commit, but didn't read that carefully, so I did: git rebase --continue Successfully rebased and updated refs/heads/master. # and was rewarded for my lack of attention with the following bash prompt: test/rebase-empty-continue (master|CHERRY-PICKING)$ # indeed: ls -l .git/CHERRY_PICK_HEAD -rw-r--r-- 1 szeder szeder 41 Jun 16 13:22 .git/CHERRY_PICK_HEAD On one hand, it's user error: it told me to run 'git reset' to achive what I want but I didn't. Note, however, how it told me about 'git reset': while 'git commit --allow-empty' is greatly emphasized by indentation and empty lines before and after, 'git reset' blends in quite well into the rebase progress. It was late, I was tired, and there was a questionable penalty on Copa América as well ;), so I simply didn't notice. On the other hand, 1. 'git rebase' claimed that "Successfully rebased...", yet it left cruft behind. I think it shouldn't. 2. 'git rebase --continue' didn't complain by the lack of prior 'git reset' and finished doing exacly what I expected from it to do (except leaving CHERRY_PICK_HEAD behind, of course). Perhaps it should complain, like it does when the worktree is dirty. Alternatively, it could just continue to DWIM, as it does already, but then it should remove CHERRY_PICK_HEAD as well. 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: [PATCH] git-prompt.sh: Document GIT_PS1_STATESEPARATOR
Quoting Joe Cridge :
The environment variable GIT_PS1_STATESEPARATOR can be used to set the
separator between the branch name and the state symbols in the prompt.
At present the variable is not mentioned in the inline documentation which
makes it difficult for the casual user to identify.
Thanks, makes sense.
Signed-off-by: Joe Cridge
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/completion/git-prompt.sh
b/contrib/completion/git-prompt.sh
index f18aedc..366f0bc 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -66,6 +66,10 @@
# git always compare HEAD to @{upstream}
# svn always compare HEAD to your SVN upstream
#
+# You can change the separator between the branch name and the above
+# state symbols by setting GIT_PS1_STATESEPARATOR. The default separator
+# is SP.
This is not a specification of a protocol or file or input/output
format, where we formally use SP and LF. Perhaps we could spell out
SP as a space here, for the sake of the "casual user"?
+#
# By default, __git_ps1 will compare HEAD to your SVN upstream if it can
# find one, or @{upstream} otherwise. Once you have set
# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
--
2.4.2
--
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: [PATCH 2/2] rebase -i: do not leave a CHERRY_PICK_HEAD file behind
Hi, Quoting Johannes Schindelin : When skipping commits whose changes were already applied via `git rebase --continue`, we need to clean up said file explicitly. The same is not true for `git rebase --skip` because that will execute `git reset --hard` as part of the "skip" handling in git-rebase.sh, even before git-rebase--interactive.sh is called. Signed-off-by: Johannes Schindelin Nice quick turnaround, thanks. So, with this change the 'git reset' won't be necessary at all, right? --- git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..16e0a82 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -849,7 +849,11 @@ continue) # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then - : Nothing to commit -- skip this + : Nothing to commit -- skip this commit "While at it", perhaps you could turn this into a proper comment with '#". Now that this if-branch starts to actually do something, there's no reason to continue (ab)using the null command. + + test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD || + rm "$GIT_DIR"/CHERRY_PICK_HEAD || + die "Could not remove CHERRY_PICK_HEAD" else if ! test -f "$author_script" then diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5d52775..241d4d1 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' -test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' +test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' git checkout -b commit-to-skip && for double in X 3 1 do -- 2.3.1.windows.1.9.g8c01ab4 -- 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: Git completion not using ls-remote to auto-complete during push
Quoting Robert Dailey > I do the following: > > $ git push origin :topic > > If I stop halfway through typing 'topic' and hit TAB, auto-completion > does not work if I do not have a local branch by that name (sometimes > I delete my local branch first, then I push to delete it remotely). I > thought that git completion code was supposed to use ls-remote to auto > complete refs used in push operations. Is this supposed to work? It's intentional. Running 'git ls-remote' with a far away remote can take ages, so instead we grab the refs on the remote from the locally stored refs under 'refs/remotes//'. See e832f5c096 (completion: avoid ls-remote in certain scenarios, 2013-05-28). The commit message mentions that you can "force" completion of remote refs via 'git ls-remote' by starting with the full refname, i.e. 'refs/', however, that seems to work only on the left hand side of the colon in the push refspec. 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: Git completion not using ls-remote to auto-complete during push
Quoting Robert Dailey : On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor wrote: Quoting Robert Dailey I do the following: $ git push origin :topic If I stop halfway through typing 'topic' and hit TAB, auto-completion does not work if I do not have a local branch by that name (sometimes I delete my local branch first, then I push to delete it remotely). I thought that git completion code was supposed to use ls-remote to auto complete refs used in push operations. Is this supposed to work? It's intentional. Running 'git ls-remote' with a far away remote can take ages, so instead we grab the refs on the remote from the locally stored refs under 'refs/remotes//'. See e832f5c096 (completion: avoid ls-remote in certain scenarios, 2013-05-28). The commit message mentions that you can "force" completion of remote refs via 'git ls-remote' by starting with the full refname, i.e. 'refs/', however, that seems to work only on the left hand side of the colon in the push refspec. Gábor If that's indeed the case, then completion should work. I have a 'refs/remotes/origin/topic'. Why will auto complete not work even though this exists? Do multiple remotes cause issues (in theory there is no reason why it should cause problems, since it should know I'm auto-completing a ref on 'origin')? The number of remotes doesn't matter. What matters is which side of the colon the ref to be completed is. You can complete git push origin refs/ and git fetch origin refs/ will even list you refs freshly queried via 'git ls-remote'. However, git push origin :refs/ git push origin branch:refs/ don't work, because there are no refs starting with the prefix ':refs/' or 'branch:refs/'. 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
[PATCH 1/2] bash prompt: test untracked files status indicator with untracked dirs
The next commit will tweak the way __git_ps1() decides whether to display the untracked files status indicator in the presence of untracked directories. Add tests to make sure it doesn't change current behavior, in particular that an empty untracked directory doesn't trigger the untracked files status indicator. Signed-off-by: SZEDER Gábor --- t/t9903-bash-prompt.sh | 25 + 1 file changed, 25 insertions(+) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 49d58e6726..6b68777b98 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -397,6 +397,31 @@ test_expect_success 'prompt - untracked files status indicator - untracked files test_cmp expected "$actual" ' +test_expect_success 'prompt - untracked files status indicator - empty untracked dir' ' + printf " (master)" >expected && + mkdir otherrepo/untracked-dir && + test_when_finished "rm -rf otherrepo/untracked-dir" && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + cd otherrepo && + __git_ps1 >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - non-empty untracked dir' ' + printf " (master %%)" >expected && + mkdir otherrepo/untracked-dir && + test_when_finished "rm -rf otherrepo/untracked-dir" && + >otherrepo/untracked-dir/untracked-file && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + cd otherrepo && + __git_ps1 >"$actual" + ) && + test_cmp expected "$actual" +' + test_expect_success 'prompt - untracked files status indicator - untracked files outside cwd' ' printf " (master %%)" >expected && ( -- 2.5.0.rc2.15.gd82f7f6 -- 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
[PATCH 2/2] bash prompt: faster untracked status indicator with untracked directories
If the untracked status indicator is enabled, __git_ps1() looks for
untracked files by running 'git ls-files'. This can be perceptibly slow
in case of an untracked directory containing lot of files, because it
lists all files found in the untracked directory only to be redirected
into /dev/null right away (this is the actual command run by __git_ps1()):
$ ls untracked-dir/ |wc -l
10
$ time git ls-files --others --exclude-standard --error-unmatch \
-- ':/*' >/dev/null 2>/dev/null
real 0m0.955s
user 0m0.936s
sys 0m0.016s
Eliminate this delay by additionally passing the '--directory
--no-empty-directory' options to 'git ls-files' to show only the name of
non-empty untracked directories instead of all their content:
$ time git ls-files --others --exclude-standard --directory \
--no-empty-directory --error-unmatch -- ':/*' >/dev/null 2>/dev/null
real 0m0.010s
user 0m0.008s
sys 0m0.000s
This follows suit of ea95c7b8f5 (completion: improve untracked directory
filtering for filename completion, 2013-09-18).
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-prompt.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 366f0bc1e9..07b52bedf1 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -491,7 +491,7 @@ __git_ps1 ()
if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ] &&
[ "$(git config --bool bash.showUntrackedFiles)" != "false"
] &&
- git ls-files --others --exclude-standard --error-unmatch --
':/*' >/dev/null 2>/dev/null
+ git ls-files --others --exclude-standard --directory
--no-empty-directory --error-unmatch -- ':/*' >/dev/null 2>/dev/null
then
u="%${ZSH_VERSION+%}"
fi
--
2.5.0.rc2.15.gd82f7f6
--
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: [PATCHv2 0/7] Fix and generalize version sort reordering
On Wed, Dec 14, 2016 at 6:08 PM, Jeff King wrote:
> On Thu, Dec 08, 2016 at 03:23:54PM +0100, SZEDER Gábor wrote:
>
>> > With my patches it looks like this:
>> >
>> > $ git -c versionsort.prereleasesuffix=-pre \
>> > -c versionsort.prereleasesuffix=-prerelease \
>> > tag -l --sort=version:refname
>> > v1.0.0-prerelease1
>> > v1.0.0-pre1
>> > v1.0.0-pre2
>> > v1.0.0
>>
>> And when there happen to be more than one matching suffixes starting
>> at the same earliest position, then we should pick the longest of
>> them. The new patch 6/7 implements this behavior.
>
> The whole approach taken by the suffix code (before your patches) leaves
> me with the nagging feeling that the comparison is not always going to
> be transitive (i.e., that "a < b && b < c" does not always imply "a <
> c"), which is going to cause nonsensical sorting results.
>
> And that may be part of the issue your 6/7 fixes.
>
> I spent some time playing with this the other day, though, and couldn't
> come up with a specific example that fails the condition above.
>
> It just seems like the whole thing would conceptually easier if we
> pre-parsed the versions into a sequence of elements, then the comparison
> between any two elements would just walk that sequence. The benefit
> there is that you can implement whatever rules you like for the parsing
> (like "prefer longer suffixes to shorter"), but you know the comparison
> will always be consistent.
I considered parsing tagnames into prefix, version number and suffix,
and then work from that, but decided against it.
versioncmp() is taken from glibc, so I assume that it's thoroughly
tested, even in corner cases (e.g. multiple leading zeros).
Furthermore, I think it's a good thing that by default (i.e. without
suffix reordering) our version sort orders the same way as glibc's
version sort does. Introducing a different algorithm would risk bugs
in the more subtle cases.
Then there are all the weird release suffixes out there, and I didn't
want to decide on a policy for splitting them sanely; don't know
whether there exist any universal rules for this splitting at
all. E.g. one of the packages here has the following version (let's
ignore the fact that because of the '~' this is an invalid refname in
git):
1.1.0~rc1-2ubuntu7-1linuxmint1
Now, it's clear that the version number is "1.1.0", and the user
should configure the suffix "~rc" for prerelease reordering. But what
about the rest? How should we split it "into a sequence of elements",
is it { "1.1.0", "~rc1", "-2ubuntu7", "-1linuxmint1" } or { "1.1.0",
"~rc1-2", "ubuntu7-1", "linuxmint1" }?
What if there is a hard-working developer who is involved in a lot of
Debian derivatives (and derivatives of derivatives...), and, for
whatever reason, wants to put derivative-specific versions in a
particular order? With my series, or conceptually even with master if
it weren't buggy, it's possible to specify the order of suffixes of
suffixes, and that dev could do this:
$ git -c versionsort.suffix=-rc
-c versionsort.suffix=linuxmint
-c versionsort.suffix=YADoaDD
tag -l --sort=version:refname
'1.1.0*'
1.1.0-rc1-2ubuntu7-1linuxmint1
1.1.0-rc1-2ubuntu7-1YADoaDD2
1.1.0
1.1.0-2ubuntu7-1linuxmint1
1.1.0-2ubuntu7-1YADoaDD2
and would get Linux Mint-specific tags before "Yet Another Derivative
of a Debian Derivative"-specific ones. Not sure whether this is
relevant in practice, but I think it's a nice property nonetheless.
(Btw, just for fun, I also found a package version
2.0.0~beta2+isreally1.8.6-0ubuntu1. "isreally". Oh yeah :)
> It would also be more efficient, I think (it seems like the sort is
> O(nr_tags * lg(nr_tags) * nr_suffixes) due to parsing suffixes in the
> comparator). Though that probably doesn't matter much in practice.
I don't think there will be more than only a few configured suffixes
in any repository. However, if you consider O as "number of
starts_with() invocations", then there is an additional suffix_length
factor. But then again, these suffixes tend to be short.
> I dunno. I think maybe your 6/7 has converged on an equivalent behavior.
> And I am certainly not volunteering to re-write it, so if what you have
> works, I'm not opposed to it.
>
> -Peff
Re: [PATCH] git-prompt.sh: add submodule indicator
I'm not well-versed in submodules, so I won't comment on whether this
is the right way to determine that a repository is a submodule, but I
was surprised to see how much you have to work to find this out.
My comments will mostly focus on how to eliminate or at least limit
the scope of subshells and command executions, because fork()s and
exec()s are rather expensive on Windows.
On Fri, Jan 20, 2017 at 1:07 AM, Benjamin Fuchs wrote:
> I expirienced that working with submodules can be confusing. This indicator
> will make you notice very easy when you switch into a submodule.
> The new prompt will look like this: (sub:master)
> Adding a new optional env variable for the new feature.
>
> Signed-off-by: Benjamin Fuchs
> ---
> contrib/completion/git-prompt.sh | 37 -
> 1 file changed, 36 insertions(+), 1 deletion(-)
Tests?
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index 97eacd7..4c82e7f 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -93,6 +93,10 @@
> # directory is set up to be ignored by git, then set
> # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
> # repository level by setting bash.hideIfPwdIgnored to "false".
> +#
> +# If you would like __git_ps1 to indicate that you are in a submodule,
> +# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> +# the branch name.
>
> # check whether printf supports -v
> __git_printf_supports_v=
> @@ -284,6 +288,32 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
> }
>
> +# __git_is_submodule
> +# Based on:
> +#
> http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> +__git_is_submodule ()
Use the '__git_ps1' prefix in the function name, like the other
functions.
> +{
> + local git_dir parent_git module_name path
> + # Find the root of this git repo, then check if its parent dir is
> also a repo
> + git_dir="$(git rev-parse --show-toplevel)"
1. This is a somewhat misleading variable name, because git_dir (with
or without underscore or dash) refers to the path to the .git
directory of a repository through the codebase, documentation and
CLI options, not the top-level directory of the worktree.
2. In a bare repository or in the .git directory of a "regular"
repository '--show-toplevel' doesn't output anything, leading to
an empty $module_name below, which ultimately results in no
submodule indicator.
As fas as behavior is concerned, this is good in the bare
repository case, because as I understand it there is no such thing
as a bare submodule. I'm not sure whether the submodule indicator
should be displayed in the ".git dir of a submodule" case. I
leave it up to you and Stefan to discuss.
However, the info about whether we are in a bare repository or
.git dir is already availabe in certain variables, so we know
upfront when the current repository can't be a submodule. In
those cases this function should not be called only to spend
several subshells and command executions to figure out what we
already knew anyway.
3. The path to the .git directory of the current repository
is already available in the (not particularly descriptively named)
$g variable from __git_ps1. Perhaps you could use that variable
instead, thus avoiding a subshell and executing a git command
here.
> + module_name="$(basename "$git_dir")"
This is executed even when there is no repository in the parent
directories, but it's only needed when there _is_ a repo up there.
Please move it into the conditional below, to avoid a subshell and
command execution in the main code path.
Since this $git_dir comes directly from our own 'git rev-parse' do we
have to be prepared for a Windows-style path there? If it were always
a UNIX-style path, then we could strip all directories with shell
parameter expansion, eliminating both the subshell and the basename
execution.
> + parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2>
> /dev/null)"
Style nit: no space after redirection, i.e. it should be '2>/dev/null'.
More importantly, I don't think you really need this variable:
1. The existence of a parent git repository can be determined from
'git rev-parse's exit code alone.
2. When you run 'git submodule' below, you don't have to cd to the
_top-level_ directory of the parent repository's worktree. You
just have to cd to _any_ directory in the parent, and you can do
that with 'git -C "$git_dir/.." submodule ...', without knowing
the parent's top-level directory.
This means that you don't need 'git rev-parse's output, thus there is
no need for the command substitution. Yet another subshell spared! :)
However, then you have to be careful with changing directories, and
should write it as 'git -C "$git_dir/.." rev-parse ...'.
> +
Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string
On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor wrote:
> ref-filter's parse_ref_filter_atom() function parses an atom between
> the start and end pointers it gets as arguments. This is fine for two
> of its callers, which process '%(atom)' format specifiers and the end
> pointer comes directly from strchr() looking for the closing ')'.
> However, it's not quite so straightforward for its other two callers,
> which process sort specifiers given as plain nul-terminated strings.
> Especially not for ref_default_sorting(), which has the default
> hard-coded as a string literal, but can't use it directly, because a
> pointer to the end of that string literal is needed as well.
> The next patch will add yet another caller using a string literal.
>
> Add a helper function around parse_ref_filter_atom() to parse an atom
> up to the terminating nul, and call this helper in those two callers.
>
> Signed-off-by: SZEDER Gábor
> ---
> ref-filter.c | 8 ++--
> ref-filter.h | 4
> 2 files changed, 6 insertions(+), 6 deletions(-)
Ping?
It looks like that this little two piece cleanup series fell on the floor.
> diff --git a/ref-filter.c b/ref-filter.c
> index dfadf577c..3c6fd4ba7 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1658,19 +1658,16 @@ void show_ref_array_item(struct ref_array_item *info,
> const char *format, int qu
> /* If no sorting option is given, use refname to sort as default */
> struct ref_sorting *ref_default_sorting(void)
> {
> - static const char cstr_name[] = "refname";
> -
> struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
>
> sorting->next = NULL;
> - sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name +
> strlen(cstr_name));
> + sorting->atom = parse_ref_filter_atom_from_string("refname");
> return sorting;
> }
>
> void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)
> {
> struct ref_sorting *s;
> - int len;
>
> s = xcalloc(1, sizeof(*s));
> s->next = *sorting_tail;
> @@ -1683,8 +1680,7 @@ void parse_sorting_string(const char *arg, struct
> ref_sorting **sorting_tail)
> if (skip_prefix(arg, "version:", &arg) ||
> skip_prefix(arg, "v:", &arg))
> s->version = 1;
> - len = strlen(arg);
> - s->atom = parse_ref_filter_atom(arg, arg+len);
> + s->atom = parse_ref_filter_atom_from_string(arg);
> }
>
> int parse_opt_ref_sorting(const struct option *opt, const char *arg, int
> unset)
> diff --git a/ref-filter.h b/ref-filter.h
> index 49466a17d..1250537cf 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -94,6 +94,10 @@ int filter_refs(struct ref_array *array, struct ref_filter
> *filter, unsigned int
> void ref_array_clear(struct ref_array *array);
> /* Parse format string and sort specifiers */
> int parse_ref_filter_atom(const char *atom, const char *ep);
> +static inline int parse_ref_filter_atom_from_string(const char *atom)
> +{
> + return parse_ref_filter_atom(atom, atom+strlen(atom));
> +}
> /* Used to verify if the given format is correct and to parse out the used
> atoms */
> int verify_ref_format(const char *format);
> /* Sort the given ref_array as per the ref_sorting provided */
> --
> 2.11.0.78.g5a2d011
>
Re: [PATCH 2/4] git-prompt.sh: rework of submodule indicator
> Rework of the first patch. The prompt now will look like this:
> (+name:master). I tried to considere all suggestions.
> Tests still missing.
>
> Signed-off-by: Benjamin Fuchs
> ---
> contrib/completion/git-prompt.sh | 49
>
> 1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index 4c82e7f..c44b9a2 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -95,8 +95,10 @@
> # repository level by setting bash.hideIfPwdIgnored to "false".
> #
> # If you would like __git_ps1 to indicate that you are in a submodule,
> -# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> -# the branch name.
> +# set GIT_PS1_SHOWSUBMODULE to a nonempty value. In this case the name
> +# of the submodule will be prepended to the branch name (e.g. module:master).
> +# The name will be prepended by "+" if the currently checked out submodule
> +# commit does not match the SHA-1 found in the index of the containing
> repository.
>
> # check whether printf supports -v
> __git_printf_supports_v=
> @@ -288,30 +290,27 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
> }
>
> -# __git_is_submodule
> -# Based on:
> -#
> http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> -__git_is_submodule ()
> -{
> - local git_dir parent_git module_name path
> - # Find the root of this git repo, then check if its parent dir is also
> a repo
> - git_dir="$(git rev-parse --show-toplevel)"
> - module_name="$(basename "$git_dir")"
> - parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2>
> /dev/null)"
> - if [[ -n $parent_git ]]; then
> - # List all the submodule paths for the parent repo
> - while read path
> - do
> - if [[ "$path" != "$module_name" ]]; then continue; fi
> - if [[ -d "$git_dir/../$path" ]];then return 0; fi
> - done < <(cd $parent_git && git submodule --quiet foreach 'echo
> $path' 2> /dev/null)
> -fi
> -return 1
> -}
> -
> +# __git_ps1_submodule
> +#
> +# $1 - git_dir path
> +#
> +# Returns the name of the submodule if we are currently inside one. The name
> +# will be prepended by "+" if the currently checked out submodule commit does
> +# not match the SHA-1 found in the index of the containing repository.
> +# NOTE: git_dir looks like in a ...
> +# - submodule: "GIT_PARENT/.git/modules/PATH_TO_SUBMODULE"
> +# - parent: "GIT_PARENT/.git" (exception "." in .git)
> __git_ps1_submodule ()
> {
> - __git_is_submodule && printf "sub:"
> + local git_dir="$1"
> + local submodule_name="$(basename "$git_dir")"
> + if [ "$submodule_name" != ".git" ] && [ "$submodule_name" != "." ]; then
> + local parent_top="${git_dir%.git*}"
> + local submodule_top="${git_dir#*modules}"
> + local status=""
> + git diff -C "$parent_top" --no-ext-diff
> --ignore-submodules=dirty --quiet -- "$submodule_top" 2>/dev/null ||
> status="+"
This 'git diff' has to read the index of the parent repository, right?
That can be potentially very expensive, if the parent repository, and
thus its index, is big.
You might want to provide finer granularity controls and separate the
"$PWD is in a submodule" indicator from the "submodule commit doesn't
match" indicator. Even if someone is in general interested in the
former, he might have some huge repositories where he would prefer to
disable the latter, because executing 'git diff' would make the prompt
lag.
I'm not sure we need brand new control knobs, though. Perhaps we can
get away by checking the env and config variables used for the dirty
index and worktree status indicator, after all they, too, are about
whether to run 'git diff' for the prompt in a repository or not.
> + printf "$status$submodule_name:"
> + fi
> }
>
> # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> @@ -545,7 +544,7 @@ __git_ps1 ()
>
> local sub=""
> if [ -n "${GIT_PS1_SHOWSUBMODULE}" ]; then
> - sub="$(__git_ps1_submodule)"
> + sub="$(__git_ps1_submodule $g)"
In Bash, and in shells in general, the visibility of variables works
differently than in "regular" programming languages:
- Any variable existing in a caller, even the ones declared as
'local' in the caller, are visible in all callees.
This means you don't have to pass $g as parameter to
__git_ps1_submodule(), because you can access it inside that
function directly. This has the benefit that there is one less
place where you can forget to quote a path variable :)
- If the callee modifies any variable that isn't declared as local
in the callee, then the caller will see the modified value of that
variable, unless the callee was invoked in a subsh
Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs wrote: > Signed-off-by: Benjamin Fuchs > --- > t/t9903-bash-prompt.sh | 43 +++ > 1 file changed, 43 insertions(+) > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index 97c9b32..4dce366 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' ' > git commit -m "yet another b2" file && > mkdir ignored_dir && > echo "ignored_dir/" >>.gitignore && > + git checkout -b submodule && > + git submodule add ./. sub && ./. ? > + git -C sub checkout master && > + git add sub && > + git commit -m submodule && > git checkout master > ' > > @@ -755,4 +760,42 @@ test_expect_success 'prompt - hide if pwd ignored - > inside gitdir (stderr)' ' > test_cmp expected "$actual" > ' > > +test_expect_success 'prompt - submodule indicator' ' > + printf " (sub:master)" >expected && > + git checkout submodule && > + test_when_finished "git checkout master" && > + ( > + cd sub && > + GIT_PS1_SHOWSUBMODULE=1 && > + __git_ps1 >"$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - submodule indicator - verify false' ' I was puzzled by the "verify false" here. You mean "disabled", right? > + printf " (master)" >expected && > + git checkout submodule && > + test_when_finished "git checkout master" && > + ( > + cd sub && > + GIT_PS1_SHOWSUBMODULE= && > + __git_ps1 >"$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - submodule indicator - dirty status indicator' ' > + printf " (+sub:b1)" >expected && > + git checkout submodule && > + git -C sub checkout b1 && > + test_when_finished "git checkout master" && > + ( > + cd sub && > + GIT_PS1_SHOWSUBMODULE=1 && > + __git_ps1 >"$actual" > + ) && > + test_cmp expected "$actual" > +' > + > + > test_done > -- > 2.7.4 >
[PATCH] .mailmap: update Gábor Szeder's email address
Signed-off-by: SZEDER Gábor --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 9c87a3840..ab59b2fac 100644 --- a/.mailmap +++ b/.mailmap @@ -225,6 +225,7 @@ Steven Walter Steven Walter Sven Verdoolaege Sven Verdoolaege +SZEDER Gábor Tay Ray Chuan Ted Percival Theodore Ts'o -- 2.11.0.555.g967c1bcb3
Re: [PATCH v2 7/7] completion: recognize more long-options
On Fri, Jan 27, 2017 at 10:17 PM, wrote: > From: Cornelius Weig > > Recognize several new long-options for bash completion in the following > commands: Adding more long options that git commands learn along the way is always an improvement. However, seeing "_several_ new long options" (or "some long options" in one of the other patches in the series) makes the reader wonder: are these the only new long options missing or are there more? If there are more, why only these are added? If there aren't any more missing long options left, then please say so, e.g. "Add all missing long options, except the potentially desctructive ones, for the following commands: " > - apply: --recount --directory= > - archive: --output > - branch: --column --no-column --sort= --points-at > - clone: --no-single-branch --shallow-submodules > - commit: --patch --short --date --allow-empty > - describe: --first-parent > - fetch, pull: --unshallow --update-shallow > - fsck: --name-objects > - grep: --break --heading --show-function --function-context > --untracked --no-index > - mergetool: --prompt --no-prompt > - reset: --keep > - revert: --strategy= --strategy-option= > - rm: --force '--force' is a potentially destructive option, too. > - shortlog: --email > - tag: --merged --no-merged --create-reflog > > Signed-off-by: Cornelius Weig > Helped-by: Johannes Sixt > --- > contrib/completion/git-completion.bash | 31 +-- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index 0e09519..933bb6e 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -936,6 +936,7 @@ _git_apply () > --apply --no-add --exclude= > --ignore-whitespace --ignore-space-change > --whitespace= --inaccurate-eof --verbose > + --recount --directory= > " > return > esac > @@ -974,7 +975,7 @@ _git_archive () > --*) > __gitcomp " > --format= --list --verbose > - --prefix= --remote= --exec= > + --prefix= --remote= --exec= --output > " > return > ;; > @@ -1029,6 +1030,7 @@ _git_branch () > --track --no-track --contains --merged --no-merged > --set-upstream-to= --edit-description --list > --unset-upstream --delete --move --remotes > + --column --no-column --sort= --points-at > " > ;; > *) > @@ -1142,6 +1144,8 @@ _git_clone () > --single-branch > --branch > --recurse-submodules > + --no-single-branch > + --shallow-submodules > " > return > ;; > @@ -1183,6 +1187,7 @@ _git_commit () > --reset-author --file= --message= --template= > --cleanup= --untracked-files --untracked-files= > --verbose --quiet --fixup= --squash= > + --patch --short --date --allow-empty > " > return > esac > @@ -1201,7 +1206,7 @@ _git_describe () > --*) > __gitcomp " > --all --tags --contains --abbrev= --candidates= > - --exact-match --debug --long --match --always > + --exact-match --debug --long --match --always > --first-parent > " > return > esac > @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" > __git_fetch_options=" > --quiet --verbose --append --upload-pack --force --keep --depth= > --tags --no-tags --all --prune --dry-run --recurse-submodules= > + --unshallow --update-shallow > " > > _git_fetch () > @@ -1333,7 +1339,7 @@ _git_fsck () > --*) > __gitcomp " > --tags --root --unreachable --cache --no-reflogs > --full > - --strict --verbose --lost-found > + --strict --verbose --lost-found --name-objects > " > return > ;; > @@ -1377,6 +1383,8 @@ _git_grep () > --max-depth > --count > --and --or --not --all-match > + --break --heading --show-function --function-context > + --untracked --no-index > " > return > ;; > @@ -1576,7 +1584,7 @@ _git_mergetool
Re: [PATCH 3/7] completion: improve bash completion for git-add
> Add some long-options for git-add and improve path completion when the > --update flag is given. Please tell us in the commit message _how_ this improves path completion. > --- > contrib/completion/git-completion.bash | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index 8329f09..652c7e2 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -947,13 +947,17 @@ _git_add () > --*) > __gitcomp " > --interactive --refresh --patch --update --dry-run > - --ignore-errors --intent-to-add > + --ignore-errors --intent-to-add --force --edit --chmod= I almost started complaining that '--force' should be used with care, but then realized that for 'git add' it only means adding ignored files. So in this particular case '--force' is not destructive and we can offer it among other long options. Good. > " > return > esac > > - # XXX should we check for --update and --all options ? > - __git_complete_index_file "--others --modified --directory > --no-empty-directory" > + local complete_opt="--others --modified --directory > --no-empty-directory" > + if test -n "$(__git_find_on_cmdline "-u --update")" > + then > + complete_opt="--modified" > + fi > + __git_complete_index_file "$complete_opt" > } > > _git_archive () > -- > 2.10.2
Re: [PATCH 2/7] completion: add subcommand completion for rerere
> Managing recorded resolutions requires command-line usage of git-rerere.
> Added subcommand completion for rerere and path completion for its
> subcommand forget.
> ---
> contrib/completion/git-completion.bash | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index c54a557..8329f09 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2401,6 +2401,17 @@ _git_replace ()
> __gitcomp_nl "$(__git_refs)"
> }
>
> +_git_rerere ()
> +{
> + local subcommands="clear forget diff remaining status gc"
> + local subcommand="$(__git_find_on_cmdline "$subcommands")"
> + if test -z "$subcommand"
> + then
> + __gitcomp "$subcommands"
> + return
> + fi
> +}
> +
> _git_reset ()
> {
> __git_has_doubledash && return
> --
> 2.10.2
You didn't add 'rerere' to the list of porcelain commands, i.e. it
won't be listed after 'git '. I'm not saying it should be
listed there, because I can't decide whether 'rerere' is considered
porcelain or plumbing... Just wanted to make sure that this omission
was intentional.
Re: [PATCH 6/7] completion: teach remote subcommands option completion
> Git-remote needs to complete remote names, its subcommands, and options
> thereof. In addition to the existing subcommand and remote name
> completion, do also complete the options
>
> - add: --track --master --fetch --tags --no-tags --mirror=
Oh, '--track' and '--master' are not even in the manpage or in 'git
remote -h', I could only find them after looking at the source code...
Good eyes :)
> - set-url: --push --add --delete
> - get-url: --push --all
> - prune: --dry-run
> ---
> contrib/completion/git-completion.bash | 36
> +++---
> 1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index e76cbd7..0e09519 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2384,24 +2384,46 @@ _git_config ()
>
> _git_remote ()
> {
> - local subcommands="add rename remove set-head set-branches set-url show
> prune update"
> + local subcommands="
> + add rename remove set-head set-branches
> + get-url set-url show prune update
> + "
> local subcommand="$(__git_find_on_cmdline "$subcommands")"
> if [ -z "$subcommand" ]; then
> - __gitcomp "$subcommands"
> + case "$cur" in
> + --*)
> + __gitcomp "--verbose"
> + ;;
> + *)
> + __gitcomp "$subcommands"
> + ;;
> + esac
> return
> fi
>
> - case "$subcommand" in
> - rename|remove|set-url|show|prune)
> - __gitcomp_nl "$(__git_remotes)"
> + case "$subcommand,$cur" in
> + add,--*)
> + __gitcomp "--track --master --fetch --tags --no-tags --mirror="
> ;;
> - set-head|set-branches)
> + add,*)
> + ;;
> + set-head,*|set-branches,*)
The 'set-head' subcommand has '--auto' and '--delete' options, and
'set-branches' has '--add'.
> __git_complete_remote_or_refspec
> ;;
> - update)
> + update,*)
The 'update' subcommand has a '--prune' option.
Otherwise it all looks good.
> __gitcomp "$(__git_get_config_variables "remotes")"
> ;;
> + set-url,--*)
> + __gitcomp "--push --add --delete"
> + ;;
> + get-url,--*)
> + __gitcomp "--push --all"
> + ;;
> + prune,--*)
> + __gitcomp "--dry-run"
> + ;;
> *)
> + __gitcomp_nl "$(__git_remotes)"
> ;;
> esac
> }
> --
> 2.10.2
Re: [PATCH 4/7] completion: teach ls-remote to complete options
> ls-remote needs to complete remote names and its own options.
And refnames, too.
> In
> addition to the existing remote name completions, do also complete
> the options --heads, --tags, --refs, and --get-url.
Why only these four options and not the other four?
There are eight options in total here, so there is really no chance
for cluttered terminals, and all eight are mentioned in the synopsis.
> ---
> contrib/completion/git-completion.bash | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 652c7e2..36fe439 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,6 +1449,12 @@ _git_ls_files ()
>
> _git_ls_remote ()
> {
> + case "$cur" in
> + --*)
> + __gitcomp "--heads --tags --refs --get-url"
> + return
> + ;;
> + esac
> __gitcomp_nl "$(__git_remotes)"
> }
>
> --
> 2.10.2
Re: [PATCH v2 7/7] completion: recognize more long-options
On Wed, Feb 1, 2017 at 5:49 PM, Cornelius Weig wrote: > Hi Gabor, > > thanks for taking a look at these commits. > > On 01/31/2017 11:17 PM, SZEDER Gábor wrote: >> On Fri, Jan 27, 2017 at 10:17 PM, wrote: >>> From: Cornelius Weig >>> >>> Recognize several new long-options for bash completion in the following >>> commands: >> >> Adding more long options that git commands learn along the way is >> always an improvement. However, seeing "_several_ new long options" >> (or "some long options" in one of the other patches in the series) >> makes the reader wonder: are these the only new long options missing >> or are there more? If there are more, why only these are added? If >> there aren't any more missing long options left, then please say so, >> e.g. "Add all missing long options, except the potentially >> desctructive ones, for the following commands: " > > Personally, I agree with you that >> Adding more long options that git commands learn along the way is >> always an improvement. > However, people may start complaining that their terminal becomes too > cluttered when doing a double-Tab. In my cover letter, I go to length > about this. My assumption was that all options that are mentioned in the > introduction of the command man-page should be important enough to have > them in the completion list. But that doesn't mean that the ones not mentioned in the synopsis section are not worth completing. The list of options listed by the completion script for several of these commands fits on a single line. The command with the most options among these is 'git pull', and even its options don't fill more than half of a 80x25 screen. I see no danger of people coming complaining. > I'll change my commit message accordingly. > >>> - rm: --force >> >> '--force' is a potentially destructive option, too. > > Thanks for spotting this. > > Btw, I haven't found that non-destructive options should not be eligible > for completion. To avoid confusion about this in the future, I suggest > to also change the documentation: > > index 933bb6e..96f1c7f 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -13,7 +13,7 @@ > #*) git email aliases for git-send-email > #*) tree paths within 'ref:path/to/file' expressions > #*) file paths within current working directory and index > -#*) common --long-options > +#*) common non-destructive --long-options I don't mind such a change, but I don't think that list was ever meant to be comprehensive or decisive. It is definitely not the former, as it's missing several things that the completion script does support. OTOH, it talks about .git/remotes, which has been considered legacy for quite some years (though it's right, because the completion script still supports it). > I take it you have also looked at the code itself? Then I would gladly > mention you as reviewer in my sign-off. Yeah, most of the changes was rather straightforward, except the completion of 'git remote's subcommands' options, but that looks good, too. Gábor
[PATCHv2 16/21] completion: respect 'git -C '
'git -C ' option(s) on the command line should be taken into
account during completion, because
- like '--git-dir=', it can lead us to a different repository,
- a few git commands executed in the completion script do care about
in which directory they are executed, and
- the command for which we are providing completion might care about
in which directory it will be executed.
However, unlike '--git-dir=', the '-C ' option can be
specified multiple times and their effect is cumulative, so we can't
just store a single '' in a variable. Nor can we simply
concatenate a path from '-C -C ...', because e.g. (in
an arguably pathological corner case) a relative path might be
followed by an absolute path.
Instead, store all '-C ' options word by word in the
$__git_C_args array in the main git completion function, and pass this
array, if present, to 'git rev-parse --absolute-git-dir' when
discovering the repository in __gitdir(), and let it take care of
multiple options, relative paths, absolute paths and everything.
Also pass all '-C options via the $__git_C_args array to those
git executions which require a worktree and for which it matters from
which directory they are executed from. There are only three such
cases:
- 'git diff-index' and 'git ls-files' in __git_ls_files_helper()
used for git-aware filename completion, and
- the 'git ls-tree' used for completing the 'ref:path' notation.
The other git commands executed in the completion script don't need
these '-C ' options, because __gitdir() already took those
options into account. It would not hurt them, either, but let's not
induce unnecessary code churn.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 19 ++--
t/t9902-completion.sh | 87 ++
2 files changed, 101 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ac5eb9ced..e003afd54 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -39,7 +39,11 @@ esac
__gitdir ()
{
if [ -z "${1-}" ]; then
- if [ -n "${__git_dir-}" ]; then
+ if [ -n "${__git_C_args-}" ]; then
+ git "${__git_C_args[@]}" \
+ ${__git_dir:+--git-dir="$__git_dir"} \
+ rev-parse --absolute-git-dir 2>/dev/null
+ elif [ -n "${__git_dir-}" ]; then
test -d "$__git_dir" || return 1
echo "$__git_dir"
elif [ -n "${GIT_DIR-}" ]; then
@@ -286,10 +290,10 @@ __git_ls_files_helper ()
local dir="$(__gitdir)"
if [ "$2" == "--committable" ]; then
- git --git-dir="$dir" -C "$1" diff-index --name-only --relative
HEAD
+ git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C
"$1" diff-index --name-only --relative HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
- git --git-dir="$dir" -C "$1" ls-files --exclude-standard $2
+ git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C
"$1" ls-files --exclude-standard $2
fi 2>/dev/null
}
@@ -519,7 +523,7 @@ __git_complete_revlist_file ()
*) pfx="$ref:$pfx" ;;
esac
- __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls"
2>/dev/null \
+ __gitcomp_nl "$(git ${__git_C_args:+"${__git_C_args[@]}"}
--git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
| sed '/^100... blob /{
s,^.*,,
s,$, ,
@@ -2792,6 +2796,7 @@ _git_worktree ()
__git_main ()
{
local i c=1 command __git_dir
+ local __git_C_args C_args_count=0
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -2800,7 +2805,11 @@ __git_main ()
--git-dir) ((c++)) ; __git_dir="${words[c]}" ;;
--bare) __git_dir="." ;;
--help) command="help"; break ;;
- -c|-C|--work-tree|--namespace) ((c++)) ;;
+ -c|--work-tree|--namespace) ((c++)) ;;
+ -C) __git_C_args[C_args_count++]=-C
+ ((c++))
+ __git_C_args[C_args_count++]="${words[c]}"
+
[PATCHv2 11/21] completion: don't list 'HEAD' when trying refs completion outside of a repo
When refs completion is attempted while not in a git repository, the
completion script offers 'HEAD' erroneously.
Check early in __git_refs() that there is either a repository or a
remote to work on, and return early if neither is given.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 8 ++--
t/t9902-completion.sh | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index fd0ba1f3b..6b489b7c8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -346,7 +346,11 @@ __git_refs ()
local list_refs_from=path remote="${1-}"
local format refs pfx
- if [ -n "$remote" ]; then
+ if [ -z "$remote" ]; then
+ if [ -z "$dir" ]; then
+ return
+ fi
+ else
if __git_is_configured_remote "$remote"; then
# configured remote takes precedence over a
# local directory with the same name
@@ -360,7 +364,7 @@ __git_refs ()
fi
fi
- if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
+ if [ "$list_refs_from" = path ]; then
case "$cur" in
refs|refs/*)
format="refname"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a201b5212..5b4defaa5 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -599,7 +599,7 @@ test_expect_success '__git_refs - non-existing URL remote -
full refs' '
test_must_be_empty "$actual"
'
-test_expect_failure '__git_refs - not in a git repository' '
+test_expect_success '__git_refs - not in a git repository' '
(
GIT_CEILING_DIRECTORIES="$ROOT" &&
export GIT_CEILING_DIRECTORIES &&
--
2.11.0.555.g967c1bcb3
[PATCHv2 07/21] completion: ensure that the repository path given on the command line exists
The __gitdir() helper function prints the path to the git repository
to its stdout or stays silent and returns with error when it can't
find a repository or when the repository given via $GIT_DIR doesn't
exist.
This is not the case, however, when the path in $__git_dir, i.e. the
path to the repository specified on the command line via 'git
--git-dir=', doesn't exist: __gitdir() still outputs it as if it
were a real existing repository, making some completion functions
believe that they operate on an existing repository.
Check that the path in $__git_dir exists and return with error without
printing anything to stdout if it doesn't.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 1 +
t/t9902-completion.sh | 8
2 files changed, 9 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ee6fb2259..5b2bd6721 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -40,6 +40,7 @@ __gitdir ()
{
if [ -z "${1-}" ]; then
if [ -n "${__git_dir-}" ]; then
+ test -d "$__git_dir" || return 1
echo "$__git_dir"
elif [ -n "${GIT_DIR-}" ]; then
test -d "${GIT_DIR-}" || return 1
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7956cb9b1..7667baabf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -211,6 +211,14 @@ test_expect_success '__gitdir - $GIT_DIR set while .git
directory in parent' '
test_cmp expected "$actual"
'
+test_expect_success '__gitdir - non-existing path in $__git_dir' '
+ (
+ __git_dir="non-existing" &&
+ test_must_fail __gitdir >"$actual"
+ ) &&
+ test_must_be_empty "$actual"
+'
+
test_expect_success '__gitdir - non-existing $GIT_DIR' '
(
GIT_DIR="$ROOT/non-existing" &&
--
2.11.0.555.g967c1bcb3
[PATCHv2 12/21] completion: list short refs from a remote given as a URL
e832f5c09680 (completion: avoid ls-remote in certain scenarios,
2013-05-28) turned a 'git ls-remote ' query into a 'git
for-each-ref refs/remotes//' to improve responsiveness of
remote refs completion by avoiding potential network communication.
However, it inadvertently made impossible to complete short refs from
a remote given as a URL, e.g. 'git fetch git://server.com/repo.git
', because there is, of course, no such thing as
'refs/remotes/git://server.com/repo.git'.
Since the previous commit we tell apart configured remotes, i.e. those
that can have a hierarchy under 'refs/remotes/', from others that
don't, including remotes given as URL, so we know when we can't use
the faster 'git for-each-ref'-based approach.
Resurrect the old, pre-e832f5c09680 'git ls-remote'-based code for the
latter case to support listing short refs from remotes given as a URL.
The code is slightly updated from the original to
- take into account the path to the repository given on the command
line (if any), and
- omit 'ORIG_HEAD' from the query, as 'git ls-remote' will never
list it anyway.
When the remote given to __git_refs() doesn't exist, then it will be
handled by this resurrected 'git ls-remote' query. This code path
doesn't list 'HEAD' unconditionally, which has the nice side effect of
fixing two more expected test failures.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 19 ---
t/t9902-completion.sh | 6 +++---
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6b489b7c8..4ded44977 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -338,6 +338,7 @@ __git_tags ()
# Lists refs from the local (by default) or from a remote repository.
# It accepts 0, 1 or 2 arguments:
# 1: The remote to list refs from (optional; ignored, if set but empty).
+#Can be the name of a configured remote, a path, or a URL.
# 2: In addition to local refs, list unique branches from refs/remotes/ for
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
__git_refs ()
@@ -410,9 +411,21 @@ __git_refs ()
done
;;
*)
- echo "HEAD"
- git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
- "refs/remotes/$remote/" 2>/dev/null | sed -e
"s#^$remote/##"
+ if [ "$list_refs_from" = remote ]; then
+ echo "HEAD"
+ git --git-dir="$dir" for-each-ref
--format="%(refname:short)" \
+ "refs/remotes/$remote/" 2>/dev/null | sed -e
"s#^$remote/##"
+ else
+ git --git-dir="$dir" ls-remote "$remote" HEAD \
+ "refs/tags/*" "refs/heads/*" "refs/remotes/*"
2>/dev/null |
+ while read -r hash i; do
+ case "$i" in
+ *^{}) ;;
+ refs/*) echo "${i#refs/*/}" ;;
+ *) echo "$i" ;; # symbolic refs
+ esac
+ done
+ fi
;;
esac
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5b4defaa5..500505dca 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -540,7 +540,7 @@ test_expect_success '__git_refs - configured remote -
remote name matches a dire
test_cmp expected "$actual"
'
-test_expect_failure '__git_refs - URL remote' '
+test_expect_success '__git_refs - URL remote' '
cat >expected <<-EOF &&
HEAD
branch-in-other
@@ -567,7 +567,7 @@ test_expect_success '__git_refs - URL remote - full refs' '
test_cmp expected "$actual"
'
-test_expect_failure '__git_refs - non-existing remote' '
+test_expect_success '__git_refs - non-existing remote' '
(
cur= &&
__git_refs non-existing >"$actual"
@@ -583,7 +583,7 @@ test_expect_success '__git_refs - non-existing remote -
full refs' '
test_must_be_empty "$actual"
'
-test_expect_failure '__git_refs - non-existing URL remote' '
+test_expect_success '__git_refs - non-existing URL remote' '
(
cur= &&
__git_refs "file://$ROOT/non-existing" >"$actual"
--
2.11.0.555.g967c1bcb3
[PATCHv2 05/21] completion tests: check __gitdir()'s output in the error cases
The __gitdir() helper function shouldn't output anything if not in a git repository. The relevant tests only checked its error code, so extend them to ensure that there's no output. Signed-off-by: SZEDER Gábor --- t/t9902-completion.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 030a16e77..f7f7d49fb 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -215,8 +215,9 @@ test_expect_success '__gitdir - non-existing $GIT_DIR' ' ( GIT_DIR="$ROOT/non-existing" && export GIT_DIR && - test_must_fail __gitdir - ) + test_must_fail __gitdir >"$actual" + ) && + test_must_be_empty "$actual" ' test_expect_success '__gitdir - gitfile in cwd' ' @@ -255,7 +256,8 @@ test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' ' ' test_expect_success '__gitdir - not a git repository' ' - nongit test_must_fail __gitdir + nongit test_must_fail __gitdir >"$actual" && + test_must_be_empty "$actual" ' test_expect_success '__gitcomp - trailing space - options' ' -- 2.11.0.555.g967c1bcb3
[PATCHv2 03/21] completion tests: make the $cur variable local to the test helper functions
The test helper functions test_gitcomp() and test_gitcomp_nl() leak
the $cur variable into the test environment. Since this variable has
a special role in the Bash completion script (it holds the word
currently being completed) it influences the behavior of most
completion functions and thus this leakage could interfere with
subsequent tests. Although there are no such issues in the current
tests, early versions of the new tests that will be added later in
this series suffered because of this.
It's better to play safe and declare $cur local in those test helper
functions. 'local' is bashism, of course, but the tests of the Bash
completion script are run under Bash anyway, and there are already
other variables declared local in this test script.
Signed-off-by: SZEDER Gábor
---
t/t9902-completion.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f490c1d05..294a08cfe 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -98,7 +98,7 @@ test_gitcomp ()
{
local -a COMPREPLY &&
sed -e 's/Z$//' >expected &&
- cur="$1" &&
+ local cur="$1" &&
shift &&
__gitcomp "$@" &&
print_comp &&
@@ -113,7 +113,7 @@ test_gitcomp_nl ()
{
local -a COMPREPLY &&
sed -e 's/Z$//' >expected &&
- cur="$1" &&
+ local cur="$1" &&
shift &&
__gitcomp_nl "$@" &&
print_comp &&
--
2.11.0.555.g967c1bcb3
[PATCHv2 04/21] completion tests: consolidate getting path of current working directory
Some tests of the __gitdir() helper function use the $TRASH_DIRECTORY
variable in direct path comparisons. In general this should be
avoided, because it might contain symbolic links. There happens to be
no issues with this here, however, because those tests use
$TRASH_DIRECTORY both for specifying the expected result and for
specifying input which in turn is just 'echo'ed verbatim.
Other __gitdir() tests ask for the path of the trash directory by
running $(pwd -P) in each test, sometimes even twice in a single test.
Run $(pwd) only once at the beginning of the test script to store the
path of the trash directory in a variable, and use that variable in
all __gitdir() tests.
Signed-off-by: SZEDER Gábor
---
t/t9902-completion.sh | 44 +---
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 294a08cfe..030a16e77 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -124,15 +124,22 @@ invalid_variable_name='${foo.bar}'
actual="$TRASH_DIRECTORY/actual"
+if test_have_prereq MINGW
+then
+ ROOT="$(pwd -W)"
+else
+ ROOT="$(pwd)"
+fi
+
test_expect_success 'setup for __gitdir tests' '
mkdir -p subdir/subsubdir &&
git init otherrepo
'
test_expect_success '__gitdir - from command line (through $__git_dir)' '
- echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+ echo "$ROOT/otherrepo/.git" >expected &&
(
- __git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
+ __git_dir="$ROOT/otherrepo/.git" &&
__gitdir >"$actual"
) &&
test_cmp expected "$actual"
@@ -157,7 +164,7 @@ test_expect_success '__gitdir - .git directory in cwd' '
'
test_expect_success '__gitdir - .git directory in parent' '
- echo "$(pwd -P)/.git" >expected &&
+ echo "$ROOT/.git" >expected &&
(
cd subdir/subsubdir &&
__gitdir >"$actual"
@@ -175,7 +182,7 @@ test_expect_success '__gitdir - cwd is a .git directory' '
'
test_expect_success '__gitdir - parent is a .git directory' '
- echo "$(pwd -P)/.git" >expected &&
+ echo "$ROOT/.git" >expected &&
(
cd .git/refs/heads &&
__gitdir >"$actual"
@@ -184,9 +191,9 @@ test_expect_success '__gitdir - parent is a .git directory'
'
'
test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
- echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+ echo "$ROOT/otherrepo/.git" >expected &&
(
- GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+ GIT_DIR="$ROOT/otherrepo/.git" &&
export GIT_DIR &&
__gitdir >"$actual"
) &&
@@ -194,9 +201,9 @@ test_expect_success '__gitdir - $GIT_DIR set while .git
directory in cwd' '
'
test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
- echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+ echo "$ROOT/otherrepo/.git" >expected &&
(
- GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+ GIT_DIR="$ROOT/otherrepo/.git" &&
export GIT_DIR &&
cd subdir &&
__gitdir >"$actual"
@@ -206,24 +213,15 @@ test_expect_success '__gitdir - $GIT_DIR set while .git
directory in parent' '
test_expect_success '__gitdir - non-existing $GIT_DIR' '
(
- GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
+ GIT_DIR="$ROOT/non-existing" &&
export GIT_DIR &&
test_must_fail __gitdir
)
'
-function pwd_P_W () {
- if test_have_prereq MINGW
- then
- pwd -W
- else
- pwd -P
- fi
-}
-
test_expect_success '__gitdir - gitfile in cwd' '
- echo "$(pwd_P_W)/otherrepo/.git" >expected &&
- echo "gitdir: $(pwd_P_W)/otherrepo/.git" >subdir/.git &&
+ echo "$ROOT/otherrepo/.git" >expected &&
+ echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
test_when_finished "rm -f
[PATCHv2 02/21] completion tests: don't add test cruft to the test repository
While preparing commits, three tests added newly created files to the
index using 'git add .', which added not only the files in question
but leftover test cruft from previous tests like the files 'expected'
and 'actual' as well. Luckily, this had no effect on the tests'
correctness.
Add only the files we are actually interested in.
Signed-off-by: SZEDER Gábor
---
t/t9902-completion.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a34e55f87..f490c1d05 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success 'git --help completion' '
test_expect_success 'setup for ref completion' '
echo content >file1 &&
echo more >file2 &&
- git add . &&
+ git add file1 file2 &&
git commit -m one &&
git branch mybranch &&
git tag mytag
@@ -517,7 +517,7 @@ test_expect_success ': completes paths' '
test_expect_success 'complete tree filename with spaces' '
echo content >"name with spaces" &&
- git add . &&
+ git add "name with spaces" &&
git commit -m spaces &&
test_completion "git show HEAD:nam" <<-\EOF
name with spaces Z
@@ -526,7 +526,7 @@ test_expect_success 'complete tree filename with spaces' '
test_expect_success 'complete tree filename with metacharacters' '
echo content >"name with \${meta}" &&
- git add . &&
+ git add "name with \${meta}" &&
git commit -m meta &&
test_completion "git show HEAD:nam" <<-\EOF
name with ${meta} Z
--
2.11.0.555.g967c1bcb3
[PATCHv2 00/21] completion: various __gitdir()-related improvements
This is a long overdue reroll of a bunch of bugfixes, cleanups and
optimizations related to how the completion script finds the path to
the repository and how it uses that path. Most importantly this
series adds support for completion following 'git -C path', and it
eliminates a few subshells and git processes, for the sake of
fork()+exec() challenged OSes.
The first round is at [1]. It made its way to pu back then, but since
the reroll didn't come it was eventually discarded.
What did NOT change since v1 is that the new option added to 'git
rev-parse' is still called '--absolute-git-dir'. There was a
suggestion [2] to turn it into an orthogonal '--absolute-dir' option
that works with other path-querying options, too, but I really doubt
it's worth it. In short, regular scripts don't care, because a
relative path doesn't make any difference for them, and before we do
this orthogonal thing we have to decide a bunch of questions first,
see [3].
Changes since v1:
- Use our real_path() instead of system realpath() to implement 'git
rev-parse --absolute-git-dir'.
- Refactored a bit how __git_refs() determines where it should list
refs from.
- Renamed a few refnames and remote in the tests (this accounts for
the bulk of the interdiff).
- Misc small adjustments: a few more comments, removed unnecessary
disambiguating '--', typofix and more consistent quoting.
- Improved commit messages.
- Rebased to current master.
The interdiff below is compared to v1 rebased on top of current master.
This series is also available at
https://github.com/szeder/git completion-gitdir-improvements
[1] -
http://public-inbox.org/git/1456440650-32623-1-git-send-email-sze...@ira.uka.de/T/
[2] -
http://public-inbox.org/git/CANoM8SXO_Rz_CVOz9ptsaVCzcQ2D1FQrSuFFW4vZ4SdRYMzD=w...@mail.gmail.com/
[3] -
http://public-inbox.org/git/20160518185825.horde.epd2njnvqew_vx4b01yw...@webmail.informatik.kit.edu/
SZEDER Gábor (21):
completion: improve __git_refs()'s in-code documentation
completion tests: don't add test cruft to the test repository
completion tests: make the $cur variable local to the test helper
functions
completion tests: consolidate getting path of current working
directory
completion tests: check __gitdir()'s output in the error cases
completion tests: add tests for the __git_refs() helper function
completion: ensure that the repository path given on the command line
exists
completion: fix most spots not respecting 'git --git-dir='
completion: respect 'git --git-dir=' when listing remote refs
completion: list refs from remote when remote's name matches a
directory
completion: don't list 'HEAD' when trying refs completion outside of a
repo
completion: list short refs from a remote given as a URL
completion: don't offer commands when 'git --opt' needs an argument
completion: fix completion after 'git -C '
rev-parse: add '--absolute-git-dir' option
completion: respect 'git -C '
completion: don't use __gitdir() for git commands
completion: consolidate silencing errors from git commands
completion: don't guard git executions with __gitdir()
completion: extract repository discovery from __gitdir()
completion: cache the path to the repository
Documentation/git-rev-parse.txt| 4 +
builtin/rev-parse.c| 26 +-
contrib/completion/git-completion.bash | 250 ++-
t/t1500-rev-parse.sh | 17 +-
t/t9902-completion.sh | 561 +
5 files changed, 690 insertions(+), 168 deletions(-)
--
2.11.0.555.g967c1bcb3
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4040b3c86..1967bafba 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -820,10 +820,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
if (!gitdir && !prefix)
gitdir = ".git";
if (gitdir) {
- char absolute_path[PATH_MAX];
- if (!realpath(gitdir,
absolute_path))
- die_errno(_("unable to
get absolute path"));
- puts(absolute_path);
+ puts(real_path(gitdir));
continue;
}
}
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 0184d0ebc..ed06cb621 100644
--- a/contrib/completion/git-completion.bash
+++ b/
[PATCHv2 13/21] completion: don't offer commands when 'git --opt' needs an argument
The main git options '--git-dir', '-c', '-C', '--worktree' and '--namespace' require an argument, but attempting completion right after them lists git commands. Don't offer anything right after these options, thus let Bash fall back to filename completion, because - the three options '--git-dir', '-C' and '--worktree' do actually require a path argument, and - we don't complete the required argument of '-c' and '--namespace', and in that case the "standard" behavior of our completion script is to not offer anything, but fall back to filename completion. Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 11 +++ 1 file changed, 11 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 4ded44977..7d25b33b8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2808,6 +2808,17 @@ __git_main () done if [ -z "$command" ]; then + case "$prev" in + --git-dir|-C|--work-tree) + # these need a path argument, let's fall back to + # Bash filename completion + return + ;; + -c|--namespace) + # we don't support completing these options' arguments + return + ;; + esac case "$cur" in --*) __gitcomp " --paginate -- 2.11.0.555.g967c1bcb3
[PATCHv2 18/21] completion: consolidate silencing errors from git commands
Outputting error messages during completion is bad: they disrupt the
command line, can't be deleted, and the user is forced to Ctrl-C and
start over most of the time. We already silence stderr of many git
commands in our Bash completion script, but there are still some in
there that can spew error messages when something goes wrong.
We could add the missing stderr redirections to all the remaining
places, but instead let's leverage that git commands are now executed
through the previously introduced __git() wrapper function, and
redirect standard error to /dev/null only in that function. This way
we need only one redirection to take care of errors from almost all
git commands. Redirecting standard error of the __git() wrapper
function thus became redundant, remove them.
The exceptions, i.e. the repo-independent git executions and those in
the __gitdir() function that don't go through __git() already have
their standard error silenced.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index e7c0b56ea..1a849761f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -66,7 +66,7 @@ __gitdir ()
__git ()
{
git ${__git_C_args:+"${__git_C_args[@]}"} \
- ${__git_dir:+--git-dir="$__git_dir"} "$@"
+ ${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
}
# The following function is based on code from:
@@ -300,7 +300,7 @@ __git_ls_files_helper ()
else
# NOTE: $2 is not quoted in order to support multiple options
__git -C "$1" ls-files --exclude-standard $2
- fi 2>/dev/null
+ fi
}
@@ -410,7 +410,7 @@ __git_refs ()
fi
case "$cur" in
refs|refs/*)
- __git ls-remote "$remote" "$cur*" 2>/dev/null | \
+ __git ls-remote "$remote" "$cur*" | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -422,10 +422,10 @@ __git_refs ()
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
__git for-each-ref --format="%(refname:short)" \
- "refs/remotes/$remote/" 2>/dev/null | sed -e
"s#^$remote/##"
+ "refs/remotes/$remote/" | sed -e "s#^$remote/##"
else
__git ls-remote "$remote" HEAD \
- "refs/tags/*" "refs/heads/*" "refs/remotes/*"
2>/dev/null |
+ "refs/tags/*" "refs/heads/*" "refs/remotes/*" |
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -451,7 +451,7 @@ __git_refs2 ()
__git_refs_remotes ()
{
local i hash
- __git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+ __git ls-remote "$1" 'refs/heads/*' | \
while read -r hash i; do
echo "$i:refs/remotes/$1/${i#refs/heads/}"
done
@@ -527,7 +527,7 @@ __git_complete_revlist_file ()
*) pfx="$ref:$pfx" ;;
esac
- __gitcomp_nl "$(__git ls-tree "$ls" 2>/dev/null \
+ __gitcomp_nl "$(__git ls-tree "$ls" \
| sed '/^100... blob /{
s,^.*,,
s,$, ,
@@ -805,7 +805,7 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section="$1" i IFS=$'\n'
- for i in $(__git config --name-only --get-regexp "^$section\..*"
2>/dev/null); do
+ for i in $(__git config --name-only --get-regexp "^$section\..*"); do
echo "${i#$section.}"
done
}
@@ -823,7 +823,7 @@ __git_aliases ()
# __git_aliased_command requires 1 argument
__git_aliased_command ()
{
- local word cmdline=$(__git config --get "alias.$1" 2>/dev/null)
+ local word cmdline=$(__git config --get "alias.$1")
for word in $cmdline; do
case "$word" in
\!gitk|gitk)
@@ -1841,9 +1841,7 @@ _git_send_email ()
{
case "$prev" in
--to|--cc|--bcc|--from)
- __gitcomp "
- $(__git send-email --dump-aliase
[PATCHv2 21/21] completion: cache the path to the repository
After the previous changes in this series there are only a handful of
$(__gitdir) command substitutions left in the completion script, but
there is still a bit of room for improvements:
1. The command substitution involves the forking of a subshell,
which has considerable overhead on some platforms.
2. There are a few cases, where this command substitution is
executed more than once during a single completion, which means
multiple subshells and possibly multiple 'git rev-parse'
executions. __gitdir() is invoked twice while completing refs
for e.g. 'git log', 'git rebase', 'gitk', or while completing
remote refs for 'git fetch' or 'git push'.
Both of these points can be addressed by using the
__git_find_repo_path() helper function introduced in the previous
commit:
1. __git_find_repo_path() stores the path to the repository in a
variable instead of printing it, so the command substitution
around the function can be avoided. Or rather: the command
substitution should be avoided to make the new value of the
variable set inside the function visible to the callers.
(Yes, there is now a command substitution inside
__git_find_repo_path() around each 'git rev-parse', but that's
executed only if necessary, and only once per completion, see
point 2. below.)
2. $__git_repo_path, the variable holding the path to the
repository, is declared local in the toplevel completion
functions __git_main() and __gitk_main(). Thus, once set, the
path is visible in all completion functions, including all
subsequent calls to __git_find_repo_path(), meaning that they
wouldn't have to re-discover the path to the repository.
So call __git_find_repo_path() and use $__git_repo_path instead of the
$(__gitdir) command substitution to access paths in the .git
directory. Turn tests checking __gitdir()'s repository discovery into
tests of __git_find_repo_path() such that only the tested function
changes but the expected results don't, ensuring that repo discovery
keeps working as it did before.
As __gitdir() is not used anymore in the completion script, mark it as
deprecated and direct users' attention to __git_find_repo_path() and
$__git_repo_path. Yet keep four __gitdir() tests to ensure that it
handles success and failure of __git_find_repo_path() and that it
still handles its optional remote argument, because users' custom
completion scriptlets might depend on it.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 46 ++
t/t9902-completion.sh | 161 +
2 files changed, 132 insertions(+), 75 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 7775411cd..ed06cb621 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -39,6 +39,11 @@ esac
# variable.
__git_find_repo_path ()
{
+ if [ -n "$__git_repo_path" ]; then
+ # we already know where it is
+ return
+ fi
+
if [ -n "${__git_C_args-}" ]; then
__git_repo_path="$(git "${__git_C_args[@]}" \
${__git_dir:+--git-dir="$__git_dir"} \
@@ -56,6 +61,7 @@ __git_find_repo_path ()
fi
}
+# Deprecated: use __git_find_repo_path() and $__git_repo_path instead
# __gitdir accepts 0 or 1 arguments (i.e., location)
# returns location of .git repo
__gitdir ()
@@ -350,10 +356,13 @@ __git_tags ()
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
__git_refs ()
{
- local i hash dir="$(__gitdir)" track="${2-}"
+ local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
local format refs pfx
+ __git_find_repo_path
+ dir="$__git_repo_path"
+
if [ -z "$remote" ]; then
if [ -z "$dir" ]; then
return
@@ -458,8 +467,8 @@ __git_refs_remotes ()
__git_remotes ()
{
- local d="$(__gitdir)"
- test -d "$d/remotes" && ls -1 "$d/remotes"
+ __git_find_repo_path
+ test -d "$__git_repo_path/remotes" && ls -1 "$__git_repo_path/remotes"
__git remote
}
@@ -957,8 +966,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
_git_am ()
{
- local dir="$(__gitdir)"
- if [ -d "$dir"/rebase-apply ]; then
+ __git_find_repo_path
+ if [ -d "$__git_repo_path"/rebase-apply ]; then
__gitcomp "--skip --continue --resolved --abort"
return
fi
@@ -1041,7 +1050,8 @@ _git_bise
[PATCHv2 08/21] completion: fix most spots not respecting 'git --git-dir='
The completion script already respects the path to the repository
specified on the command line most of the time, here we add the
necessary '--git-dir=$(__gitdir)' options to most of the places where
git was executed without it. The exceptions where said option is not
added are the git invocations:
- in __git_refs() which are non-trivial and will be the subject of
the following patch,
- getting the list of git commands, merge strategies and archive
formats, because these are independent from the repository and
thus don't need it, and
- the 'git rev-parse --git-dir' in __gitdir() itself.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 5b2bd6721..295f6de24 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -283,11 +283,13 @@ __gitcomp_file ()
# argument, and using the options specified in the second argument.
__git_ls_files_helper ()
{
+ local dir="$(__gitdir)"
+
if [ "$2" == "--committable" ]; then
- git -C "$1" diff-index --name-only --relative HEAD
+ git --git-dir="$dir" -C "$1" diff-index --name-only --relative
HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
- git -C "$1" ls-files --exclude-standard $2
+ git --git-dir="$dir" -C "$1" ls-files --exclude-standard $2
fi 2>/dev/null
}
@@ -408,7 +410,7 @@ __git_refs2 ()
__git_refs_remotes ()
{
local i hash
- git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+ git --git-dir="$(__gitdir)" ls-remote "$1" 'refs/heads/*' 2>/dev/null |
\
while read -r hash i; do
echo "$i:refs/remotes/$1/${i#refs/heads/}"
done
@@ -1186,7 +1188,7 @@ _git_commit ()
return
esac
- if git rev-parse --verify --quiet HEAD >/dev/null; then
+ if git --git-dir="$(__gitdir)" rev-parse --verify --quiet HEAD
>/dev/null; then
__git_complete_index_file "--committable"
else
# This is the first commit
@@ -1486,7 +1488,7 @@ _git_log ()
{
__git_has_doubledash && return
- local g="$(git rev-parse --git-dir 2>/dev/null)"
+ local g="$(__gitdir)"
local merge=""
if [ -f "$g/MERGE_HEAD" ]; then
merge="--merge"
--
2.11.0.555.g967c1bcb3
[PATCHv2 20/21] completion: extract repository discovery from __gitdir()
To prepare for caching the path to the repository in the following
commit, extract the repository discovering part of __gitdir() into the
__git_find_repo_path() helper function, which stores the found path in
the $__git_repo_path variable instead of printing it. Make __gitdir()
a wrapper around this new function. Declare $__git_repo_path local in
the toplevel completion functions __git_main() and __gitk_main() to
ensure that it never leaks into the environment and influences
subsequent completions (though this isn't necessary right now, as
__gitdir() is still only executed in subshells, but will matter for
the following commit).
Adjust tests checking __gitdir() or any other completion function
calling __gitdir() to perform those checks in a subshell to prevent
$__git_repo_path from leaking into the test environment. Otherwise
leave the tests unchanged to demonstrate that this change doesn't
alter __gitdir()'s behavior.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 42 +-
t/t9902-completion.sh | 22 +-
2 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 1c7493ff2..7775411cd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,26 +34,35 @@ case "$COMP_WORDBREAKS" in
*) COMP_WORDBREAKS="$COMP_WORDBREAKS:"
esac
+# Discovers the path to the git repository taking any '--git-dir=' and
+# '-C ' options into account and stores it in the $__git_repo_path
+# variable.
+__git_find_repo_path ()
+{
+ if [ -n "${__git_C_args-}" ]; then
+ __git_repo_path="$(git "${__git_C_args[@]}" \
+ ${__git_dir:+--git-dir="$__git_dir"} \
+ rev-parse --absolute-git-dir 2>/dev/null)"
+ elif [ -n "${__git_dir-}" ]; then
+ test -d "$__git_dir" &&
+ __git_repo_path="$__git_dir"
+ elif [ -n "${GIT_DIR-}" ]; then
+ test -d "${GIT_DIR-}" &&
+ __git_repo_path="$GIT_DIR"
+ elif [ -d .git ]; then
+ __git_repo_path=.git
+ else
+ __git_repo_path="$(git rev-parse --git-dir 2>/dev/null)"
+ fi
+}
+
# __gitdir accepts 0 or 1 arguments (i.e., location)
# returns location of .git repo
__gitdir ()
{
if [ -z "${1-}" ]; then
- if [ -n "${__git_C_args-}" ]; then
- git "${__git_C_args[@]}" \
- ${__git_dir:+--git-dir="$__git_dir"} \
- rev-parse --absolute-git-dir 2>/dev/null
- elif [ -n "${__git_dir-}" ]; then
- test -d "$__git_dir" || return 1
- echo "$__git_dir"
- elif [ -n "${GIT_DIR-}" ]; then
- test -d "${GIT_DIR-}" || return 1
- echo "$GIT_DIR"
- elif [ -d .git ]; then
- echo .git
- else
- git rev-parse --git-dir 2>/dev/null
- fi
+ __git_find_repo_path || return 1
+ echo "$__git_repo_path"
elif [ -d "$1/.git" ]; then
echo "$1/.git"
else
@@ -2783,7 +2792,7 @@ _git_worktree ()
__git_main ()
{
- local i c=1 command __git_dir
+ local i c=1 command __git_dir __git_repo_path
local __git_C_args C_args_count=0
while [ $c -lt $cword ]; do
@@ -2855,6 +2864,7 @@ __gitk_main ()
{
__git_has_doubledash && return
+ local __git_repo_path
local g="$(__gitdir)"
local merge=""
if [ -f "$g/MERGE_HEAD" ]; then
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 984df05b2..1816ed2e0 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -147,19 +147,25 @@ test_expect_success '__gitdir - from command line
(through $__git_dir)' '
test_expect_success '__gitdir - repo as argument' '
echo "otherrepo/.git" >expected &&
- __gitdir "otherrepo" >"$actual" &&
+ (
+ __gitdir "otherrepo" >"$actual"
+ ) &&
test_cmp expected "$actual"
'
test_expect_success '__gitdir - remote as argument' '
echo "remote" >expected &&
- __gitdir "remote" >"$actual" &&
+ (
+ __
[PATCHv2 10/21] completion: list refs from remote when remote's name matches a directory
If the remote given to __git_refs() happens to match both the name of
a configured remote and the name of a directory in the current working
directory, then that directory is assumed to be a git repository, and
listing refs from that directory will be attempted. This is wrong,
because in such a situation git commands (e.g. 'git fetch|pull|push
' whom these refs will eventually be passed to) give
precedence to the configured remote. Therefore, __git_refs() should
list refs from the configured remote as well.
Add the helper function __git_is_configured_remote() that checks
whether its argument matches the name of a configured remote. Use
this helper to decide how to handle the remote passed to __git_refs().
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 20 ++--
t/t9902-completion.sh | 11 ++-
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 7d7e8b9d9..fd0ba1f3b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -347,12 +347,16 @@ __git_refs ()
local format refs pfx
if [ -n "$remote" ]; then
- if [ -d "$remote/.git" ]; then
+ if __git_is_configured_remote "$remote"; then
+ # configured remote takes precedence over a
+ # local directory with the same name
+ list_refs_from=remote
+ elif [ -d "$remote/.git" ]; then
dir="$remote/.git"
elif [ -d "$remote" ]; then
dir="$remote"
else
- list_refs_from=remote
+ list_refs_from=url
fi
fi
@@ -435,6 +439,18 @@ __git_remotes ()
git --git-dir="$d" remote
}
+# Returns true if $1 matches the name of a configured remote, false otherwise.
+__git_is_configured_remote ()
+{
+ local remote
+ for remote in $(__git_remotes); do
+ if [ "$remote" = "$1" ]; then
+ return 0
+ fi
+ done
+ return 1
+}
+
__git_list_merge_strategies ()
{
git merge -s help 2>&1 |
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6e64cd6ba..a201b5212 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -373,6 +373,15 @@ test_expect_success '__git_remotes - list remotes from
$GIT_DIR/remotes and from
test_cmp expect actual
'
+test_expect_success '__git_is_configured_remote' '
+ test_when_finished "git remote remove remote_1" &&
+ git remote add remote_1 git://remote_1 &&
+ test_when_finished "git remote remove remote_2" &&
+ git remote add remote_2 git://remote_2 &&
+ verbose __git_is_configured_remote remote_2 &&
+ test_must_fail __git_is_configured_remote non-existent
+'
+
test_expect_success 'setup for ref completion' '
git commit --allow-empty -m initial &&
git branch matching-branch &&
@@ -516,7 +525,7 @@ test_expect_success '__git_refs - configured remote - full
refs - repo given on
test_cmp expected "$actual"
'
-test_expect_failure '__git_refs - configured remote - remote name matches a
directory' '
+test_expect_success '__git_refs - configured remote - remote name matches a
directory' '
cat >expected <<-EOF &&
HEAD
branch-in-other
--
2.11.0.555.g967c1bcb3
[PATCHv2 17/21] completion: don't use __gitdir() for git commands
Several completion functions contain the following pattern to run git
commands respecting the path to the repository specified on the
command line:
git --git-dir="$(__gitdir)"
This imposes the overhead of fork()ing a subshell for the command
substitution and potentially fork()+exec()ing 'git rev-parse' inside
__gitdir().
Now, if neither '--gitdir=' nor '-C ' options are
specified on the command line, then those git commands are perfectly
capable to discover the repository on their own. If either one or
both of those options are specified on the command line, then, again,
the git commands could discover the repository, if we pass them all of
those options from the command line.
This means we don't have to run __gitdir() at all for git commands and
can spare its fork()+exec() overhead.
Use Bash parameter expansions to check the $__git_dir variable and
$__git_C_args array and to assemble the appropriate '--git-dir='
and '-C ' options if either one or both are present on the
command line. These parameter expansions are, however, rather long,
so instead of changing all git executions and make already long lines
even longer, encapsulate running git with '--git-dir= -C '
options into the new __git() wrapper function. Furthermore, this
wrapper function will also enable us to silence error messages from
git commands uniformly in one place in a later commit.
There's one tricky case, though: in __git_refs() local refs are listed
with 'git for-each-ref', where "local" is not necessarily the
repository we are currently in, but it might mean a remote repository
in the filesystem (e.g. listing refs for 'git fetch /some/other/repo
'). Use one-shot variable assignment to override $__git_dir with
the path of the repository where the refs should come from. Although
one-shot variable assignments in front of shell functions are to be
avoided in our scripts in general, in the Bash completion script we
can do that safely.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 60 ++
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index e003afd54..e7c0b56ea 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -61,6 +61,14 @@ __gitdir ()
fi
}
+# Runs git with all the options given as argument, respecting any
+# '--git-dir=' and '-C ' options present on the command line
+__git ()
+{
+ git ${__git_C_args:+"${__git_C_args[@]}"} \
+ ${__git_dir:+--git-dir="$__git_dir"} "$@"
+}
+
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
@@ -287,13 +295,11 @@ __gitcomp_file ()
# argument, and using the options specified in the second argument.
__git_ls_files_helper ()
{
- local dir="$(__gitdir)"
-
if [ "$2" == "--committable" ]; then
- git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C
"$1" diff-index --name-only --relative HEAD
+ __git -C "$1" diff-index --name-only --relative HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
- git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C
"$1" ls-files --exclude-standard $2
+ __git -C "$1" ls-files --exclude-standard $2
fi 2>/dev/null
}
@@ -323,8 +329,7 @@ __git_heads ()
{
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
- git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
- refs/heads
+ __git for-each-ref --format='%(refname:short)' refs/heads
return
fi
}
@@ -333,8 +338,7 @@ __git_tags ()
{
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
- git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
- refs/tags
+ __git for-each-ref --format='%(refname:short)' refs/tags
return
fi
}
@@ -385,14 +389,14 @@ __git_refs ()
refs="refs/tags refs/heads refs/remotes"
;;
esac
- git --git-dir="$dir" for-each-ref --format="$pfx%($format)" \
+ __git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
$refs
if [ -n "$track" ]; then
# employ the heuristic used by git checkou
[PATCHv2 19/21] completion: don't guard git executions with __gitdir()
Three completion functions, namely __git_index_files(), __git_heads()
and __git_tags(), first run __gitdir() and check that the path it
outputs exists, i.e. that there is a git repository, and run a git
command only if there is one.
After the previous changes in this series there are no further uses of
__gitdir()'s output in these functions besides those checks. And
those checks are unnecessary, because we can just execute those git
commands outside of a repository and let them error out. We don't
perform such a check in other places either.
Remove this check and the __gitdir() call from these functions,
sparing the fork()+exec() overhead of the command substitution and the
potential 'git rev-parse' execution.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 1a849761f..1c7493ff2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -312,35 +312,25 @@ __git_ls_files_helper ()
#slash.
__git_index_files ()
{
- local dir="$(__gitdir)" root="${2-.}" file
-
- if [ -d "$dir" ]; then
- __git_ls_files_helper "$root" "$1" |
- while read -r file; do
- case "$file" in
- ?*/*) echo "${file%%/*}" ;;
- *) echo "$file" ;;
- esac
- done | sort | uniq
- fi
+ local root="${2-.}" file
+
+ __git_ls_files_helper "$root" "$1" |
+ while read -r file; do
+ case "$file" in
+ ?*/*) echo "${file%%/*}" ;;
+ *) echo "$file" ;;
+ esac
+ done | sort | uniq
}
__git_heads ()
{
- local dir="$(__gitdir)"
- if [ -d "$dir" ]; then
- __git for-each-ref --format='%(refname:short)' refs/heads
- return
- fi
+ __git for-each-ref --format='%(refname:short)' refs/heads
}
__git_tags ()
{
- local dir="$(__gitdir)"
- if [ -d "$dir" ]; then
- __git for-each-ref --format='%(refname:short)' refs/tags
- return
- fi
+ __git for-each-ref --format='%(refname:short)' refs/tags
}
# Lists refs from the local (by default) or from a remote repository.
--
2.11.0.555.g967c1bcb3
[PATCHv2 15/21] rev-parse: add '--absolute-git-dir' option
The output of 'git rev-parse --git-dir' can be either a relative or an
absolute path, depending on whether the current working directory is
at the top of the worktree or the .git directory or not, or how the
path to the repository is specified via the '--git-dir=' option
or the $GIT_DIR environment variable. And if that output is a
relative path, then it is relative to the directory where any 'git
-C ' options might have led us.
This doesn't matter at all for regular scripts, because the git
wrapper automatically takes care of changing directories according to
the '-C ' options, and the scripts can then simply follow any
path returned by 'git rev-parse --git-dir', even if it's a relative
path.
Our Bash completion script, however, is unique in that it must run
directly in the user's interactive shell environment. This means that
it's not executed through the git wrapper and would have to take care
of any '-C options on its own, and it can't just change
directories as it pleases. Consequently, adding support for taking
any '-C ' options on the command line into account during
completion turned out to be considerably more difficult, error prone
and required more subshells and git processes when it had to cope with
a relative path to the .git directory.
Help this rather special use case and teach 'git rev-parse' a new
'--absolute-git-dir' option which always outputs a canonicalized
absolute path to the .git directory, regardless of whether the path is
discovered automatically or is specified via $GIT_DIR or 'git
--git-dir='.
Signed-off-by: SZEDER Gábor
---
Documentation/git-rev-parse.txt | 4
builtin/rev-parse.c | 26 ++
t/t1500-rev-parse.sh| 17 +
3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 7241e9689..91c02b8c8 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -217,6 +217,10 @@ If `$GIT_DIR` is not defined and the current directory
is not detected to lie in a Git repository or work tree
print a message to stderr and exit with nonzero status.
+--absolute-git-dir::
+ Like `--git-dir`, but its output is always the canonicalized
+ absolute path.
+
--git-common-dir::
Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1..1967bafba 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -802,17 +802,27 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
putchar('\n');
continue;
}
- if (!strcmp(arg, "--git-dir")) {
+ if (!strcmp(arg, "--git-dir") ||
+ !strcmp(arg, "--absolute-git-dir")) {
const char *gitdir =
getenv(GIT_DIR_ENVIRONMENT);
char *cwd;
int len;
- if (gitdir) {
- puts(gitdir);
- continue;
- }
- if (!prefix) {
- puts(".git");
- continue;
+ if (arg[2] == 'g') {/* --git-dir */
+ if (gitdir) {
+ puts(gitdir);
+ continue;
+ }
+ if (!prefix) {
+ puts(".git");
+ continue;
+ }
+ } else {/* --absolute-git-dir */
+ if (!gitdir && !prefix)
+ gitdir = ".git";
+ if (gitdir) {
+ puts(real_path(gitdir));
+ continue;
+ }
}
cwd = xgetcwd();
len = strlen(cwd);
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c40..8b62ed85b 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -3,7 +3,7 @@
test_description='test git rev-parse'
. ./test-lib.sh
-# usage: [options] label is-bare is-inside-git is-inside-work prefix gi
[PATCHv2 01/21] completion: improve __git_refs()'s in-code documentation
That "first argument is passed to __gitdir()" statement in particular
is not really helpful, and after this series it won't be the case
anyway.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6721ff80f..ee6fb2259 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -332,9 +332,11 @@ __git_tags ()
fi
}
-# __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
-# presence of 2nd argument means use the guess heuristic employed
-# by checkout for tracking branches
+# Lists refs from the local (by default) or from a remote repository.
+# It accepts 0, 1 or 2 arguments:
+# 1: The remote to list refs from (optional; ignored, if set but empty).
+# 2: In addition to local refs, list unique branches from refs/remotes/ for
+#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
__git_refs ()
{
local i hash dir="$(__gitdir "${1-}")" track="${2-}"
--
2.11.0.555.g967c1bcb3
[PATCHv2 06/21] completion tests: add tests for the __git_refs() helper function
Check how __git_refs() lists refs in different scenarios, i.e. - short and full refs, - from a local or from a remote repository, - remote specified via path, name or URL, - with or without a repository specified on the command line, - non-existing remote, - unique remote branches for 'git checkout's tracking DWIMery, - not in a git repository, and - interesting combinations of the above. Seven of these tests expect failure, mostly demonstrating bugs related to listing refs from a remote repository: - ignoring the repository specified on the command line (2 tests), - listing refs from the wrong place when the name of a configured remote happens to match a directory, - listing only 'HEAD' but no short refs from a remote given as URL, - listing 'HEAD' even from non-existing remotes (2 tests), and - listing 'HEAD' when not in a repository. Signed-off-by: SZEDER Gábor --- t/t9902-completion.sh | 265 +- 1 file changed, 264 insertions(+), 1 deletion(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index f7f7d49fb..7956cb9b1 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -365,6 +365,269 @@ test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from test_cmp expect actual ' +test_expect_success 'setup for ref completion' ' + git commit --allow-empty -m initial && + git branch matching-branch && + git tag matching-tag && + ( + cd otherrepo && + git commit --allow-empty -m initial && + git branch -m master master-in-other && + git branch branch-in-other && + git tag tag-in-other + ) && + git remote add other "$ROOT/otherrepo/.git" && + git fetch --no-tags other && + rm -f .git/FETCH_HEAD && + git init thirdrepo +' + +test_expect_success '__git_refs - simple' ' + cat >expected <<-EOF && + HEAD + master + matching-branch + other/branch-in-other + other/master-in-other + matching-tag + EOF + ( + cur= && + __git_refs >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - full refs' ' + cat >expected <<-EOF && + refs/heads/master + refs/heads/matching-branch + EOF + ( + cur=refs/heads/ && + __git_refs >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - repo given on the command line' ' + cat >expected <<-EOF && + HEAD + branch-in-other + master-in-other + tag-in-other + EOF + ( + __git_dir="$ROOT/otherrepo/.git" && + cur= && + __git_refs >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - remote on local file system' ' + cat >expected <<-EOF && + HEAD + branch-in-other + master-in-other + tag-in-other + EOF + ( + cur= && + __git_refs otherrepo >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - remote on local file system - full refs' ' + cat >expected <<-EOF && + refs/heads/branch-in-other + refs/heads/master-in-other + refs/tags/tag-in-other + EOF + ( + cur=refs/ && + __git_refs otherrepo >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - configured remote' ' + cat >expected <<-EOF && + HEAD + branch-in-other + master-in-other + EOF + ( + cur= && + __git_refs other >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success '__git_refs - configured remote - full refs' ' + cat >expected <<-EOF && + refs/heads/branch-in-other + refs/heads/master-in-other + refs/tags/tag-in-other + EOF + ( + cur=refs/ && + __git_refs other >"$actual" + ) && + test_cmp expected "$actual" +' + +test_exp
[PATCHv2 09/21] completion: respect 'git --git-dir=' when listing remote refs
In __git_refs() the git commands listing refs, both short and full,
from a given remote repository are run without giving them the path to
the git repository which might have been specified on the command line
via 'git --git-dir='. This is bad, those git commands should
access the 'refs/remotes//' hierarchy or the remote and
credentials configuration in that specified repository.
Use the __gitdir() helper only to find the path to the .git directory
and pass the resulting path to the 'git ls-remote' and 'for-each-ref'
executions that list remote refs. While modifying that 'for-each-ref'
line, remove the superfluous disambiguating doubledash.
Don't use __gitdir() to check that the given remote is on the file
system: basically it performs only a single if statement for us at the
considerable cost of fork()ing a subshell for a command substitution.
We are better off to perform all the necessary checks of the remote in
__git_refs().
Though __git_refs() was the last remaining callsite that passed a
remote to __gitdir(), don't delete __gitdir()'s remote-handling part
yet, just in case some users' custom completion scriptlets depend on
it.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 22 +-
t/t9902-completion.sh | 4 ++--
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 295f6de24..7d7e8b9d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -342,9 +342,21 @@ __git_tags ()
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
__git_refs ()
{
- local i hash dir="$(__gitdir "${1-}")" track="${2-}"
+ local i hash dir="$(__gitdir)" track="${2-}"
+ local list_refs_from=path remote="${1-}"
local format refs pfx
- if [ -d "$dir" ]; then
+
+ if [ -n "$remote" ]; then
+ if [ -d "$remote/.git" ]; then
+ dir="$remote/.git"
+ elif [ -d "$remote" ]; then
+ dir="$remote"
+ else
+ list_refs_from=remote
+ fi
+ fi
+
+ if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
case "$cur" in
refs|refs/*)
format="refname"
@@ -381,7 +393,7 @@ __git_refs ()
fi
case "$cur" in
refs|refs/*)
- git ls-remote "$dir" "$cur*" 2>/dev/null | \
+ git --git-dir="$dir" ls-remote "$remote" "$cur*" 2>/dev/null | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -391,8 +403,8 @@ __git_refs ()
;;
*)
echo "HEAD"
- git for-each-ref --format="%(refname:short)" -- \
- "refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
+ git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
+ "refs/remotes/$remote/" 2>/dev/null | sed -e
"s#^$remote/##"
;;
esac
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7667baabf..6e64cd6ba 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success '__git_refs - configured remote - full
refs' '
test_cmp expected "$actual"
'
-test_expect_failure '__git_refs - configured remote - repo given on the
command line' '
+test_expect_success '__git_refs - configured remote - repo given on the
command line' '
cat >expected <<-EOF &&
HEAD
branch-in-other
@@ -501,7 +501,7 @@ test_expect_failure '__git_refs - configured remote - repo
given on the command
test_cmp expected "$actual"
'
-test_expect_failure '__git_refs - configured remote - full refs - repo given
on the command line' '
+test_expect_success '__git_refs - configured remote - full refs - repo given
on the command line' '
cat >expected <<-EOF &&
refs/heads/branch-in-other
refs/heads/master-in-other
--
2.11.0.555.g967c1bcb3
[PATCHv2 14/21] completion: fix completion after 'git -C '
The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word. As it is not aware of 'git -C's
mandatory path argument, if the '-C ' option is present, 'path'
will be the first such word and it will be mistaken for a git command.
This breaks completion in various ways:
- If 'path' happens to match one of the commands supported by the
completion script, then options of that command will be offered.
- If 'path' doesn't match a supported command and doesn't contain any
characters not allowed in Bash identifier names, then the
completion script does basically nothing and Bash in turn falls
back to filename completion for all subsequent words.
- Otherwise, if 'path' does contain such an unallowed character, then
it leads to a more or less ugly error message in the middle of the
command line. The standard '/' directory separator is such a
character, and it happens to trigger one of the uglier errors:
$ git -C some/path sh.exe": declare: `_git_some/path': not a valid
identifier
error: invalid key: alias.some/path
Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line. Extend the relevant test with
this case and, while at it, with cases that needed similar treatment
in the past ('--git-dir', '-c', '--work-tree' and '--namespace').
Additionally, silence the standard error of the 'declare' builtins
looking for the completion function associated with the git command
and of the 'git config' query for the aliased command. So if git ever
learns a new option with a mandatory argument in the future, then,
though the completion script will again misbehave, at least the
command line will not be utterly disrupted by those error messages.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 8
t/t9902-completion.sh | 7 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 7d25b33b8..ac5eb9ced 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -816,7 +816,7 @@ __git_aliases ()
__git_aliased_command ()
{
local word cmdline=$(git --git-dir="$(__gitdir)" \
- config --get "alias.$1")
+ config --get "alias.$1" 2>/dev/null)
for word in $cmdline; do
case "$word" in
\!gitk|gitk)
@@ -2800,7 +2800,7 @@ __git_main ()
--git-dir) ((c++)) ; __git_dir="${words[c]}" ;;
--bare) __git_dir="." ;;
--help) command="help"; break ;;
- -c|--work-tree|--namespace) ((c++)) ;;
+ -c|-C|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command="$i"; break ;;
esac
@@ -2844,13 +2844,13 @@ __git_main ()
fi
local completion_func="_git_${command//-/_}"
- declare -f $completion_func >/dev/null && $completion_func && return
+ declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
&& return
local expansion=$(__git_aliased_command "$command")
if [ -n "$expansion" ]; then
words[1]=$expansion
completion_func="_git_${expansion//-/_}"
- declare -f $completion_func >/dev/null && $completion_func
+ declare -f $completion_func >/dev/null 2>/dev/null &&
$completion_func
fi
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 500505dca..be2ed8704 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -755,7 +755,12 @@ test_expect_success 'general options plus command' '
test_completion "git --namespace=foo check" "checkout " &&
test_completion "git --paginate check" "checkout " &&
test_completion "git --info-path check" "checkout " &&
- test_completion "git --no-replace-objects check" "checkout "
+ test_completion "git --no-replace-objects check" "checkout " &&
+ test_completion "git --git-dir some/path check" "checkout " &&
+ test_completion "git -c conf.var=value check" "checkout " &&
+ test_completion "git -C some/path check" "checkout " &&
+ test_completion "git --work-tree some/path check" "checkout " &&
+ test_completion "git --namespace name/space check" "checkout "
'
test_expect_success 'git --help completion' '
--
2.11.0.555.g967c1bcb3
[PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit()
Those two options are specifying the default values that
__gitcomp_nl() would use anyway when invoked with no options at all.
Signed-off-by: SZEDER Gábor
---
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 ed06cb621..f20d4600c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1216,7 +1216,7 @@ _git_commit ()
{
case "$prev" in
-c|-C)
- __gitcomp_nl "$(__git_refs)" "" "${cur}"
+ __gitcomp_nl "$(__git_refs)"
return
;;
esac
--
2.11.0.555.g967c1bcb3
[PATCH 00/12] completion: speed up refs completion
This series speeds up refs completion for large number of refs, partly by giving up disambiguating ambiguous refs (patch 6) and partly by eliminating most of the shell processing between 'git for-each-ref' and 'ls-remote' and Bash's completion facility. The rest is a bit of preparatory reorganization, cleanup and bugfixes. The last patch touches the ZSH wrapper, too. By a lucky educated guess I managed to get it work on the first try, but I don't really know what I've actually done, so... ZSH users, please have a closer look. At the end of this series refs completion from a local repository is as fast as it can possibly get, at least as far as the completion script is concerned, because it basically does nothing anymore :) All it does is run 'git for-each-ref' with assorted options to do all the work, and feed its output directly, without any processing into Bash's COMPREPLY array. There is still room for improvements in the code paths using 'git ls-remote', but for that we would need enhancements to 'ls-remote'. It goes on top of the __gitdir() improvements series I just posted at: http://public-inbox.org/git/20170203024829.8071-1-szeder@gmail.com/T/ This series is also available at: https://github.com/szeder/git completion-refs-speedup SZEDER Gábor (12): completion: remove redundant __gitcomp_nl() options from _git_commit() completion: wrap __git_refs() for better option parsing completion: support completing full refs after '--option=refs/' completion: support excluding full refs completion: don't disambiguate tags and branches completion: don't disambiguate short refs completion: let 'for-each-ref' and 'ls-remote' filter matching refs completion: let 'for-each-ref' strip the remote name from remote branches completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery completion: list only matching symbolic and pseudorefs when completing refs completion: fill COMPREPLY directly when completing refs contrib/completion/git-completion.bash | 205 contrib/completion/git-completion.zsh | 9 ++ t/t9902-completion.sh | 282 + 3 files changed, 430 insertions(+), 66 deletions(-) -- 2.11.0.555.g967c1bcb3
[PATCH 10/12] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
When listing unique remote branches for 'git checkout's tracking DWIMery, __git_refs() runs the classic '... |sort |uniq -u' pattern to filter out duplicate remote branches. Let 'git for-each-ref' do the sorting, sparing the overhead of fork()+exec()ing 'sort' and a stage in the pipeline where potentially relatively large amount of data can be passed between two subsequent pipeline stages. This speeds up refs completion for 'git checkout' a bit when a lot of remote branches match the current word to be completed. Listing a single local and 100k remote branches, all packed, best of five: On Linux, before: $ time __git_complete_refs --track real0m1.856s user0m1.816s sys 0m0.060s After: real0m1.550s user0m1.512s sys 0m0.060s On Windows, before: real0m3.128s user0m2.155s sys 0m0.183s After: real0m2.781s user0m1.826s sys 0m0.136s Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e2c4794f3..9f5a6f44e 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -414,8 +414,9 @@ __git_refs () # Try to find a remote branch that matches the completion word # but only output if the branch name is unique __git for-each-ref --format="%(refname:strip=3)" \ + --sort="refname:strip=3" \ "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \ - sort | uniq -u + uniq -u fi return fi -- 2.11.0.555.g967c1bcb3
[PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
The code listing unique remote branches for 'git checkout's tracking DWIMery outputs only remote branches that match the current word to be completed, but the filtering is done in a shell loop iterating over all remote refs. Let 'git for-each-ref' do the filtering, as it can do so much more efficiently and we can remove that shell loop entirely. This speeds up refs completion for 'git checkout' considerably when there are a lot of non-matching remote refs to be filtered out. Uniquely completing a branch in a repository with 100k remote branches, all packed, best of five: On Linux, before: $ time __git_complete_refs --cur=maste --track real0m1.993s user0m1.740s sys 0m0.304s After: real0m0.266s user0m0.248s sys 0m0.012s On Windows, before: real0m6.187s user0m3.358s sys 0m2.121s After: real0m0.750s user0m0.015s sys 0m0.090s Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8f1203025..e2c4794f3 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -413,15 +413,9 @@ __git_refs () # employ the heuristic used by git checkout # Try to find a remote branch that matches the completion word # but only output if the branch name is unique - local ref entry - __git for-each-ref --shell --format="ref=%(refname:strip=3)" \ - "refs/remotes/" | \ - while read -r entry; do - eval "$entry" - if [[ "$ref" == "$cur_"* ]]; then - echo "$ref" - fi - done | sort | uniq -u + __git for-each-ref --format="%(refname:strip=3)" \ + "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \ + sort | uniq -u fi return fi -- 2.11.0.555.g967c1bcb3
[PATCH 06/12] completion: don't disambiguate short refs
When the completion script lists short refs it does so using the 'git for-each-ref' format 'refname:short', which makes sure that all listed refs are unambiguous. While disambiguating refs is technically correct in this case, as opposed to the cases discussed in the previous patch, this disambiguation involves several stat() syscalls for each ref, thus, unfortunately, comes at a steep cost especially on Windows and/or when there are a lot of refs to be listed. A user of Git for Windows reported[1] 'git checkout ' taking ~11 seconds in a repository with just about 4000 refs. However, it's questionable whether ambiguous refs are really that bad to justify that much extra cost: - Ambiguous refs are not that common, - even if a repository contains ambiguous refs, they only hurt when the user actually happens to want to do something with one of the ambiguous refs, and - the issue can be easily circumvented by renaming those ambiguous refs. - On the other hand, apparently not that many refs are needed to make refs completion unacceptably slow on Windows, - and this slowness bites each and every time the user attempts refs completion, even when the repository doesn't contain any ambiguous refs. - Furthermore, circumventing the issue might not be possible or might be considerably more difficult and requires various trade-offs (e.g. working in a repository with only a few selected important refs while keeping a separate repository with all refs for reference). Arguably, in this case the benefits of technical correctness are rather minor compared to the price we pay for it, and we are better off opting for performance over correctness. Use the 'git for-each-ref' format 'refname:strip=2' to list short refs to spare the substantial cost of disambiguating. This speeds up refs completion considerably. Uniquely completing a branch in a repository with 100k local branches, all packed, best of five: On Linux, before: $ time __git_complete_refs --cur=maste real0m1.662s user0m1.368s sys 0m0.296s After: real0m0.831s user0m0.808s sys 0m0.028s On Windows, before: real0m12.457s user0m1.016s sys 0m0.092s After: real0m1.480s user0m1.031s sys 0m0.060s [1] - https://github.com/git-for-windows/git/issues/524 Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 19799e3ba..d55eadfd1 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -401,7 +401,7 @@ __git_refs () for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do if [ -e "$dir/$i" ]; then echo $pfx$i; fi done - format="refname:short" + format="refname:strip=2" refs="refs/tags refs/heads refs/remotes" ;; esac @@ -412,7 +412,7 @@ __git_refs () # Try to find a remote branch that matches the completion word # but only output if the branch name is unique local ref entry - __git for-each-ref --shell --format="ref=%(refname:short)" \ + __git for-each-ref --shell --format="ref=%(refname:strip=2)" \ "refs/remotes/" | \ while read -r entry; do eval "$entry" @@ -437,7 +437,7 @@ __git_refs () *) if [ "$list_refs_from" = remote ]; then echo "HEAD" - __git for-each-ref --format="%(refname:short)" \ + __git for-each-ref --format="%(refname:strip=2)" \ "refs/remotes/$remote/" | sed -e "s#^$remote/##" else __git ls-remote "$remote" HEAD \ -- 2.11.0.555.g967c1bcb3
[PATCH 12/12] completion: fill COMPREPLY directly when completing refs
__gitcomp_nl() iterates over all the possible completion words it gets
as argument
- filtering matching words,
- appending a trailing space to each matching word (in all but two
cases),
- prepending a prefix to each matching word (when completing words
after e.g. '--option=' or 'master..'), and
- adding each matching word to the COMPREPLY array.
This takes a while when a lot of refs are passed to __gitcomp_nl().
The previous changes in this series ensure that __git_refs() lists
only refs matching the current word to be completed, making a second
filtering in __gitcomp_nl() redundant.
Adding the necessary prefix and suffix could be done in __git_refs()
as well:
- When refs come from 'git for-each-ref', then that prefix and
suffix could be added much more efficiently using a 'git
for-each-ref' format containing said prefix and suffix.
- When refs come from 'git ls-remote', then that prefix and suffix
can be added in the shell loop that has to process 'git
ls-remote's output anyway.
- Finally, the prefix and suffix can be added to that handful of
potentially matching symbolic and pseudo refs right away in the
shell loop listing them.
And then all what is still left to do is to assign a bunch of
newline-separated words to a shell array, which can be done without a
shell loop iterating over each word, basically making all of
__gitcomp_nl() unnecessary for refs completion.
Add the helper function __gitcomp_direct() to fill the COMPREPLY array
with prefiltered and preprocessed words without any additional
processing, without a shell loop, with just one single compound
assignment. Modify __git_refs() to accept prefix and suffix
parameters and add them to each and every listed ref. Modify
__git_complete_refs() to pass the prefix and suffix parameters to
__git_refs() and to feed __git_refs()'s output to __gitcomp_direct()
instead of __gitcomp_nl().
This speeds up refs completion when there are a lot of refs matching
the current word to be completed. Listing all branches for completion
in a repo with 100k local branches, all packed, best of five:
On Linux, near the beginning of this series, for reference:
$ time __git_complete_refs
real0m2.028s
user0m1.692s
sys 0m0.344s
Before this patch:
real0m1.135s
user0m1.112s
sys 0m0.024s
After:
real0m0.367s
user0m0.352s
sys 0m0.020s
On Windows, near the beginning:
real0m13.078s
user0m1.609s
sys 0m0.060s
Before this patch:
real0m2.093s
user0m1.641s
sys 0m0.060s
After:
real0m0.683s
user0m0.203s
sys 0m0.076s
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 54 --
contrib/completion/git-completion.zsh | 9 ++
t/t9902-completion.sh | 16 ++
3 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 0ad02cec6..dbbb62f5f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -213,6 +213,20 @@ _get_comp_words_by_ref ()
}
fi
+# Fills the COMPREPLY array with prefiltered words without any additional
+# processing.
+# Callers must take care of providing only words that match the current word
+# to be completed and adding any prefix and/or suffix (trailing space!), if
+# necessary.
+# 1: List of newline-separated matching completion words, complete with
+#prefix and suffix.
+__gitcomp_direct ()
+{
+ local IFS=$'\n'
+
+ COMPREPLY=($1)
+}
+
__gitcompappend ()
{
local x i=${#COMPREPLY[@]}
@@ -354,17 +368,19 @@ __git_tags ()
#Can be the name of a configured remote, a path, or a URL.
# 2: In addition to local refs, list unique branches from refs/remotes/ for
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
-# 3: Currently ignored.
+# 3: A prefix to be added to each listed ref (optional).
# 4: List only refs matching this word instead of the current word being
-#completed (optional).
+#completed (optional; NOT ignored, if empty, but lists all refs).
+# 5: A suffix to be appended to each listed ref (optional; ignored, if set
+#but empty).
#
# Use __git_complete_refs() instead.
__git_refs ()
{
local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
- local format refs pfx
- local cur_="${4-$cur}"
+ local format refs
+ local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
__git_find_repo_path
dir="$__git_repo_path"
@@ -389,7 +405,7 @@ __git_refs ()
if [ "$list_refs_from" = path ]; then
if [[ "$cur_&quo
[PATCH 03/12] completion: support completing full refs after '--option=refs/'
Completing full refs currently only works when the full ref stands on
in its own on the command line, but doesn't work when the current word
to be completed contains a prefix before the full ref, e.g.
'--option=refs/' or 'master..refs/bis'.
The reason is that __git_refs() looks at the current word to be
completed ($cur) as a whole to decide whether it has to list full (if
it starts with 'refs/') or short refs (otherwise). However, $cur also
holds said '--option=' or 'master..' prefixes, which of course throw
off this decision. Luckily, the default action is to list short refs,
that's why completing short refs happens to work even after a
'master..' prefix and similar cases.
Pass only the ref part of the current word to be completed to
__git_refs() as a new positional parameter, so it can make the right
decision even if the whole current word contains some kind of a
prefix.
Make this new parameter the 4. positional parameter and leave the 3.
as an ignored placeholder for now (it will be used later in this patch
series).
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 21 ++---
t/t9902-completion.sh | 31 +++
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 7f19e2a4f..67a03cfd4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -354,6 +354,8 @@ __git_tags ()
#Can be the name of a configured remote, a path, or a URL.
# 2: In addition to local refs, list unique branches from refs/remotes/ for
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
+# 3: Currently ignored.
+# 4: The current ref to be completed (optional).
#
# Use __git_complete_refs() instead.
__git_refs ()
@@ -361,6 +363,7 @@ __git_refs ()
local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
local format refs pfx
+ local cur_="${4-$cur}"
__git_find_repo_path
dir="$__git_repo_path"
@@ -384,14 +387,17 @@ __git_refs ()
fi
if [ "$list_refs_from" = path ]; then
- case "$cur" in
+ case "$cur_" in
refs|refs/*)
format="refname"
- refs="${cur%/*}"
+ refs="${cur_%/*}"
track=""
;;
*)
- [[ "$cur" == ^* ]] && pfx="^"
+ if [[ "$cur_" == ^* ]]; then
+ pfx="^"
+ cur_=${cur_#^}
+ fi
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
@@ -411,16 +417,16 @@ __git_refs ()
while read -r entry; do
eval "$entry"
ref="${ref#*/}"
- if [[ "$ref" == "$cur"* ]]; then
+ if [[ "$ref" == "$cur_"* ]]; then
echo "$ref"
fi
done | sort | uniq -u
fi
return
fi
- case "$cur" in
+ case "$cur_" in
refs|refs/*)
- __git ls-remote "$remote" "$cur*" | \
+ __git ls-remote "$remote" "$cur_*" | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -475,7 +481,8 @@ __git_complete_refs ()
shift
done
- __gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+ __gitcomp_nl "$(__git_refs "$remote" "$track" "" "$cur_")" \
+ "$pfx" "$cur_" "$sfx"
}
# __git_refs2 requires 1 argument (to pass to __git_refs)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 50c534072..8fe748839 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -775,6 +775,37 @@ test_expect_success '__git_refs - unique remote branches
for git checkout DWIMer
test_cmp expected "$actual"
'
+test_expect_success '__git_refs - after --opt=' '
+ cat >expected <<-EOF &&
+ HEAD
+ mast
[PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
When completing refs, several __git_refs() code paths list all the
refs from the refs/{heads,tags,remotes}/ hierarchy and then
__gitcomp_nl() iterates over those refs in a shell loop to filter out
refs not matching the current word to be completed. This comes with a
considerable performance penalty when a repository contains a lot of
refs but the current word can be uniquely completed or when only a
handful of refs match the current word.
Specify appropriate globbing patterns to 'git for-each-ref' and 'git
ls-remote' to list only those refs that match the current word to be
completed. This reduces the number of iterations in __gitcomp_nl()
from the number of refs to the number of matching refs.
This speeds up refs completion considerably when there are a lot of
non-matching refs to be filtered out. Uniquely completing a branch in
a repository with 100k local branches, all packed, best of five:
On Linux, before:
$ time __git_complete_refs --cur=maste
real0m0.831s
user0m0.808s
sys 0m0.028s
After:
real0m0.119s
user0m0.104s
sys 0m0.008s
On Windows, before:
real0m1.480s
user0m1.031s
sys 0m0.060s
After:
real0m0.377s
user0m0.015s
sys 0m0.030s
Strictly speaking this is a fundamental behavior change in
__git_refs(), a helper function that is likely used in users' custom
completion scriptlets. However, arguably this change should be
harmless, because:
- __git_refs() was only ever intended to feed refs to Bash's
completion machinery, for which non-matching refs have to be
filtered out anyway.
- There were already code paths that list only matching refs
(listing unique remote branches for 'git checkout's tracking
DWIMery and listing full remote refs via 'git ls-remote').
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 14 +++--
t/t9902-completion.sh | 102 +
2 files changed, 111 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index d55eadfd1..615292f8b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -394,7 +394,7 @@ __git_refs ()
case "$cur_" in
refs|refs/*)
format="refname"
- refs="${cur_%/*}"
+ refs=("$cur_*" "$cur_*/**")
track=""
;;
*)
@@ -402,11 +402,13 @@ __git_refs ()
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
format="refname:strip=2"
- refs="refs/tags refs/heads refs/remotes"
+ refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
+ "refs/heads/$cur_*" "refs/heads/$cur_*/**"
+ "refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
;;
esac
__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
- $refs
+ "${refs[@]}"
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the
completion word
@@ -438,10 +440,12 @@ __git_refs ()
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
__git for-each-ref --format="%(refname:strip=2)" \
- "refs/remotes/$remote/" | sed -e "s#^$remote/##"
+ "refs/remotes/$remote/$cur_*" \
+ "refs/remotes/$remote/$cur_*/**" | sed -e
"s#^$remote/##"
else
__git ls-remote "$remote" HEAD \
- "refs/tags/*" "refs/heads/*" "refs/remotes/*" |
+ "refs/tags/$cur_*" "refs/heads/$cur_*" \
+ "refs/remotes/$cur_*" |
while read -r hash i; do
case "$i" in
*^{}) ;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7b42a686c..499be5879 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -837,6 +837,108 @@ test_expect_success '__git refs - exluding full refs' '
test_cmp expected "$act
[PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs
The previous changes in this series ensure that __git_refs() lists
only refs that match the current word to be completed, but
non-matching symbolic and pseudo refs still show up in its output.
Filter out non-matching symbolic and pseudo refs as well, so anything
__git_refs() outputs matches the current word to be completed, as it
will allow us to optimize how refs are placed into the COMPREPLY
array.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 20
t/t9902-completion.sh | 4
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 9f5a6f44e..0ad02cec6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -355,7 +355,8 @@ __git_tags ()
# 2: In addition to local refs, list unique branches from refs/remotes/ for
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
# 3: Currently ignored.
-# 4: The current ref to be completed (optional).
+# 4: List only refs matching this word instead of the current word being
+#completed (optional).
#
# Use __git_complete_refs() instead.
__git_refs ()
@@ -399,7 +400,12 @@ __git_refs ()
;;
*)
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
- if [ -e "$dir/$i" ]; then echo $pfx$i; fi
+ case "$i" in
+ $cur_*) if [ -e "$dir/$i" ]; then
+ echo $pfx$i
+ fi
+ ;;
+ esac
done
format="refname:strip=2"
refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
@@ -432,12 +438,18 @@ __git_refs ()
;;
*)
if [ "$list_refs_from" = remote ]; then
- echo "HEAD"
+ case "HEAD" in
+ $cur_*) echo "HEAD" ;;
+ esac
__git for-each-ref --format="%(refname:strip=3)" \
"refs/remotes/$remote/$cur_*" \
"refs/remotes/$remote/$cur_*/**"
else
- __git ls-remote "$remote" HEAD \
+ local query_symref
+ case "HEAD" in
+ $cur_*) query_symref="HEAD" ;;
+ esac
+ __git ls-remote "$remote" $query_symref \
"refs/tags/$cur_*" "refs/heads/$cur_*" \
"refs/remotes/$cur_*" |
while read -r hash i; do
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 499be5879..5e06acc17 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -847,7 +847,6 @@ test_expect_success 'setup for filtering matching refs' '
test_expect_success '__git_refs - only matching refs' '
cat >expected <<-EOF &&
- HEAD
matching-branch
matching/branch
matching-tag
@@ -874,7 +873,6 @@ test_expect_success '__git_refs - only matching refs - full
refs' '
test_expect_success '__git_refs - only matching refs - remote on local file
system' '
cat >expected <<-EOF &&
- HEAD
master-in-other
matching/branch-in-other
EOF
@@ -887,7 +885,6 @@ test_expect_success '__git_refs - only matching refs -
remote on local file syst
test_expect_success '__git_refs - only matching refs - configured remote' '
cat >expected <<-EOF &&
- HEAD
master-in-other
matching/branch-in-other
EOF
@@ -912,7 +909,6 @@ test_expect_success '__git_refs - only matching refs -
remote - full refs' '
test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
cat >expected <<-EOF &&
- HEAD
matching-branch
matching/branch
matching-tag
--
2.11.0.555.g967c1bcb3
[PATCH 02/12] completion: wrap __git_refs() for better option parsing
__git_refs() currently accepts two optional positional parameters: a
remote and a flag for 'git checkout's tracking DWIMery. To fix a
minor bug, and, more importantly, for faster refs completion, this
series will add three more parameters: a prefix, the current word to
be completed and a suffix, i.e. the options accepted by __gitcomp() &
friends, and will change __git_refs() to list only refs matching that
given current word and to add that given prefix and suffix to the
listed refs.
However, __git_refs() is the helper function that is most likely used
in users' custom completion scriptlets for their own git commands, and
we don't want to break those, so
- we can't change __git_refs()'s default output format, i.e. we
can't by default append a trailing space to every listed ref,
meaning that the suffix parameter containing the default trailing
space would have to be specified on every invocation, and
- we can't change the position of existing positional parameters
either, so there would have to be plenty of set-but-empty
placeholder positional parameters all over the completion script.
Furthermore, with five positional parameters it would be really hard
to remember which position means what.
To keep callsites simple, add the new wrapper function
__git_complete_refs() around __git_refs(), which:
- instead of positional parameters accepts real '--opt=val'-style
options and with minimalistic option parsing translates them to
__git_refs()'s and __gitcomp_nl()'s positional parameters, and
- includes the '__gitcomp_nl "$(__git_refs ...)" ...' command
substitution to make its behavior match its name and the behavior
of other __git_complete_* functions, and to limit future changes
in this series to __git_refs() and this new wrapper function.
Call this wrapper function instead of __git_refs() wherever possible
throughout the completion script, i.e. when __git_refs()'s output is
fed to __gitcomp_nl() right away without further processing, which
means all callsites except a single one in the __git_refs2() helper.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 102 ---
t/t9902-completion.sh | 106 +
2 files changed, 173 insertions(+), 35 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index f20d4600c..7f19e2a4f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -354,6 +354,8 @@ __git_tags ()
#Can be the name of a configured remote, a path, or a URL.
# 2: In addition to local refs, list unique branches from refs/remotes/ for
#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
+#
+# Use __git_complete_refs() instead.
__git_refs ()
{
local i hash dir track="${2-}"
@@ -446,6 +448,36 @@ __git_refs ()
esac
}
+# Completes refs, short and long, local and remote, symbolic and pseudo.
+#
+# Usage: __git_complete_refs []...
+# --remote=: The remote to list refs from, can be the name of a
+#configured remote, a path, or a URL.
+# --track: List unique remote branches for 'git checkout's tracking DWIMery.
+# --pfx=: A prefix to be added to each ref.
+# --cur=: The current ref to be completed. Defaults to the current
+# word to be completed.
+# --sfx=: A suffix to be appended to each ref instead of the default
+# space.
+__git_complete_refs ()
+{
+ local remote track pfx cur_="$cur" sfx=" "
+
+ while test $# != 0; do
+ case "$1" in
+ --remote=*) remote="${1##--remote=}" ;;
+ --track)track="yes" ;;
+ --pfx=*)pfx="${1##--pfx=}" ;;
+ --cur=*)cur_="${1##--cur=}" ;;
+ --sfx=*)sfx="${1##--sfx=}" ;;
+ *) return 1 ;;
+ esac
+ shift
+ done
+
+ __gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+}
+
# __git_refs2 requires 1 argument (to pass to __git_refs)
__git_refs2 ()
{
@@ -554,15 +586,15 @@ __git_complete_revlist_file ()
*...*)
pfx="${cur_%...*}..."
cur_="${cur_#*...}"
- __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+ __git_complete_refs --pfx="$pfx" --cur="$cur_"
;;
*..*)
pfx="${cur_%..*}.."
cur_="${cur_#*..}"
- __gitcomp_nl "$(__git_refs)" "$pfx" &quo
[PATCH 04/12] completion: support excluding full refs
Commit 49416ad22 (completion: support excluding refs, 2016-08-24) made
possible to complete short refs with a '^' prefix.
Extend the support to full refs to make completing '^refs/...' work.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 8
t/t9902-completion.sh | 31 +++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 67a03cfd4..63e803154 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -387,6 +387,10 @@ __git_refs ()
fi
if [ "$list_refs_from" = path ]; then
+ if [[ "$cur_" == ^* ]]; then
+ pfx="^"
+ cur_=${cur_#^}
+ fi
case "$cur_" in
refs|refs/*)
format="refname"
@@ -394,10 +398,6 @@ __git_refs ()
track=""
;;
*)
- if [[ "$cur_" == ^* ]]; then
- pfx="^"
- cur_=${cur_#^}
- fi
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fe748839..7b42a686c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -806,6 +806,37 @@ test_expect_success '__git_refs - after --opt= - full
refs' '
test_cmp expected "$actual"
'
+test_expect_success '__git refs - exluding refs' '
+ cat >expected <<-EOF &&
+ ^HEAD
+ ^master
+ ^matching-branch
+ ^other/branch-in-other
+ ^other/master-in-other
+ ^matching-tag
+ EOF
+ (
+ cur=^ &&
+ __git_refs >"$actual"
+ ) &&
+ test_cmp expected "$actual"
+'
+
+test_expect_success '__git refs - exluding full refs' '
+ cat >expected <<-EOF &&
+ ^refs/heads/master
+ ^refs/heads/matching-branch
+ ^refs/remotes/other/branch-in-other
+ ^refs/remotes/other/master-in-other
+ ^refs/tags/matching-tag
+ EOF
+ (
+ cur=^refs/ &&
+ __git_refs >"$actual"
+ ) &&
+ test_cmp expected "$actual"
+'
+
test_expect_success '__git_complete_refs - simple' '
sed -e "s/Z$//g" >expected <<-EOF &&
HEAD Z
--
2.11.0.555.g967c1bcb3
[PATCH 05/12] completion: don't disambiguate tags and branches
When the completion script has to list only tags or only branches, it
uses the 'git for-each-ref' format 'refname:short', which makes sure
that all listed tags and branches are unambiguous. However,
disambiguating tags and branches in these cases is wrong, because:
- __git_tags(), the helper function listing possible tagname
arguments for 'git tag', lists an ambiguous tag
'refs/tags/ambiguous' as 'tags/ambiguous'. Its only consumer,
'git tag' expects its tagname argument to be under 'refs/tags/',
thus it interprets that abgiguous tag as
'refs/tags/tags/ambiguous'. Clearly wrong.
- __git_heads() lists possible branchname arguments for 'git branch'
and possible 'branch.' configuration subsections.
Both of these expect branchnames to be under 'refs/heads/' and
misinterpret a disambiguated branchname like 'heads/ambiguous'.
Furthermore, disambiguation involves several stat() syscalls for each
tag or branch, thus comes at a steep cost especially on Windows and/or
when there are a lot of tags or branches to be listed.
Use the 'git for-each-ref' format 'refname:strip=2' instead of
'refname:short' to avoid harmful disambiguation of tags and branches
in __git_tags() and __git_heads().
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 63e803154..19799e3ba 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -340,12 +340,12 @@ __git_index_files ()
__git_heads ()
{
- __git for-each-ref --format='%(refname:short)' refs/heads
+ __git for-each-ref --format='%(refname:strip=2)' refs/heads
}
__git_tags ()
{
- __git for-each-ref --format='%(refname:short)' refs/tags
+ __git for-each-ref --format='%(refname:strip=2)' refs/tags
}
# Lists refs from the local (by default) or from a remote repository.
--
2.11.0.555.g967c1bcb3
[PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches
The code listing unique remote branches for 'git checkout's tracking
DWIMery uses a shell parameter expansion in a loop iterating over each
listed ref to remove the remote's name from the remote branches, i.e.
the leading path component from the short ref. When listing refs from
a configured remote repository, '| sed s///' is used for the same
purpose.
Let 'git for-each-ref' strip one more leading path component from the
refs, i.e. use the format 'refname:strip=3' instead of '=2', making
that parameter expansion and 'sed' execution unnecessary.
This speeds up refs completion for 'git checkout'. Uniquely
completing a branch for 'git checkout maste' in a repo with 100k
remote branches, all packed, best of five:
On Linux, near the beginning of this series, for reference:
$ time __git_complete_refs --cur=maste --track
real0m8.185s
user0m6.896s
sys 0m1.616s
Before this patch:
real0m2.714s
user0m2.344s
sys 0m0.436s
After:
real0m1.993s
user0m1.740s
sys 0m0.304s
On Windows, near the beginning:
real1m8.421s
user0m7.591s
sys 0m3.557s
Before this patch:
real0m8.191s
user0m4.638s
sys 0m2.918s
After:
real0m6.187s
user0m3.358s
sys 0m2.121s
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 615292f8b..8f1203025 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -414,11 +414,10 @@ __git_refs ()
# Try to find a remote branch that matches the
completion word
# but only output if the branch name is unique
local ref entry
- __git for-each-ref --shell
--format="ref=%(refname:strip=2)" \
+ __git for-each-ref --shell
--format="ref=%(refname:strip=3)" \
"refs/remotes/" | \
while read -r entry; do
eval "$entry"
- ref="${ref#*/}"
if [[ "$ref" == "$cur_"* ]]; then
echo "$ref"
fi
@@ -439,9 +438,9 @@ __git_refs ()
*)
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
- __git for-each-ref --format="%(refname:strip=2)" \
+ __git for-each-ref --format="%(refname:strip=3)" \
"refs/remotes/$remote/$cur_*" \
- "refs/remotes/$remote/$cur_*/**" | sed -e
"s#^$remote/##"
+ "refs/remotes/$remote/$cur_*/**"
else
__git ls-remote "$remote" HEAD \
"refs/tags/$cur_*" "refs/heads/$cur_*" \
--
2.11.0.555.g967c1bcb3
[PATCH] squash! completion: fill COMPREPLY directly when completing refs
Care should be taken, though, because that prefix might contain
'for-each-ref' format specifiers as part of the left hand side of a
'..' range or '...' symmetric difference notation or fetch/push/etc.
refspec, e.g. 'git log "evil-%(refname)..br'. Doubling every '%'
in the prefix will prevent 'git for-each-ref' from interpolating any
of those contained format specifiers.
---
This is really pathological, and I'm sure this has nothing to do with
whatever breakage Jacob experienced.
The shell metacharacters '(' and ')' still cause us trouble in various
ways, but that's nothing new and has been the case for quite a while
(always?).
It's already incorporated into (the rewritten)
https://github.com/szeder/git completion-refs-speedup
contrib/completion/git-completion.bash | 8 +---
t/t9902-completion.sh | 11 +++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index dbbb62f5f..058f1d0ee 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -381,6 +381,7 @@ __git_refs ()
local list_refs_from=path remote="${1-}"
local format refs
local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
+ local fer_pfx="${pfx//\%/%%}"
__git_find_repo_path
dir="$__git_repo_path"
@@ -406,6 +407,7 @@ __git_refs ()
if [ "$list_refs_from" = path ]; then
if [[ "$cur_" == ^* ]]; then
pfx="$pfx^"
+ fer_pfx="$fer_pfx^"
cur_=${cur_#^}
fi
case "$cur_" in
@@ -429,13 +431,13 @@ __git_refs ()
"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
;;
esac
- __git_dir="$dir" __git for-each-ref
--format="$pfx%($format)$sfx" \
+ __git_dir="$dir" __git for-each-ref
--format="$fer_pfx%($format)$sfx" \
"${refs[@]}"
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the
completion word
# but only output if the branch name is unique
- __git for-each-ref
--format="$pfx%(refname:strip=3)$sfx" \
+ __git for-each-ref
--format="$fer_pfx%(refname:strip=3)$sfx" \
--sort="refname:strip=3" \
"refs/remotes/*/$cur_*"
"refs/remotes/*/$cur_*/**" | \
uniq -u
@@ -457,7 +459,7 @@ __git_refs ()
case "HEAD" in
$cur_*) echo "${pfx}HEAD$sfx" ;;
esac
- __git for-each-ref
--format="$pfx%(refname:strip=3)$sfx" \
+ __git for-each-ref
--format="$fer_pfx%(refname:strip=3)$sfx" \
"refs/remotes/$remote/$cur_*" \
"refs/remotes/$remote/$cur_*/**"
else
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 1206a38ed..be584c069 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -951,6 +951,17 @@ test_expect_success 'teardown after filtering matching
refs' '
git update-ref -d refs/remotes/other/matching/branch-in-other
'
+test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
+ cat >expected <<-EOF &&
+ evil-%%-%42-%(refname)..master
+ EOF
+ (
+ cur="evil-%%-%42-%(refname)..mas" &&
+ __git_refs "" "" "evil-%%-%42-%(refname).." mas >"$actual"
+ ) &&
+ test_cmp expected "$actual"
+'
+
test_expect_success '__git_complete_refs - simple' '
sed -e "s/Z$//g" >expected <<-EOF &&
HEAD Z
--
2.11.1.499.gb6fcc8b3a
Re: [PATCH 00/12] completion: speed up refs completion
On Mon, Feb 6, 2017 at 7:31 PM, Jacob Keller wrote: > On Fri, Feb 3, 2017 at 7:15 PM, Jacob Keller wrote: >> I haven't had a chance to further investigate, but I tried this series >> out (from your github) and it appears that this series (or the >> previous series for __gitdir work) breaks "git log" ref completion. >> I'll have further details when I am able to investigate a it more. >> >> Thanks, >> Jake > > At first I had the same problem, but I verified by re-installing the > completion script and the problem appears to have gone away. I suspect > what happened is that the original time, I forgot to actually install > the new version of git, and only installed the completion script, so > when some of the commands were run with new options they (silently) > failed and the result was missing completion values. > > Once I properly re-installed everything it appears to work as > expected. I haven't found any other issues yet. Thanks, that's good to hear. Still, I'm a bit puzzled as to what exactly might have caused your problem. Considering new options: - the __gitdir()-related series added the 'git rev-parse --absolute-git-dir' option, but only ever used it if you invoked completion after 'git -C some/where'. - The refs completion speedup didn't add any new options but started to use two that it previously didn't: - 'git for-each-ref --sort=' option, but that's with us since the earliest ever 'for-each-ref' version from more than a decade ago... - 'git for-each-ref' format modifier 'strip=2', which was introduced in v2.7.1~15^2 (tag: do not show ambiguous tag names as "tags/foo", 2016-01-25), only about a year ago. Were you using a pre-2.7.1 version when seeing the problems? Gábor
Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
> * sg/completion-refs-speedup (2017-02-06) 13 commits > - squash! completion: fill COMPREPLY directly when completing refs > - completion: fill COMPREPLY directly when completing refs > - completion: list only matching symbolic and pseudorefs when completing refs > - completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery > - completion: let 'for-each-ref' filter remote branches for 'checkout' > DWIMery > - completion: let 'for-each-ref' strip the remote name from remote branches > - completion: let 'for-each-ref' and 'ls-remote' filter matching refs > - completion: don't disambiguate short refs > - completion: don't disambiguate tags and branches > - completion: support excluding full refs > - completion: support completing full refs after '--option=refs/' > - completion: wrap __git_refs() for better option parsing > - completion: remove redundant __gitcomp_nl() options from _git_commit() > (this branch uses sg/completion.) > > Will hold. > This seems to break 9902 when merged to 'pu'. All failing tests fail with the same error: fatal: unrecognized %(refname:strip=2) argument: strip=2 That's because of this topic: > * kn/ref-filter-branch-list (2017-01-31) 20 commits > (merged to 'next' on 2017-01-31 at e7592a5461) > + branch: implement '--format' option > + branch: use ref-filter printing APIs > + branch, tag: use porcelain output > + ref-filter: allow porcelain to translate messages in the output > + ref-filter: add an 'rstrip=' option to atoms which deal with refnames > + ref-filter: modify the 'lstrip=' option to work with negative '' > + ref-filter: Do not abruptly die when using the 'lstrip=' option > + ref-filter: rename the 'strip' option to 'lstrip' And in particular this commit, which, well, does what it's subject says it's doing, thus breaking backwards compatibility. > + ref-filter: make remote_ref_atom_parser() use > refname_atom_parser_internal() > + ref-filter: introduce refname_atom_parser() > + ref-filter: introduce refname_atom_parser_internal() > + ref-filter: make "%(symref)" atom work with the ':short' modifier > + ref-filter: add support for %(upstream:track,nobracket) > + ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams > + ref-filter: introduce format_ref_array_item() > + ref-filter: move get_head_description() from branch.c > + ref-filter: modify "%(objectname:short)" to take length > + ref-filter: implement %(if:equals=) and %(if:notequals=) > + ref-filter: include reference to 'used_atom' within 'atom_value' > + ref-filter: implement %(if), %(then), and %(else) atoms > > The code to list branches in "git branch" has been consolidated > with the more generic ref-filter API. > > Will cook in 'next'.
[PATCH] completion: restore removed line continuating backslash
Recent commit 1cd23e9e0 (completion: don't use __gitdir() for git
commands, 2017-02-03) rewrapped a couple of long lines, and while
doing so it inadvertently removed a '\' from the end of a line, thus
breaking completion for 'git config remote.name.push '.
Signed-off-by: SZEDER Gábor
---
I wanted this to be a fixup!, but then noticed that this series is
already in next, hence the proper commit message.
I see the last What's cooking marks this series as "will merge to
master". That doesn't mean that it will be in v2.12, does it?
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 993085af6..f2b294aac 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2086,7 +2086,7 @@ _git_config ()
remote.*.push)
local remote="${prev#remote.}"
remote="${remote%.push}"
- __gitcomp_nl "$(__git for-each-ref
+ __gitcomp_nl "$(__git for-each-ref \
--format='%(refname):%(refname)' refs/heads)"
return
;;
--
2.11.1.499.ge87add2e7
Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs
On Fri, Feb 10, 2017 at 10:44 PM, Junio C Hamano wrote: > Should I expect a reroll to come, or is this the only fix-up to the > series that begins at <20170203025405.8242-1-szeder@gmail.com>? > > No hurries. Yes, definitely. I found a minor bug in the middle of the series, and haven't quite made up my mind about it and its fix yet. Perhaps in a day or three. Regarding the 'strip' -> 'lstrip' 'for-each-ref' format modifier rename that broke pu: I should keep using 'strip', right? Gábor
Re: [PATCH] completion: complete modified files for checkout with '--'
On Tue, Feb 14, 2017 at 12:33 AM, wrote:
> From: Cornelius Weig
>
> The command line completion for git-checkout bails out when seeing '--'
> as an isolated argument. For git-checkout this signifies the start of a
> list of files which are to be checked out. Checkout of files makes only
> sense for modified files,
No, there is e.g. 'git checkout that-branch this-path', too.
> therefore completion can be a bit smarter:
> Instead of bailing out, offer modified files for completion.
>
> Signed-off-by: Cornelius Weig
> ---
> contrib/completion/git-completion.bash | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 6c6e1c7..d6523fd 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1059,7 +1059,10 @@ _git_bundle ()
>
> _git_checkout ()
> {
> - __git_has_doubledash && return
> + __git_has_doubledash && {
> + __git_complete_index_file "--modified"
> + return
> + }
>
> case "$cur" in
> --conflict=*)
> --
> 2.10.2
>
Re: [PATCH] completion: restore removed line continuating backslash
On Mon, Feb 13, 2017 at 9:45 PM, Junio C Hamano wrote: > SZEDER Gábor writes: > >> Recent commit 1cd23e9e0 (completion: don't use __gitdir() for git >> commands, 2017-02-03) rewrapped a couple of long lines, and while >> doing so it inadvertently removed a '\' from the end of a line, thus >> breaking completion for 'git config remote.name.push '. >> >> Signed-off-by: SZEDER Gábor >> --- >> >> I wanted this to be a fixup!, but then noticed that this series is >> already in next, hence the proper commit message. > > We get "--format=... : command not found"? Yeah, that's the one. > Thanks for a set of sharp eyes. Heh. Sharp?! It took over five minutes to notice after I first got that error... Furthermore, that '\' was already missing in v1, almost a year ago :) >> I see the last What's cooking marks this series as "will merge to >> master". That doesn't mean that it will be in v2.12, does it? > > I actually was hoping that these can go in. Others that can (or > should) wait are marked as "Will cook in 'next'". > > If you feel uncomfortable and want these to cook longer, please tell > me so. Well, it was mainly my surprise that a 20+ patch series arriving so late that it gets queued on top of -rc0 would still make it into the release. However, I have been using this series with only minor modifications for the better part of a year now, so it's unlikely that there will be any big issues with it. Maybe something small, like this missing '\', but we will deal with it when it arises. Gábor
Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
On Tue, Feb 14, 2017 at 10:24 PM, wrote:
> From: Cornelius Weig
>
> Git-checkout completes words starting with '--' as options and other
> words as refs. Even after specifying a ref, further words not starting
> with '--' are completed as refs, which is invalid for git-checkout.
Refs completion is never attempted for words after the disambiguating
double-dash.
Even when refs completion is attempted, if it is unsuccessful, i.e.
there is no ref that matches the current word to be completed, then
Bash falls back to standard filename completion. No refs match
'./'.
Furthermore, Bash performs filename completion on Alt-/ independently
from any completion function.
Granted, none of these will limit to only modified files... But that
might be a good thing, see below.
> This commit ensures that after specifying a ref, further non-option
> words are completed as paths. Four cases are considered:
>
> - If the word contains a ':', do not treat it as reference and use
>regular revlist completion.
> - If no ref is found on the command line, complete non-options as refs
>as before.
> - If the ref is HEAD or @, complete only with modified files because
>checking out unmodified files is a noop.
Here you use "modified" in the 'ls-files --modified' sense, but that
doesn't include modifications already staged in the index, see below.
>This case also applies if no ref is given, but '--' is present.
> - If a ref other than HEAD or @ is found, offer only valid paths from
>that revision.
>
> Note that one corner-case is not covered by the current implementation:
> if a refname contains a ':' and is followed by '--' the completion would
> not recognize the valid refname.
I'm not sure what you mean here. Refnames can't contain ':'.
> Signed-off-by: Cornelius Weig
> ---
> contrib/completion/git-completion.bash | 39
> +++---
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 4ab119d..df46f62 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1068,7 +1068,7 @@ _git_bundle ()
>
> _git_checkout ()
> {
> - __git_has_doubledash && return
> + local i c=2 ref="" seen_double_dash=""
>
> case "$cur" in
> --conflict=*)
> @@ -1081,13 +1081,36 @@ _git_checkout ()
> "
> ;;
> *)
> - # check if --track, --no-track, or --no-guess was specified
> - # if so, disable DWIM mode
> - local flags="--track --no-track --no-guess" track=1
> - if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> - track=''
> - fi
> - __gitcomp_nl "$(__git_refs '' $track)"
> + while [ $c -lt $cword ]; do
> + i="${words[c]}"
> + case "$i" in
> + --) seen_double_dash=1 ;;
> + -*|?*:*) ;;
> + *) ref="$i"; break ;;
I haven't tried it, but this would trigger on e.g. 'git checkout -b
new-feature ', wouldn't it?
> + esac
> + ((c++))
> + done
> +
> + case "$ref,$seen_double_dash,$cur" in
> + ,,*:*)
> + __git_complete_revlist_file
> + ;;
> + ,,*)
> + # check for --track, --no-track, or --no-guess
> + # if so, disable DWIM mode
> + local flags="--track --no-track --no-guess" track=1
> + if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> + track=''
> + fi
> + __gitcomp_nl "$(__git_refs '' $track)"
> + ;;
> + ,1,*|@,*|HEAD,*)
> + __git_complete_index_file "--modified"
What about
$ echo "unintentional change" >>tracked-file && git add -u
$ git rm important-file
$ git checkout HEAD
? It seems it will offer neither 'tracked-file' nor 'important-file',
but I think it should offer both.
We have __git_complete_index_file() for a while now, but only use it
for commands that accept only --options and filenames, e.g. 'add',
'clean', 'rm'. This would be the first case when we would use it for
a command that accept both refs and filenames. Perhaps similar corner
cases and the easy ways to trigger filename completion are why no one
thought it's worth it.
> + ;;
> + *)
> + __git_complete_tree_file "$ref" "$cur"
Well, here you could go all-in, and say that this should complete only
files that are different from the version in $ref, because checking
out files that are still the same is a noop :)
> + ;;
> +
Re: [PATCH] completion: restore removed line continuating backslash
On Wed, Feb 15, 2017 at 3:41 AM, Junio C Hamano wrote: > SZEDER Gábor writes: > >>> If you feel uncomfortable and want these to cook longer, please tell >>> me so. >> >> Well, it was mainly my surprise that a 20+ patch series arriving so >> late that it gets queued on top of -rc0 would still make it into the >> release. > > It all depends on what area the changes are about ;-) Sure. I actually wanted to end that email with something like "and it's in contrib anyway :)", but by the time I got there I forgot about it.
Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
On Tue, Feb 14, 2017 at 10:24 PM, wrote: > + *) > + __git_complete_tree_file "$ref" "$cur" > + ;; There is one more caveat here. Both our __git_complete_index_file() and Bash's builtin filename completion lists matching paths like this: $ git rm contrib/co coccinelle/contacts/ completion/convert-grafts-to-replace-refs.sh i.e. the leading path components are not redundantly repeated. Now, with this patch in this code path the list would look like this: $ git checkout completion-refs-speedup contrib/co contrib/coccinelle/ contrib/completion/ contrib/contacts/ contrib/convert-grafts-to-replace-refs.sh See the difference? I once made a feeble attempt to make completion of the : notation (i.e. what you extracted into __git_complete_tree_file()) look like regular filename completion, but couldn't. Gábor
Re: [PATCH 5/5] versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
Quoting SZEDER Gábor : Quoting SZEDER Gábor : Version sort with prerelease reordering sometimes puts tagnames in the wrong order, when the common part of two compared tagnames ends with the leading character(s) of one or more configured prerelease suffixes. $ git config --get-all versionsort.prereleaseSuffix -beta $ git tag -l --sort=version:refname v2.1.* v2.1.0-beta-2 v2.1.0-beta-3 v2.1.0 v2.1.0-RC1 v2.1.0-RC2 v2.1.0-beta-1 v2.1.1 v2.1.2 The reason is that when comparing a pair of tagnames, first versioncmp() looks for the first different character in a pair of tagnames, and then the swap_prereleases() helper function checks for prerelease suffixes _starting at_ that character. Thus, when in the above example the sorting algorithm happens to compare the tagnames "v2.1.0-beta-1" and "v2.1.0-RC2", swap_prereleases() will try to match the suffix "-beta" against "beta-1" to no avail, and the two tagnames erroneously end up being ordered lexicographically. To fix this issue change swap_prereleases() to look for configured prerelease suffixes containing that first different character. Now, while I believe this is the right thing to do to fix this bug, there is a corner case, where multiple configured prerelease suffixes might match the same tagname: $ git config --get-all versionsort.prereleaseSuffix -bar -baz -foo-bar $ ~/src/git/git tag -l --sort=version:refname v1.0-foo-bar v1.0-foo-baz I.e. when comparing these two tags, both "-bar" and "-foo-bar" would match "v1.0-foo-bar", and as "-bar" comes first in the config file, it wins, and "v1.0-foo-bar" is ordered first. An argument could be made to prefer longer matches, in which case "v1.0-foo-bar" would be ordered according to "-foo-bar", i.e. as second. However, I don't know what that argument could be, to me neither behavior is better than the other, but the implementation of the "longest match counts" would certainly be more complicated. The argument I would make is that this is a pathological corner case that doesn't worth worrying about. After having slept on this a couple of times, I think the longest matching prerelease suffix should determine the sorting order. A release tag usually consists of an optional prefix, e.g. 'v' or 'snapshot-', followed by the actual version number, followed by an optional suffix. In the contrived example quoted above this suffix is '-foo-bar', thus it feels wrong to match '-bar' against it, when the user explicitly configured '-foo-bar' as prerelease suffix as well. Then here is a more realistic case for sorting based on the longest matching suffix, where - a longer suffix starts with the shorter one, - and the longer suffix comes after the shorter one in the configuration. With my patches it looks like this: $ git -c versionsort.prereleasesuffix=-pre \ -c versionsort.prereleasesuffix=-prerelease \ tag -l --sort=version:refname v1.0.0-prerelease1 v1.0.0-pre1 v1.0.0-pre2 v1.0.0 Yeah, having both '-pre' and '-prerelease' suffixes seems somewhat silly, but who knows what similar but more reasonable prerelease suffixes e.g. non-English speaking users might use in their native language. Anyway, my intuition says that any '-prereleaseX' tags should come after all the '-preX' tags. (Ironically, current git just happens to get this particular case right.) My patches get this wrong, because they look for prerelease suffixes _containing_ the first different character. However, when a '-preX' and a '-prereleaseX' tag are compared, then the whole '-pre' suffix is part of the common part, thus it doesn't match, only '-prerelease' matches. So, to sort this case right the implementation should - look for a prerelease suffix containing the first different character or ending right before the first different character, (This means that when comparing 'v1.0.0-pre1' and 'v1.0.0-pre2' swap_prereleases() would match '-pre' in both: no big deal, it should return "undecided" and let the caller do the right thing by sorting based on '1' and '2') - and sort a tag based on the longest matching prerelease suffix. (In my quoted email above I alluded that its implementation must be more complicated. No, it turns out that it actually isn't.) (Just for the record: it's still not 100% foolproof, though. Someone asking for trouble might use letters instead of numbers to indicate subsequent prereleases, and might have tags 'v1.0-prea', 'v1.0-preb', ..., 'v1.0-prer', and 'v1.0-prerelease'. In this case the sorting algorithm might happen to decide to compare 'v1.0-prer
Re: [PATCH 5/5] versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
Quoting Junio C Hamano : SZEDER Gábor writes: And a final sidenote: sorting based on the longest matching suffix also allows us to (ab)use version sort with prerelease suffixes to sort postrelease tags as we please, too: $ ~/src/git/git -c versionsort.prereleasesuffix=-alpha \ -c versionsort.prereleasesuffix=-beta \ -c versionsort.prereleasesuffix= \ -c versionsort.prereleasesuffix=-gamma \ -c versionsort.prereleasesuffix=-delta \ tag -l --sort=version:refname 'v3.0*' v3.0-alpha1 v3.0-beta1 v3.0 v3.0-gamma1 v3.0-delta1 Assuming that gamma and delta are meant to indicate "these are post-release updates", Indeed they were meant as post-release suffixes. Naturally following alpha and beta, they were the first to spring to mind that should be sorted in non-lexicographical order, so I could show of postrelease reordering. It's just that we don't have a config like 'versionsort.postreleasesuffix', which is half the abuse. The other half of the abuse is that I had to explicitly indicate the position of suffixless versions with an empty suffix between pre- and postrelease suffixes. The empty suffix matches on every tag, but then it's overridden by all configured suffixes, so such a version just stays in the middle. I think a mechanism that can yield the above result is fantastic ;-) Heh. Gut feeling tells me that I should take this as a subtle encouragement to look into adding 'versionsort.postreleasesuffix', shouldn't I ;)
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Quoting Duy Nguyen : On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski wrote: W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: On Fri, 7 Oct 2016, Jakub Narębski wrote: Note that we would have to teach git completion about new syntax; or new configuration variable if we go that route. Why would we? Git's completion does not expand aliases, it only completes the aliases' names, not their values. Because Git completion finds out which _options_ and _arguments_ to complete for an alias based on its expansion. Yes, this is nice bit of trickery... It's c63437c (bash: improve aliased command recognition - 2010-02-23) isn't it? This may favor j6t's approach [1] because retrieving alias attributes is much easier. [1] https://public-inbox.org/git/20161006190720.4ecf3ptl6mszt...@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4 -- Duy The completion script is concerned in three ways: 1. it has to get the names of all aliases, to offer them along with git commands for 'git ' or 'git help ', 2. it has to get the command executed in place of the alias, and 3. strip everything that can't be a git command, so it can offer the right options for the aliased command. The '!!' syntax is the easiest, I think it will just work even with the current completion code, no changes necessary. The '(nocd)' form is still easy, we only have to add this '(nocd)' to that list of stripped words for (3), but no further changes necessary for (1) and (2). 'alias.d2u.command' is tricky. Both (1) and (2) require adjustments, preferably in a way that doesn't involve additional git processes, at least for (1), as it is executed often, for every 'git ', for the sake of users on platforms with not particularly stellar fork()+exec() performance. I think it would be possible to have only one 'git config --get-regexp' and a little bit of post processing in each case, but I didn't think too hard about possible pitfalls, especially when dealing with ambiguity when both 'alias.d2u' and 'alias.d2u.command' are set. And I won't think any more about it until a conclusion is reached that we'll go this route :)
Re: [Bug?] git notes are not copied during rebase
> I am currently a heavy user of rebasing and noticed that my notes > don't get correctly applied, even if notes.rewrite.rebase is set > explicitly to true (though manual says that is the default). Setting 'notes.rewrite.rebase' is, as you mentioned, not necessary, but not sufficient either. See here, especially the second paragraph: notes.rewriteRef When copying notes during a rewrite, specifies the (fully qualified) ref whose notes should be copied. May be a glob, in which case notes in all matching refs will be copied. You may also specify this configuration several times. Does not have a default value; you must configure this variable to enable note rewriting. Can be overridden with the GIT_NOTES_REWRITE_REF environment variable. Gábor
