On 07/11/17 00:18, Junio C Hamano wrote:
> Jacob Keller <jacob.keller@xxxxxxxxx> writes:
>
> I would have to say that the describe's one is wrong if it does not
> match what for_each_glob_ref() does for the log family of commands'
> "--branches=<pattern>" etc. describe.c::get_name() uses positive
> and negative patterns, just like log-tree.c::add_ref_decoration()
> would with the patch we are discussing, so perhaps the items in
> these lists should get the same "normalize" treatment the patch 1/2
> of this series brings in to make things consistent?
>
I agree that describe should receive the "normalize" treatment. However,
and following the same reasoning, why should describe users adopt the
rules imposed by --glob? I could argue they're also used to the way it
works now.
That being said, the suggestion I mentioned earlier would allow to keep
both current behaviors consistent at the expense of the extra call to
refs.c::ref_exists().
+if (!has_glob_specials(pattern) && !ref_exists(normalized_pattern->buf)) {
+ /* Append implied '/' '*' if not present. */
+ strbuf_complete(normalized_pattern, '/');
+ /* No need to check for '*', there is none. */
+ strbuf_addch(normalized_pattern, '*');
+}
But I don't have enough expertise to decide if this consistency is worth
the extra call to refs.c::ref_exists() or if there are other side-effects
I am not considering.
>> That being said, if we think the extra glob would not cause
>> problems and generally do what people mean... I guess consistent
>> with --glob would be good... But it's definitely not what I'd
>> expect at first glance.
My position is that consistency is good, but the "first glance
expectation" is definitely something important we should take into
consideration.
