Re: [PATCH 3/5] setup_revisions: remember whether a ref was positive or not

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2011, 22:24 UTC
Message-ID: <7vpqkfv8x1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAGdFq_hLy6_AW-Yh_9fi318Z6jdkFWw5+cYrwMtOitDkGQorFA@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

Show 11 quoted lines

>> But the point is that this codepath does not have a faintest clue whether
>> the "obj" parameter is something the end user actively asked for (which
>> might have been marked as uninteresting for other reasons, namely, because
>> it is reachable from other negative refs). So passing unconditional 0 is
>> just as bad.
>
> Doesn't passing 0 indicate that we _did not_ receive any explicit user
> input on this ref, which is exactly what we want to record? If the
> user passed us any explicit input on the ref, we record it at the
> other call-sites, here, the user told us nothing, so we record exactly
> that, nothing.

Hmm, which means you have a way to say "explicitly affirmative" vs "no information", but no way to say "explicitly negative", for example, and the worse part is that it is unclear if the approach the patch takes is extensible enough to allow that in the future. That is the kind of "myopic hack" attitude I did not particularly like in this patch.

"The next person who needs more generic framework can rip out what this patch does and the work required is the same amount" is not a convincing argument---it would mean you are burdening that other person with an extra work to _redo_ what this series does properly, and it is not likely to be of help for that person after your interest in this codepath has long waned.

Show 10 quoted lines

>> As I said, I am not married to the verbose struct/union representation
>> (the only reason I showed that verbosity was because it allowed me to do
>> away without having to enumerate all the syntax sugars we already
>> support); if your "flags" can express the same thing (it may needs to
>> become a bitfield with enough width, but I highly suspect that you would
>> also need at least a component that says "this is the string the user gave
>> us --- the user said 'master', not 'master^0', for example) and is a lot
>> more compact, that is definitely we want to go with.
>
> Don't we already store that in the name field?

Please remind yourself why then it is not sufficient for your patch to read from the name field please?

After all, wasn't the issue that "master^0..master" yields an empty set but you somehow wanted to know that the RHS of that dotdot was given as a positive ref?