Re: [PATCH v2 1/2] Introduce config variable "diff.primer"

[Junio C Hamano <gitster@pobox.com>]

Keith Cascio <keith@CS.UCLA.EDU> writes:

> I think introducing explicit dirty masks and explicit layer flattening is the 
> right way to go forward,...

I am not so sure about this claim.  I kind of like idea of "we know the
desirable value of this bit, do not touch it any further" mask, but I
think it is independent of flattening.  In fact, I do not think flattening
is a good thing.

Any codepath could call DIFF_OPT_SET()/CLR(), whether it is in response to
end user's input from the command line (e.g. "the user said --foo, so I am
flipping the foo bit on/off), or to enforce restriction to achieve sane
semantics (e.g. "it does not make any sense to run this internal diff
without --binary set, so I am using OPT_SET()").  Is it still true, or do
some bit flipping now need to be protected by "is it locked" check and
some others don't?

The latter one (i.e. in the above example, "this internal diff must run
with --binary") we may want to use "opts->mask" to lock down (I wouldn't
call it "dirty", it is more like "locked") the "binary" bit, and we may
even want to issue a warning or an error when the end user attempts to
countermand with --no-binary.  Similarly, I think you would want to lock
down what you got from the true command line so that you can leave them
untouched when you process the value you read from diff.primer.

Doesn't it suggest that you may want two layers of masks, not a flat one,
if you really want the mechanism to scale?

In any case, I think the mechanism based on the lock-down mask is worth
considering when we enhance the option parsing mechanism for the diff and
log family, and if it is done right, I think the code to parse revision
options would benefit quite a bit.  There are codepaths that initialize
the bits to the command's liking (e.g. show wants to always have diff),
let setup_revisions() to process command line flags, and then further
tweak the remaining bits (e.g. whatchanged wants to default to raw), all
interacting in quite subtle ways.

But that should probably be for later cycle, post 1.6.2.