From: Bert Wesarg Date: Tue, 14 Apr 2009 18:18:45 GMT Subject: Re: [PATCH] remote.c: use shorten_unambiguous_ref Message-ID: <36ca99e90904141118w425a059v6bd22a7e6400f78f@mail.gmail.com> In-Reply-To: <7vprff41lf.fsf@gitster.siamese.dyndns.org> On Tue, Apr 14, 2009 at 18:55, Junio C Hamano wrote: > Jeff King writes: > >> Hmm. I was thinking we might be able to just do away with prettify_ref, >> but I didn't consider the fact that we need to prettify remote things. I >> think you could still unambiguously prettify the local half of those >> callsites, though. >> >> Given that the two functions are closely related, should we perhaps >> rename them to >> >>   const char *shorten_ref(const char *); >>   const char *shorten_ref_unambiguous(const char *); >> >> ? The implementations are quite different, with prettify_ref not really >> respecting the ref lookup rules, but rather just considering a few >> pre-determined bits of the hierarchy as uninteresting. It shouldn't be >> that hard to have them both use the same implementation, like: >> >>   const char *shorten_ref(const char *, int unambiguous); > > I was hoping that a single "shorten" function that does not even take > "unambiguous" parameter would be used by almost everybody.  As far as I > can see, Bert's "rev-parse --abbrev-ref" RFC is the only caller that might > need to use a value different from warn_ambiguous_refs, and all the other > existing callers (including fill_tracking_info() for "upstream" report by > git-branch) do not have to pass "0" but can use the default.  IOW, we can > have: > >        const char *shorten_ref_unambiguous(const char *ref, int strict); >        const char *shorten_ref(const char *ref) >        { >                return shorten_ref_unambiguous(ref, warn_ambiguous_refs); >        } There's too much confusion: Whatever input you gave to shorten_unambiguous_ref(), if you pass the result to dwim_ref() you get the input again. That is, all return values are unambiguous. The only small different (and here comes a little word-confusion) is, that in strict mode you wont get any warning for 'ambiguous' refs. So the names "shorten_ref_unambiguous" and "shorten_rer" are misleading, because it looks like the former returns unambiguous refs and the latter not. Which is wrong. I can't think about this further, no time sorry. Bert