Junio C Hamano venit, vidit, dixit 31.03.2017 20:33: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Michael J Gruber <git@xxxxxxxxx> writes: >> >>>> The only case that this change may make a difference I can think of >>>> is when you have a tag object pointed at from outside refs/tags >>>> (e.g. refs/heads/foo is a tag object); if you are trying to change >>>> the definition of "from_tag" from the current "Is the tip inside >>>> refs/tags/?" to "Is the tip either inside refs/tags/ or is it a tag >>>> object anywhere?", that may be a good change (I didn't think things >>>> through, though), but that shouldn't be hidden inside a commit that >>>> claims to only add support for debugging. >>>> >>>> What problem are you solving? >>> >>> Sorry, I forgot about that change and failed to mention it. >>> >>> It makes no difference in the non-debug case which cares about the >>> Boolean only. In the debug case, I want to distinguish between >>> annotated and lightweight tags, just like describe --debug does. By >>> adding 1 via deref and passing this down, I know that an annotated tag >>> gets the value 2, a lightweight tag 1 and everything else 0, just like >>> describe --tags. >> >> So it sounds like you meant to do something else, and the >> implementation is wrong for that something else (i.e. it wouldn't do >> the right thing for a tag object outside refs/tags/, with or without >> the "--debug" option passed). > > The damage seems worse, but I may be misreading the code. > > is_better_name() compares name->from_tag and from_tag numerically, > because it was designed to take a boolean view of that variable. > Now, an artificially bumped 2 gets compared with name->from_tag that > may be 1 and gets different priority. That artificially inflated > value may be propagated to name->from_tag when the current tip is > judged as a better basis for naming the object. No, I checked not to change the existing behaviour. If you look at the comment above that then you see that one of the sides of the comparison is a non-tag, so we compare 0 to 1 or 2, the boolean outcome being the same. > If this change is only for debugging, perhaps inside if(data->debug) > you added, instead of looking at from_tag, you can look at both > from_tag and deref to choose which prio-nmes to show, without > butchering the value in from_tag variable to affect the existing > code that is exercised with or without --debug? What I did overlook, though, was that name-rev uses the notion "under refs/tags" for "being a tag". In fact, it's puzzling how different describe and name-rev proceed and weigh the alternatives. I didn't mean to change that. In retrospect, displaying the "same" debug information for the two commands doesn't make too much sense as long as they use different information. name-rev does-not distinguish between tag types, so why even display it? I think I should change 3/4 to display exactly those bits that name-rev actually uses for weighing different possible descriptions; they are differents from the "describe-bits". So please withhold 3/4 and 4/4. Michael