Re: [PATCH DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.

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

Pierre Habouzit <madcoder@debian.org> writes:

> reverse_diff was a bit-value in disguise, it's merged in the flags now.
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>

Just my first impression, as I am in the middle of unrelated
bisect.  I haven't read beyond diff-lib.c changes.

> diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
> index 0b591c8..e71841a 100644
> --- a/builtin-diff-tree.c
> +++ b/builtin-diff-tree.c
> @@ -118,12 +118,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	if (!read_stdin)
> -		return opt->diffopt.exit_with_status ?
> -		    opt->diffopt.has_changes: 0;
> +		return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
> +			&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);

Had to think a bit about this, although it is correct.

>  	if (opt->diffopt.detect_rename)
>  		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
> -				       DIFF_SETUP_USE_CACHE);
> +							   DIFF_SETUP_USE_CACHE);

I wonder what this is about.

> diff --git a/combine-diff.c b/combine-diff.c
> index fe5a2a1..3cab04b 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -664,7 +664,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
>  	int mode_differs = 0;
>  	int i, show_hunks;
>  	int working_tree_file = is_null_sha1(elem->sha1);
> -	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
> +        int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;

Indent?

> diff --git a/diff-lib.c b/diff-lib.c
> index da55713..69b5dc9 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -188,8 +188,7 @@ static int handle_diff_files_args(struct rev_info *revs,
>  		else if (!strcmp(argv[1], "-n") ||
>  				!strcmp(argv[1], "--no-index")) {
>  			revs->max_count = -2;
> -			revs->diffopt.exit_with_status = 1;
> -			revs->diffopt.no_index = 1;
> +			revs->diffopt.flags |= DIFF_OPT_EXIT_WITH_STATUS | DIFF_OPT_NO_INDEX;
>  		}

Now this looks harder to read that everybody else uses
DIFF_OPT_SET() for this, without DIFF_OPT_ prefix for the
bitmask names.