* [PATCH/RFC] ref-filter: support sorting case-insensitively
@ 2016-11-17 10:21 Nguyễn Thái Ngọc Duy
2016-11-30 12:35 ` [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering Nguyễn Thái Ngọc Duy
0 siblings, 1 reply; 5+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-11-17 10:21 UTC (permalink / raw)
To: git; +Cc: karthik.188, Nguyễn Thái Ngọc Duy
Similar to version:refname sorting refs by versions, icase:refname
will sort by refnames are usually, but strcasecmp will be used instead
of strcmp. This may be helpful sometimes when people name their
branches <group>-<details> but somebody names it <group>-, some goes
with <Group>-, or even <GROUP>-
Syntax is a big problem. This patch does not support
icase:version:refname or version:icase:refname, for example. If
version sorting learns about this thing, I think I prefer
iversion:refname...
Or perhaps we can. I'm losing touch with for-each-ref "pretty"
formats, I'm not quite sure what's the guideline here.
Another option is just use a symbol, like '-' or '*' to mark
case-insensitivity. But that does not look very descriptive. I don't
see any symbol suggesting this case stuff.
What do you think?
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-for-each-ref.txt | 3 ++-
ref-filter.c | 8 ++++++--
ref-filter.h | 1 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69bc83..e41005cf0e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -171,7 +171,8 @@ For sorting purposes, fields with numeric values sort in numeric order
All other fields are used to sort in their byte-value order.
There is also an option to sort by versions, this can be done by using
-the fieldname `version:refname` or its alias `v:refname`.
+the fieldname `version:refname` or its alias `v:refname`. Prefixing
+"icase:" (e.g. `icase:refname`) makes sorting case-insensitive.
In any case, a field name that refers to a field inapplicable to
the object referred by the ref does not cause an error. It
diff --git a/ref-filter.c b/ref-filter.c
index d4c2931f3a..fd63b9c710 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1542,12 +1542,12 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
if (s->version)
cmp = versioncmp(va->s, vb->s);
else if (cmp_type == FIELD_STR)
- cmp = strcmp(va->s, vb->s);
+ cmp = s->strcmp(va->s, vb->s);
else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
- cmp = strcmp(a->refname, b->refname);
+ cmp = s->strcmp(a->refname, b->refname);
else
cmp = 1;
}
@@ -1646,6 +1646,7 @@ struct ref_sorting *ref_default_sorting(void)
sorting->next = NULL;
sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+ sorting->strcmp = strcmp;
return sorting;
}
@@ -1660,6 +1661,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
s = xcalloc(1, sizeof(*s));
s->next = *sorting_tail;
+ s->strcmp = strcmp;
*sorting_tail = s;
if (*arg == '-') {
@@ -1669,6 +1671,8 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
if (skip_prefix(arg, "version:", &arg) ||
skip_prefix(arg, "v:", &arg))
s->version = 1;
+ else if (skip_prefix(arg, "icase:", &arg))
+ s->strcmp = strcasecmp;
len = strlen(arg);
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e2cc..ea2db565f1 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -30,6 +30,7 @@ struct ref_sorting {
int atom; /* index into used_atom array (internal) */
unsigned reverse : 1,
version : 1;
+ int (*strcmp)(const char *, const char *);
};
struct ref_array_item {
--
2.11.0.rc0.161.g80d5b92
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
2016-11-17 10:21 [PATCH/RFC] ref-filter: support sorting case-insensitively Nguyễn Thái Ngọc Duy
@ 2016-11-30 12:35 ` Nguyễn Thái Ngọc Duy
2016-11-30 21:21 ` Junio C Hamano
2016-12-04 2:52 ` Nguyễn Thái Ngọc Duy
0 siblings, 2 replies; 5+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-11-30 12:35 UTC (permalink / raw)
To: git; +Cc: karthik.188, Nguyễn Thái Ngọc Duy
This option makes sorting ignore case, which is great when you have
branches named bug-12-do-something, Bug-12-do-some-more and
BUG-12-do-what and want to group them together. Sorting externally may
not be an option because we lose coloring and column layout from
git-branch and git-tag.
The same could be said for filtering, but it's probably less important
because you can always go with the ugly pattern [bB][uU][gG]-* if you're
desperate.
You can't have case-sensitive filtering and case-insensitive sorting (or
the other way around) with this though. But who would want that?
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2 has a different approach, and I think it's a better one even with
that unanswered question above.
Documentation/git-branch.txt | 4 ++++
Documentation/git-for-each-ref.txt | 3 +++
Documentation/git-tag.txt | 4 ++++
builtin/branch.c | 23 ++++++++++++++---------
builtin/for-each-ref.c | 5 ++++-
builtin/tag.c | 4 ++++
ref-filter.c | 28 +++++++++++++++++++++-------
ref-filter.h | 2 ++
t/t3203-branch-output.sh | 22 ++++++++++++++++++++++
t/t7004-tag.sh | 20 ++++++++++++++++++++
10 files changed, 98 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1fe7344..5516a47 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -118,6 +118,10 @@ OPTIONS
default to color output.
Same as `--color=never`.
+-i::
+--ignore-case::
+ Sorting and filtering branches are case insensitive.
+
--column[=<options>]::
--no-column::
Display branch listing in columns. See configuration variable
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69b..6d22974 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -79,6 +79,9 @@ OPTIONS
Only list refs which contain the specified commit (HEAD if not
specified).
+--ignore-case::
+ Sorting and filtering refs are case insensitive.
+
FIELD NAMES
-----------
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 80019c5..76cfe40 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -108,6 +108,10 @@ OPTIONS
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
+-i::
+--ignore-case::
+ Sorting and filtering tags are case insensitive.
+
--column[=<options>]::
--no-column::
Display tag listing in columns. See configuration variable
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8..36e0a21 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -512,15 +512,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
- /*
- * If no sorting parameter is given then we default to sorting
- * by 'refname'. This would give us an alphabetically sorted
- * array with the 'HEAD' ref at the beginning followed by
- * local branches 'refs/heads/...' and finally remote-tacking
- * branches 'refs/remotes/...'.
- */
- if (!sorting)
- sorting = ref_default_sorting();
ref_array_sort(sorting, &array);
for (i = 0; i < array.nr; i++)
@@ -645,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *new_upstream = NULL;
enum branch_track track;
struct ref_filter filter;
+ int icase = 0;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
struct option options[] = {
@@ -686,6 +678,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only branches of the object"), 0, parse_opt_object_name
},
+ OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END(),
};
@@ -723,6 +716,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (filter.abbrev == -1)
filter.abbrev = DEFAULT_ABBREV;
+ filter.ignore_case = icase;
+
finalize_colopts(&colopts, -1);
if (filter.verbose) {
if (explicitly_enable_column(colopts))
@@ -744,6 +739,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
+ /*
+ * If no sorting parameter is given then we default to sorting
+ * by 'refname'. This would give us an alphabetically sorted
+ * array with the 'HEAD' ref at the beginning followed by
+ * local branches 'refs/heads/...' and finally remote-tacking
+ * branches 'refs/remotes/...'.
+ */
+ if (!sorting)
+ sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
print_ref_list(&filter, sorting);
print_columns(&output, colopts, NULL);
string_list_clear(&output, 0);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4e9f6c2..df41fa0 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
int i;
const char *format = "%(objectname) %(objecttype)\t%(refname)";
struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
- int maxcount = 0, quote_style = 0;
+ int maxcount = 0, quote_style = 0, icase = 0;
struct ref_array array;
struct ref_filter filter;
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_MERGED(&filter, N_("print only refs that are merged")),
OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+ OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END(),
};
@@ -63,6 +64,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
if (!sorting)
sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
+ filter.ignore_case = icase;
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..73df728 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -335,6 +335,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
const char *format = NULL;
+ int icase = 0;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -370,6 +371,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
N_("print only tags of the object"), 0, parse_opt_object_name
},
OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")),
+ OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END()
};
@@ -401,6 +403,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
}
if (!sorting)
sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
+ filter.ignore_case = icase;
if (cmdmode == 'l') {
int ret;
if (column_active(colopts)) {
diff --git a/ref-filter.c b/ref-filter.c
index f5f7a70..bd98010 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1231,8 +1231,14 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
* matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
* matches "refs/heads/mas*", too).
*/
-static int match_pattern(const char **patterns, const char *refname)
+static int match_pattern(const struct ref_filter *filter, const char *refname)
{
+ const char **patterns = filter->name_patterns;
+ unsigned flags = 0;
+
+ if (filter->ignore_case)
+ flags |= WM_CASEFOLD;
+
/*
* When no '--format' option is given we need to skip the prefix
* for matching refs of tags and branches.
@@ -1243,7 +1249,7 @@ static int match_pattern(const char **patterns, const char *refname)
skip_prefix(refname, "refs/", &refname));
for (; *patterns; patterns++) {
- if (!wildmatch(*patterns, refname, 0, NULL))
+ if (!wildmatch(*patterns, refname, flags, NULL))
return 1;
}
return 0;
@@ -1255,9 +1261,15 @@ static int match_pattern(const char **patterns, const char *refname)
* matches a pattern "refs/heads/" but not "refs/heads/m") or a
* wildcard (e.g. the same ref matches "refs/heads/m*", too).
*/
-static int match_name_as_path(const char **pattern, const char *refname)
+static int match_name_as_path(const struct ref_filter *filter, const char *refname)
{
+ const char **pattern = filter->name_patterns;
int namelen = strlen(refname);
+ unsigned flags = WM_PATHNAME;
+
+ if (filter->ignore_case)
+ flags |= WM_CASEFOLD;
+
for (; *pattern; pattern++) {
const char *p = *pattern;
int plen = strlen(p);
@@ -1280,8 +1292,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
if (!*filter->name_patterns)
return 1; /* No pattern always matches */
if (filter->match_as_path)
- return match_name_as_path(filter->name_patterns, refname);
- return match_pattern(filter->name_patterns, refname);
+ return match_name_as_path(filter, refname);
+ return match_pattern(filter, refname);
}
/*
@@ -1536,18 +1548,20 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
struct atom_value *va, *vb;
int cmp;
cmp_type cmp_type = used_atom[s->atom].type;
+ int (*cmp_fn)(const char *, const char *);
get_ref_atom_value(a, s->atom, &va);
get_ref_atom_value(b, s->atom, &vb);
+ cmp_fn = s->ignore_case ? strcasecmp : strcmp;
if (s->version)
cmp = versioncmp(va->s, vb->s);
else if (cmp_type == FIELD_STR)
- cmp = strcmp(va->s, vb->s);
+ cmp = cmp_fn(va->s, vb->s);
else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
- cmp = strcmp(a->refname, b->refname);
+ cmp = cmp_fn(a->refname, b->refname);
else
cmp = 1;
}
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..fc55fa3 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -29,6 +29,7 @@ struct ref_sorting {
struct ref_sorting *next;
int atom; /* index into used_atom array (internal) */
unsigned reverse : 1,
+ ignore_case : 1,
version : 1;
};
@@ -62,6 +63,7 @@ struct ref_filter {
unsigned int with_commit_tag_algo : 1,
match_as_path : 1,
+ ignore_case : 1,
detached : 1;
unsigned int kind,
lines;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index c6a3ccb..fad79e8 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -89,6 +89,11 @@ test_expect_success 'git branch --list -v pattern shows branch summaries' '
awk "{print \$NF}" <tmp >actual &&
test_cmp expect actual
'
+test_expect_success 'git branch --ignore-case --list -v pattern shows branch summaries' '
+ git branch --list --ignore-case -v BRANCH* >tmp &&
+ awk "{print \$NF}" <tmp >actual &&
+ test_cmp expect actual
+'
test_expect_success 'git branch -v pattern does not show branch summaries' '
test_must_fail git branch -v branch*
@@ -196,4 +201,21 @@ test_expect_success 'local-branch symrefs shortened properly' '
test_cmp expect actual
'
+test_expect_success 'sort branches, ignore case' '
+ (
+ git init sort-icase &&
+ cd sort-icase &&
+ test_commit initial &&
+ git branch branch-one &&
+ git branch BRANCH-two &&
+ git branch --list -i | awk "{print \$NF}" >actual &&
+ cat >expected <<-\EOF &&
+ branch-one
+ BRANCH-two
+ master
+ EOF
+ test_cmp expected actual
+ )
+'
+
test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..2d9cae3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -27,6 +27,23 @@ test_expect_success 'listing all tags in an empty tree should output nothing' '
test $(git tag | wc -l) -eq 0
'
+test_expect_success 'sort tags, ignore case' '
+ (
+ git init sort &&
+ cd sort &&
+ test_commit initial &&
+ git tag tag-one &&
+ git tag TAG-two &&
+ git tag -l -i >actual &&
+ cat >expected <<-\EOF &&
+ initial
+ tag-one
+ TAG-two
+ EOF
+ test_cmp expected actual
+ )
+'
+
test_expect_success 'looking for a tag in an empty tree should fail' \
'! (tag_exists mytag)'
@@ -81,6 +98,9 @@ test_expect_success 'listing all tags if one exists should output that tag' '
test_expect_success 'listing a tag using a matching pattern should succeed' \
'git tag -l mytag'
+test_expect_success 'listing a tag using a matching pattern should succeed' \
+ 'git tag -l --ignore-case MYTAG'
+
test_expect_success \
'listing a tag using a matching pattern should output that tag' \
'test $(git tag -l mytag) = mytag'
--
2.8.2.524.g6ff3d78
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
2016-11-30 12:35 ` [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering Nguyễn Thái Ngọc Duy
@ 2016-11-30 21:21 ` Junio C Hamano
2016-12-04 2:52 ` Nguyễn Thái Ngọc Duy
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2016-11-30 21:21 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, karthik.188
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This option makes sorting ignore case, which is great when you have
> branches named bug-12-do-something, Bug-12-do-some-more and
> BUG-12-do-what and want to group them together. Sorting externally may
> not be an option because we lose coloring and column layout from
> git-branch and git-tag.
>
> The same could be said for filtering, but it's probably less important
> because you can always go with the ugly pattern [bB][uU][gG]-* if you're
> desperate.
But of course --ignore-case is of course much easier.
> You can't have case-sensitive filtering and case-insensitive sorting (or
> the other way around) with this though. But who would want that?
I do not feel uncomfortable declaring that the answer to that
question is "nobody" ;-)
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> v2 has a different approach, and I think it's a better one even with
> that unanswered question above.
Yeah, I think this would be easier to use.
> @@ -512,15 +512,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
> if (filter->verbose)
> maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
>
> - /*
> - * If no sorting parameter is given then we default to sorting
> - * by 'refname'. This would give us an alphabetically sorted
> - * array with the 'HEAD' ref at the beginning followed by
> - * local branches 'refs/heads/...' and finally remote-tacking
> - * branches 'refs/remotes/...'.
> - */
> - if (!sorting)
> - sorting = ref_default_sorting();
So it is now a BUG() to give sorting==NULL to this function, which
is OK and I do not think we even need an assert() for it (i.e. the
code with the patch looks good).
> @@ -744,6 +739,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
> filter.kind |= FILTER_REFS_DETACHED_HEAD;
> filter.name_patterns = argv;
> + /*
> + * If no sorting parameter is given then we default to sorting
> + * by 'refname'. This would give us an alphabetically sorted
> + * array with the 'HEAD' ref at the beginning followed by
> + * local branches 'refs/heads/...' and finally remote-tacking
> + * branches 'refs/remotes/...'.
> + */
> + if (!sorting)
> + sorting = ref_default_sorting();
> + sorting->ignore_case = icase;
> print_ref_list(&filter, sorting);
> print_columns(&output, colopts, NULL);
> string_list_clear(&output, 0);
... and the fallback is moved to the caller, which makes sense.
> diff --git a/ref-filter.c b/ref-filter.c
> index f5f7a70..bd98010 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1231,8 +1231,14 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
> * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
> * matches "refs/heads/mas*", too).
> */
> -static int match_pattern(const char **patterns, const char *refname)
> +static int match_pattern(const struct ref_filter *filter, const char *refname)
> {
> + const char **patterns = filter->name_patterns;
> + unsigned flags = 0;
> +
> + if (filter->ignore_case)
> + flags |= WM_CASEFOLD;
> +
Ahh, OK. My reading stuttered when seeing "sorting and filtering"
in the option description for "git tag", but this makes perfect
sense. Good job.
> @@ -1255,9 +1261,15 @@ static int match_pattern(const char **patterns, const char *refname)
> * matches a pattern "refs/heads/" but not "refs/heads/m") or a
> * wildcard (e.g. the same ref matches "refs/heads/m*", too).
> */
> -static int match_name_as_path(const char **pattern, const char *refname)
> +static int match_name_as_path(const struct ref_filter *filter, const char *refname)
> {
> + const char **pattern = filter->name_patterns;
> int namelen = strlen(refname);
> + unsigned flags = WM_PATHNAME;
> +
> + if (filter->ignore_case)
> + flags |= WM_CASEFOLD;
> +
Likewise. Very simple and nicely done.
> @@ -1536,18 +1548,20 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
> struct atom_value *va, *vb;
> int cmp;
> cmp_type cmp_type = used_atom[s->atom].type;
> + int (*cmp_fn)(const char *, const char *);
>
> get_ref_atom_value(a, s->atom, &va);
> get_ref_atom_value(b, s->atom, &vb);
> + cmp_fn = s->ignore_case ? strcasecmp : strcmp;
> if (s->version)
> cmp = versioncmp(va->s, vb->s);
> else if (cmp_type == FIELD_STR)
> - cmp = strcmp(va->s, vb->s);
> + cmp = cmp_fn(va->s, vb->s);
> else {
> if (va->ul < vb->ul)
> cmp = -1;
> else if (va->ul == vb->ul)
> - cmp = strcmp(a->refname, b->refname);
> + cmp = cmp_fn(a->refname, b->refname);
> else
> cmp = 1;
> }
OK.
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index c6a3ccb..fad79e8 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -89,6 +89,11 @@ test_expect_success 'git branch --list -v pattern shows branch summaries' '
> awk "{print \$NF}" <tmp >actual &&
> test_cmp expect actual
> '
> +test_expect_success 'git branch --ignore-case --list -v pattern shows branch summaries' '
> + git branch --list --ignore-case -v BRANCH* >tmp &&
> + awk "{print \$NF}" <tmp >actual &&
> + test_cmp expect actual
> +'
The way the test ensures it found only branch-one and branch-two
looks very sloppy, but that was inherited from the existing one
before this new one, so I'll let it pass.
> @@ -196,4 +201,21 @@ test_expect_success 'local-branch symrefs shortened properly' '
> test_cmp expect actual
> '
>
> +test_expect_success 'sort branches, ignore case' '
> + (
> + git init sort-icase &&
> + cd sort-icase &&
> + test_commit initial &&
> + git branch branch-one &&
> + git branch BRANCH-two &&
> + git branch --list -i | awk "{print \$NF}" >actual &&
> + cat >expected <<-\EOF &&
> + branch-one
> + BRANCH-two
> + master
> + EOF
> + test_cmp expected actual
> + )
> +'
Is there an existing test that uses refs with mixed cases, i.e. the
result of listing sorts differently with and without the -i option?
If not, this one should test output from both cases to ensure that
the command run without -i stays case sensitive.
> test_done
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 8b0f71a..2d9cae3 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -27,6 +27,23 @@ test_expect_success 'listing all tags in an empty tree should output nothing' '
> test $(git tag | wc -l) -eq 0
> '
>
> +test_expect_success 'sort tags, ignore case' '
> + (
> + git init sort &&
> + cd sort &&
> + test_commit initial &&
> + git tag tag-one &&
> + git tag TAG-two &&
> + git tag -l -i >actual &&
> + cat >expected <<-\EOF &&
> + initial
> + tag-one
> + TAG-two
> + EOF
> + test_cmp expected actual
> + )
> +'
Ditto.
> test_expect_success 'looking for a tag in an empty tree should fail' \
> '! (tag_exists mytag)'
>
> @@ -81,6 +98,9 @@ test_expect_success 'listing all tags if one exists should output that tag' '
> test_expect_success 'listing a tag using a matching pattern should succeed' \
> 'git tag -l mytag'
>
> +test_expect_success 'listing a tag using a matching pattern should succeed' \
> + 'git tag -l --ignore-case MYTAG'
The existing one before this one merely says that "git tag -l" must
exit with 0 status code, no?
IOW, even "git tag -l no-such-tag-anywhere && echo OK" yields OK.
So there is not much point replicating it with "-i", unless you want
to say that "git tag -i -l" also must exit with 0 status code.
> test_expect_success \
> 'listing a tag using a matching pattern should output that tag' \
> 'test $(git tag -l mytag) = mytag'
I think the new one would want to mimic this one instead. Look for
MYTAG with the -i option and see it output mytag (in lowercase).
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
2016-11-30 12:35 ` [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering Nguyễn Thái Ngọc Duy
2016-11-30 21:21 ` Junio C Hamano
@ 2016-12-04 2:52 ` Nguyễn Thái Ngọc Duy
2016-12-06 21:11 ` Junio C Hamano
1 sibling, 1 reply; 5+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-12-04 2:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, karthik.188,
Nguyễn Thái Ngọc Duy
This options makes sorting ignore case, which is great when you have
branches named bug-12-do-something, Bug-12-do-some-more and
BUG-12-do-what and want to group them together. Sorting externally may
not be an option because we lose coloring and column layout from
git-branch and git-tag.
The same could be said for filtering, but it's probably less important
because you can always go with the ugly pattern [bB][uU][gG]-* if you're
desperate.
You can't have case-sensitive filtering and case-insensitive sorting (or
the other way around) with this though. For branch and tag, that should
be no problem. for-each-ref, as a plumbing, might want finer control.
But we can always add --{filter,sort}-ignore-case when there is a need
for it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Changes are in tests only:
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index fad79e8..52283df 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -208,6 +208,13 @@ test_expect_success 'sort branches, ignore case' '
test_commit initial &&
git branch branch-one &&
git branch BRANCH-two &&
+ git branch --list | awk "{print \$NF}" >actual &&
+ cat >expected <<-\EOF &&
+ BRANCH-two
+ branch-one
+ master
+ EOF
+ test_cmp expected actual &&
git branch --list -i | awk "{print \$NF}" >actual &&
cat >expected <<-\EOF &&
branch-one
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 2d9cae3..07869b0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -34,6 +34,13 @@ test_expect_success 'sort tags, ignore case' '
test_commit initial &&
git tag tag-one &&
git tag TAG-two &&
+ git tag -l >actual &&
+ cat >expected <<-\EOF &&
+ TAG-two
+ initial
+ tag-one
+ EOF
+ test_cmp expected actual &&
git tag -l -i >actual &&
cat >expected <<-\EOF &&
initial
@@ -98,8 +105,8 @@ test_expect_success 'listing all tags if one exists should output that tag' '
test_expect_success 'listing a tag using a matching pattern should succeed' \
'git tag -l mytag'
-test_expect_success 'listing a tag using a matching pattern should succeed' \
- 'git tag -l --ignore-case MYTAG'
+test_expect_success 'listing a tag with --ignore-case' \
+ 'test $(git tag -l --ignore-case MYTAG) = mytag'
test_expect_success \
'listing a tag using a matching pattern should output that tag' \
Documentation/git-branch.txt | 4 ++++
Documentation/git-for-each-ref.txt | 3 +++
Documentation/git-tag.txt | 4 ++++
builtin/branch.c | 23 ++++++++++++++---------
builtin/for-each-ref.c | 5 ++++-
builtin/tag.c | 4 ++++
ref-filter.c | 28 +++++++++++++++++++++-------
ref-filter.h | 2 ++
t/t3203-branch-output.sh | 29 +++++++++++++++++++++++++++++
t/t7004-tag.sh | 27 +++++++++++++++++++++++++++
10 files changed, 112 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1fe7344..5516a47 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -118,6 +118,10 @@ OPTIONS
default to color output.
Same as `--color=never`.
+-i::
+--ignore-case::
+ Sorting and filtering branches are case insensitive.
+
--column[=<options>]::
--no-column::
Display branch listing in columns. See configuration variable
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69b..6d22974 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -79,6 +79,9 @@ OPTIONS
Only list refs which contain the specified commit (HEAD if not
specified).
+--ignore-case::
+ Sorting and filtering refs are case insensitive.
+
FIELD NAMES
-----------
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 80019c5..76cfe40 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -108,6 +108,10 @@ OPTIONS
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
+-i::
+--ignore-case::
+ Sorting and filtering tags are case insensitive.
+
--column[=<options>]::
--no-column::
Display tag listing in columns. See configuration variable
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8..36e0a21 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -512,15 +512,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
- /*
- * If no sorting parameter is given then we default to sorting
- * by 'refname'. This would give us an alphabetically sorted
- * array with the 'HEAD' ref at the beginning followed by
- * local branches 'refs/heads/...' and finally remote-tacking
- * branches 'refs/remotes/...'.
- */
- if (!sorting)
- sorting = ref_default_sorting();
ref_array_sort(sorting, &array);
for (i = 0; i < array.nr; i++)
@@ -645,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *new_upstream = NULL;
enum branch_track track;
struct ref_filter filter;
+ int icase = 0;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
struct option options[] = {
@@ -686,6 +678,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only branches of the object"), 0, parse_opt_object_name
},
+ OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END(),
};
@@ -723,6 +716,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (filter.abbrev == -1)
filter.abbrev = DEFAULT_ABBREV;
+ filter.ignore_case = icase;
+
finalize_colopts(&colopts, -1);
if (filter.verbose) {
if (explicitly_enable_column(colopts))
@@ -744,6 +739,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
+ /*
+ * If no sorting parameter is given then we default to sorting
+ * by 'refname'. This would give us an alphabetically sorted
+ * array with the 'HEAD' ref at the beginning followed by
+ * local branches 'refs/heads/...' and finally remote-tacking
+ * branches 'refs/remotes/...'.
+ */
+ if (!sorting)
+ sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
print_ref_list(&filter, sorting);
print_columns(&output, colopts, NULL);
string_list_clear(&output, 0);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4e9f6c2..df41fa0 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
int i;
const char *format = "%(objectname) %(objecttype)\t%(refname)";
struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
- int maxcount = 0, quote_style = 0;
+ int maxcount = 0, quote_style = 0, icase = 0;
struct ref_array array;
struct ref_filter filter;
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_MERGED(&filter, N_("print only refs that are merged")),
OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+ OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END(),
};
@@ -63,6 +64,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
if (!sorting)
sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
+ filter.ignore_case = icase;
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..73df728 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -335,6 +335,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
const char *format = NULL;
+ int icase = 0;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -370,6 +371,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
N_("print only tags of the object"), 0, parse_opt_object_name
},
OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")),
+ OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END()
};
@@ -401,6 +403,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
}
if (!sorting)
sorting = ref_default_sorting();
+ sorting->ignore_case = icase;
+ filter.ignore_case = icase;
if (cmdmode == 'l') {
int ret;
if (column_active(colopts)) {
diff --git a/ref-filter.c b/ref-filter.c
index f5f7a70..bd98010 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1231,8 +1231,14 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
* matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
* matches "refs/heads/mas*", too).
*/
-static int match_pattern(const char **patterns, const char *refname)
+static int match_pattern(const struct ref_filter *filter, const char *refname)
{
+ const char **patterns = filter->name_patterns;
+ unsigned flags = 0;
+
+ if (filter->ignore_case)
+ flags |= WM_CASEFOLD;
+
/*
* When no '--format' option is given we need to skip the prefix
* for matching refs of tags and branches.
@@ -1243,7 +1249,7 @@ static int match_pattern(const char **patterns, const char *refname)
skip_prefix(refname, "refs/", &refname));
for (; *patterns; patterns++) {
- if (!wildmatch(*patterns, refname, 0, NULL))
+ if (!wildmatch(*patterns, refname, flags, NULL))
return 1;
}
return 0;
@@ -1255,9 +1261,15 @@ static int match_pattern(const char **patterns, const char *refname)
* matches a pattern "refs/heads/" but not "refs/heads/m") or a
* wildcard (e.g. the same ref matches "refs/heads/m*", too).
*/
-static int match_name_as_path(const char **pattern, const char *refname)
+static int match_name_as_path(const struct ref_filter *filter, const char *refname)
{
+ const char **pattern = filter->name_patterns;
int namelen = strlen(refname);
+ unsigned flags = WM_PATHNAME;
+
+ if (filter->ignore_case)
+ flags |= WM_CASEFOLD;
+
for (; *pattern; pattern++) {
const char *p = *pattern;
int plen = strlen(p);
@@ -1280,8 +1292,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
if (!*filter->name_patterns)
return 1; /* No pattern always matches */
if (filter->match_as_path)
- return match_name_as_path(filter->name_patterns, refname);
- return match_pattern(filter->name_patterns, refname);
+ return match_name_as_path(filter, refname);
+ return match_pattern(filter, refname);
}
/*
@@ -1536,18 +1548,20 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
struct atom_value *va, *vb;
int cmp;
cmp_type cmp_type = used_atom[s->atom].type;
+ int (*cmp_fn)(const char *, const char *);
get_ref_atom_value(a, s->atom, &va);
get_ref_atom_value(b, s->atom, &vb);
+ cmp_fn = s->ignore_case ? strcasecmp : strcmp;
if (s->version)
cmp = versioncmp(va->s, vb->s);
else if (cmp_type == FIELD_STR)
- cmp = strcmp(va->s, vb->s);
+ cmp = cmp_fn(va->s, vb->s);
else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
- cmp = strcmp(a->refname, b->refname);
+ cmp = cmp_fn(a->refname, b->refname);
else
cmp = 1;
}
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..fc55fa3 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -29,6 +29,7 @@ struct ref_sorting {
struct ref_sorting *next;
int atom; /* index into used_atom array (internal) */
unsigned reverse : 1,
+ ignore_case : 1,
version : 1;
};
@@ -62,6 +63,7 @@ struct ref_filter {
unsigned int with_commit_tag_algo : 1,
match_as_path : 1,
+ ignore_case : 1,
detached : 1;
unsigned int kind,
lines;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index c6a3ccb..52283df 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -89,6 +89,11 @@ test_expect_success 'git branch --list -v pattern shows branch summaries' '
awk "{print \$NF}" <tmp >actual &&
test_cmp expect actual
'
+test_expect_success 'git branch --ignore-case --list -v pattern shows branch summaries' '
+ git branch --list --ignore-case -v BRANCH* >tmp &&
+ awk "{print \$NF}" <tmp >actual &&
+ test_cmp expect actual
+'
test_expect_success 'git branch -v pattern does not show branch summaries' '
test_must_fail git branch -v branch*
@@ -196,4 +201,28 @@ test_expect_success 'local-branch symrefs shortened properly' '
test_cmp expect actual
'
+test_expect_success 'sort branches, ignore case' '
+ (
+ git init sort-icase &&
+ cd sort-icase &&
+ test_commit initial &&
+ git branch branch-one &&
+ git branch BRANCH-two &&
+ git branch --list | awk "{print \$NF}" >actual &&
+ cat >expected <<-\EOF &&
+ BRANCH-two
+ branch-one
+ master
+ EOF
+ test_cmp expected actual &&
+ git branch --list -i | awk "{print \$NF}" >actual &&
+ cat >expected <<-\EOF &&
+ branch-one
+ BRANCH-two
+ master
+ EOF
+ test_cmp expected actual
+ )
+'
+
test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..07869b0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -27,6 +27,30 @@ test_expect_success 'listing all tags in an empty tree should output nothing' '
test $(git tag | wc -l) -eq 0
'
+test_expect_success 'sort tags, ignore case' '
+ (
+ git init sort &&
+ cd sort &&
+ test_commit initial &&
+ git tag tag-one &&
+ git tag TAG-two &&
+ git tag -l >actual &&
+ cat >expected <<-\EOF &&
+ TAG-two
+ initial
+ tag-one
+ EOF
+ test_cmp expected actual &&
+ git tag -l -i >actual &&
+ cat >expected <<-\EOF &&
+ initial
+ tag-one
+ TAG-two
+ EOF
+ test_cmp expected actual
+ )
+'
+
test_expect_success 'looking for a tag in an empty tree should fail' \
'! (tag_exists mytag)'
@@ -81,6 +105,9 @@ test_expect_success 'listing all tags if one exists should output that tag' '
test_expect_success 'listing a tag using a matching pattern should succeed' \
'git tag -l mytag'
+test_expect_success 'listing a tag with --ignore-case' \
+ 'test $(git tag -l --ignore-case MYTAG) = mytag'
+
test_expect_success \
'listing a tag using a matching pattern should output that tag' \
'test $(git tag -l mytag) = mytag'
--
2.8.2.524.g6ff3d78
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
2016-12-04 2:52 ` Nguyễn Thái Ngọc Duy
@ 2016-12-06 21:11 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2016-12-06 21:11 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, karthik.188
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This options makes sorting ignore case, which is great when you have
> branches named bug-12-do-something, Bug-12-do-some-more and
> BUG-12-do-what and want to group them together. Sorting externally may
> not be an option because we lose coloring and column layout from
> git-branch and git-tag.
>
> The same could be said for filtering, but it's probably less important
> because you can always go with the ugly pattern [bB][uU][gG]-* if you're
> desperate.
>
> You can't have case-sensitive filtering and case-insensitive sorting (or
> the other way around) with this though. For branch and tag, that should
> be no problem. for-each-ref, as a plumbing, might want finer control.
> But we can always add --{filter,sort}-ignore-case when there is a need
> for it.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
It took me a while to figure out the interactions with topics in
flight, but I think I resolved it correctly now. There was a topic
that added "--format" to branch and tag.
Will be pushed out as part of today's integration cycle.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-06 21:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-17 10:21 [PATCH/RFC] ref-filter: support sorting case-insensitively Nguyễn Thái Ngọc Duy
2016-11-30 12:35 ` [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering Nguyễn Thái Ngọc Duy
2016-11-30 21:21 ` Junio C Hamano
2016-12-04 2:52 ` Nguyễn Thái Ngọc Duy
2016-12-06 21:11 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).