Re: [PATCH DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 8, 2007, 06:39 UTC
- Message-ID
- <7vy7d95ji1.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1194430832-6224-7-git-send-email-madcoder@debian.org>
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.
Show 12 quoted lines
> 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.
Show 10 quoted lines
> 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?
Show 12 quoted lines
> 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.