Re: [PATCH 0/3] convert diff flags to be stored in a struct

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

Brandon Williams <bmwill@google.com> writes:

> There has be some desire to add additional flags to the diff machineery
> (https://public-inbox.org/git/20171024000931.14814-1-sbeller@google.com/) but
> due to the limits of the number of bits in an unsigned int on some systems we
> can't add any additonal flags to the 'flags' variable.  This series converts
> the flags to be stored in bitfields in a struct instead of in bit positions in
> an unsigned int.

I haven't checked the patches yet, but its smallness look promising.
One reason why I didn't do this myself long time ago was because I
suspected that we may be passing an unsigned int that is a collection
of flags as a parameter in many codepaths (to which I did not see a
good conversion once we start using discrete bitfields in a struct,
without passing a "flags" struct instead, which felt somewhat ugly).

> Some thoughts:
>  * We may want to do a follow on patch to convert all the flags from being in
>    uppercase to lower case.

If and only if we do not use macros to set/clr/test, this makes a
lot of sense.  Otherwise, probably not.

>  * Maybe we can figure out how to remove the 'touched_flags' things (since its
>    only used in one place) and then we may even be able to stop needing to use
>    macros to set/clr/test the flags.

Yup.  That is closely tied with the above one, but back then we
didn't think of a better implementation than the "this was given
from the command line" vs "this was initialized and kept", so we
need some thought.

Thanks for starting this.

> Brandon Williams (3):
>   add: use DIFF_OPT_SET macro to set a diff flag
>   reset: use DIFF_OPT_SET macro to set a diff flag
>   diff: convert flags to be stored in bitfields
>
>  builtin/add.c    |  2 +-
>  builtin/commit.c |  7 +++--
>  builtin/log.c    |  2 +-
>  builtin/reset.c  |  2 +-
>  diff-lib.c       |  6 ++--
>  diff.c           |  3 +-
>  diff.h           | 96 +++++++++++++++++++++++++++++++++-----------------------
>  sequencer.c      |  5 +--
>  8 files changed, 72 insertions(+), 51 deletions(-)