On Thu, Nov 08, 2007 at 06:39:02AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@xxxxxxxxxx> writes:
>
> > reverse_diff was a bit-value in disguise, it's merged in the flags now.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@xxxxxxxxxx>
>
> 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.
err I code with tabs of size 4 and I believe my editor was
over-zealous when I asked to reindent some part that I changed :P
> > 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?
will fix.
> > 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.
that could be splitted in two DIFF_OPT_SET indeed. will do.
--
·O· Pierre Habouzit
··O madcoder@xxxxxxxxxx
OOO http://www.madism.org
Attachment:
pgpwvfjqmaVMP.pgp
Description: PGP signature
