Karthik Nayak <karthik....@gmail.com> writes:

> On Wed, Jul 29, 2015 at 9:26 PM, Matthieu Moy
> <matthieu....@grenoble-inp.fr> wrote:
>> Karthik Nayak <karthik....@gmail.com> writes:
>>
>>> On Tue, Jul 28, 2015 at 7:47 PM, Matthieu Moy
>>> <matthieu....@grenoble-inp.fr> wrote:
>>>>
>>>> I'm not sure what's the convention, but I think the test description
>>>> should give the expected behavior even with test_expect_failure.
>>>>
>>>> And please help the reviewers by saying what's the status wrt this test
>>>> (any plan on how to fix it?).
>>>>
>>>
>>> On the other hand I wonder if the test is even needed as, we don't
>>> really need it
>>> Cause we remove that ability of branch.c by using filter_refs().
>>
>> Please read d0f810f (refs.c: allow listing and deleting badly named
>> refs, 2014-09-03). I think the reasoning makes sense, and we should keep
>> this ability.
>>
>
> This makes sense, I didn't have a thorough look at this but it breaks
> a little in
> ref-filter.c while getting object attributes. So is it okay to mark
> this as TODO?

Solving this doesn't seem much harder than

diff --git a/ref-filter.c b/ref-filter.c
index 6c0189f..a4df287 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1117,7 +1117,7 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
        struct commit *commit = NULL;
        unsigned int kind;
 
-       if (flag & REF_BAD_NAME) {
+       if (!filter->show_bad_name_refs && (flag & REF_BAD_NAME)) {
                  warning("ignoring ref with broken name %s", refname);
                  return 0;
        }
diff --git a/ref-filter.h b/ref-filter.h
index 98ebd3b..b9d2bbc 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -79,7 +79,7 @@ struct ref_filter {
                match_as_path : 1;
        unsigned int lines, branch_kind;
        int abbrev, verbose;
-       int detached : 1;
+       int detached : 1, show_bad_name_refs : 1;
 };
 
 struct ref_filter_cbdata {

and setting filter->show_bad_name_refs when needed (untested). Did I
miss something?

IIRC, historicaly Git allowed some weirdly named refs which made some
commands ambiguous (e.g. a branch named after an option like '-d').
We're forbidding their creation so people shouldn't have any, but we
it's important to continue showing them in case some people have old
bad-named branches lying around.

I'd rather have the code strictly better after your contribution than
before.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to