Re: [PATCH 0/3] convert diff flags to be stored in a struct
From: Junio C Hamano <hidden>
Date: 2017-10-29 01:22:49
Brandon Williams [off-list ref] 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(-)