convert diff flags to be stored in a struct

PATCH Superseded Brandon Williams 3 participants 47 messages Oct 27, 2017 – Nov 1, 2017

Share & tools

Brandon Williams

author

2017-10-27 22:29 UTC #0 lore raw

cover convert diff flags to be stored in a struct

There has be some desire to add additional flags to the diff machineery
(https://public-inbox.org/git/[email protected]/) 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.

Some thoughts:
* We may want to do a follow on patch to convert all the flags from being in uppercase to lower case.
* 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.

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(-)

Signature · 1 line

2.15.0.rc2.357.g7e34df9404-goog

Brandon Williams

author

2017-10-27 22:29 UTC #1 lore raw

PATCH 1/3 add: use DIFF_OPT_SET macro to set a diff flag

Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/add.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

builtin/add.c +1 -1

index a648cf4c5..b70e8a779 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	rev.diffopt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

Signature · 1 line

2.15.0.rc2.357.g7e34df9404-goog

Brandon Williams

author

2017-10-27 22:29 UTC #2 lore raw

PATCH 2/3 reset: use DIFF_OPT_SET macro to set a diff flag

In reply to Brandon Williams

Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

builtin/reset.c +1 -1

index 9cd89b230..ea2fad5a0 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

Signature · 1 line

2.15.0.rc2.357.g7e34df9404-goog

Junio C Hamano

2017-10-29 01:26 UTC #3 lore raw

Re: [PATCH 2/3] reset: use DIFF_OPT_SET macro to set a diff flag

Quoted 5 lines · Brandon Williams

Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.
Signed-off-by Brandon Williams <[email protected]>
---

Looks good. It's not like one of 1/3 and 2/3 could be a good idea while the other is not, so it would make a lot more sense to combine them into a single preliminary clean-up patch, though.

In any case, these two are very good clean-up patches, whose value does not diminish even we do not go ahead with 3/3 yet.

Nicely spotted; thanks.

Quoted 16 lines

 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

builtin/reset.c +1 -1

index 9cd89b230..ea2fad5a0 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

Brandon Williams

author

2017-10-30 18:06 UTC #4 lore raw

Re: [PATCH 2/3] reset: use DIFF_OPT_SET macro to set a diff flag

Quoted 5 lines · Junio C Hamano

Quoted 3 lines · Brandon Williams
Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.

Quoted 7 lines

Quoted 2 lines
Signed-off-by Brandon Williams <[email protected]>
---
Looks good. It's not like one of 1/3 and 2/3 could be a good idea while the other is not, so it would make a lot more sense to combine them into a single preliminary clean-up patch, though.

I'll squash them together in v2.

Quoted 22 lines

In any case, these two are very good clean-up patches, whose value does not diminish even we do not go ahead with 3/3 yet.

Nicely spotted; thanks.

Quoted 16 lines

 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

builtin/reset.c +1 -1

index 9cd89b230..ea2fad5a0 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

Signature · 1 line

Brandon Williams

Brandon Williams

author

2017-10-27 22:29 UTC #5 lore raw

PATCH 3/3 diff: convert flags to be stored in bitfields

In reply to Brandon Williams

We have have reached the limit of the number of flags that can be stored in a single unsigned int. In order to allow for more flags to be added to the diff machinery in the future this patch converts the flags to be stored in bitfields in 'struct diff_flags'.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/commit.c |  7 +++--
 builtin/log.c    |  2 +-
 diff-lib.c       |  6 ++--
 diff.c           |  3 +-
 diff.h           | 96 +++++++++++++++++++++++++++++++++-----------------------
 sequencer.c      |  5 +--
 6 files changed, 70 insertions(+), 49 deletions(-)

builtin/commit.c +4 -3

index d75b3805e..de08c2594 100644
@@ -912,11 +912,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * submodules which were manually staged, which would
 			 * be really confusing.
 			 */
-			int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+			struct diff_flags flags = DIFF_FLAGS_INIT;
+			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
-			commitable = index_differs_from(parent, diff_flags, 1);
+				flags.IGNORE_SUBMODULES = 1;
+			commitable = index_differs_from(parent, flags, 1);
 		}
 	}
 	strbuf_release(&committer_ident);

builtin/log.c +1 -1

index d81a09051..780975ed4 100644
@@ -134,7 +134,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-	rev->diffopt.touched_flags = 0;
+	rev->diffopt.touched_flags = diff_flags_cleared;
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

diff-lib.c +3 -3

index 4e0980caa..7375ef71d 100644
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 {
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
-		unsigned orig_flags = diffopt->flags;
+		struct diff_flags orig_flags = diffopt->flags;
 		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
 		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))

@@ -534,7 +534,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
 	return 0;
 }
 
-int index_differs_from(const char *def, int diff_flags,
+int index_differs_from(const char *def, struct diff_flags flags,
 		       int ita_invisible_in_index)
 {
 	struct rev_info rev;

@@ -546,7 +546,7 @@ int index_differs_from(const char *def, int diff_flags,
 	setup_revisions(0, NULL, &rev, &opt);
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-	rev.diffopt.flags |= diff_flags;
+	rev.diffopt.flags = diff_flags_or(rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);

diff.c +2 -1

index 6fd288420..6f1050d42 100644
@@ -46,6 +46,7 @@ static int diff_no_prefix;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
+const struct diff_flags diff_flags_cleared = DIFF_FLAGS_INIT;
 static long diff_algorithm;
 static unsigned ws_error_highlight_default = WSEH_NEW;

@@ -5899,7 +5900,7 @@ int diff_can_quit_early(struct diff_options *opt)
 static int is_submodule_ignored(const char *path, struct diff_options *options)
 {
 	int ignored = 0;
-	unsigned orig_flags = options->flags;
+	struct diff_flags orig_flags = options->flags;
 	if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
 		set_diffopt_flags_from_submodule_config(options, path);
 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))

diff.h +57 ~~-39~~

114 lines not on this page · raw message

sequencer.c +3 -2

index f2a10cc4f..cf9bec716 100644
@@ -959,7 +959,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		unborn = get_oid("HEAD", &head);
 		if (unborn)
 			oidcpy(&head, &empty_tree_oid);
-		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0, 0))
+		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD",
+				       diff_flags_cleared, 0))
 			return error_dirty_index(opts);
 	}
 	discard_cache();

@@ -2279,7 +2280,7 @@ int sequencer_continue(struct replay_opts *opts)
 			if (res)
 				goto release_todo_list;
 		}
-		if (index_differs_from("HEAD", 0, 0)) {
+		if (index_differs_from("HEAD", diff_flags_cleared, 0)) {
 			res = error_dirty_index(opts);
 			goto release_todo_list;
 		}

Signature · 1 line

2.15.0.rc2.357.g7e34df9404-goog

Junio C Hamano

2017-10-29 01:55 UTC #6 lore raw

Re: [PATCH 3/3] diff: convert flags to be stored in bitfields

Quoted 1 line · Brandon Williams

We have have reached the limit of the number of flags that can be stored

s/have have/have/; but bit #9 is unused.

"We cannot add many more flags even if we wanted to" would be more flexible and does not take this change hostage to whatever topic that tries to claim that bit, I would think.

Quoted 27 lines

in a single unsigned int. In order to allow for more flags to be added to the diff machinery in the future this patch converts the flags to be stored in bitfields in 'struct diff_flags'.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/commit.c |  7 +++--
 builtin/log.c    |  2 +-
 diff-lib.c       |  6 ++--
 diff.c           |  3 +-
 diff.h           | 96 +++++++++++++++++++++++++++++++++-----------------------
 sequencer.c      |  5 +--
 6 files changed, 70 insertions(+), 49 deletions(-)

diff.h +0 -4

index aca150ba2..d58f06106 100644
@@ -60,42 +60,59 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 
 #define DIFF_FORMAT_CALLBACK	0x1000
 
-#define DIFF_OPT_RECURSIVE           (1 <<  0)
-#define DIFF_OPT_TREE_IN_RECURSIVE   (1 <<  1)
-#define DIFF_OPT_BINARY              (1 <<  2)
-...

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 64

Quoted 10 lines

diff.h +2 -8

-#define DIFF_OPT_FUNCCONTEXT         (1 << 29)
-#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
-#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)
-
-#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
-#define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)
-#define DIFF_OPT_SET(opts, flag)    (((opts)->flags |= DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
-#define DIFF_OPT_CLR(opts, flag)    (((opts)->flags &= ~DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
+#define DIFF_FLAGS_INIT { 0 }
+extern const struct diff_flags diff_flags_cleared;

This thing is curious. Seeing the scary diff_flags_or(), I would have expected we'd have diff_flags_clear(&flags).

Quoted 12 lines

diff.h +12 -0

+struct diff_flags {
+	unsigned RECURSIVE:1;
+	unsigned TREE_IN_RECURSIVE:1;
+	unsigned BINARY:1;
+...
+	unsigned FUNCCONTEXT:1;
+	unsigned PICKAXE_IGNORE_CASE:1;
+	unsigned DEFAULT_FOLLOW_RENAMES:1;
+};
+
+static inline struct diff_flags diff_flags_or(struct diff_flags a,
+					      struct diff_flags b)

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 109

Quoted 10 lines

diff.h +10 -0

+{
+	char *tmp_a = (char *)&a;
+	char *tmp_b = (char *)&b;
+	int i;
+
+	for (i = 0; i < sizeof(struct diff_flags); i++)
+		tmp_a[i] |= tmp_b[i];
+
+	return a;
+}

This is doubly scary, but let's see why we need it by looking at the callers.

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 114

Quoted 4 lines

diff.h +4 -0

+#define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
+#define DIFF_OPT_TOUCHED(opts, flag)	((opts)->touched_flags.flag)
+#define DIFF_OPT_SET(opts, flag)	(((opts)->flags.flag = 1),((opts)->touched_flags.flag = 1))
+#define DIFF_OPT_CLR(opts, flag)	(((opts)->flags.flag = 0),((opts)->touched_flags.flag = 1))

These are trivial and straight-forward.

Quoted 16 lines

diff.h +3 -2

+
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)

@@ -122,8 +139,8 @@ struct diff_options {
 	const char *a_prefix, *b_prefix;
 	const char *line_prefix;
 	size_t line_prefix_length;
-	unsigned flags;
-	unsigned touched_flags;
+	struct diff_flags flags;
+	struct diff_flags touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;

diff.h extern int diff_result_code(struct diff_options *, int); · line 409

Quoted 6 lines

diff.h +2 -1

@@ -388,7 +405,8 @@ extern int diff_result_code(struct diff_options *, int);
 
 extern void diff_no_index(struct rev_info *, int, const char **);
 
-extern int index_differs_from(const char *def, int diff_flags, int ita_invisible_in_index);
+extern int index_differs_from(const char *def, struct diff_flags flags,
+			      int ita_invisible_in_index);

OK. I tend to think twice before passing any struct by value (even something that starts its life as a small/single-word struct), but let's see how much simpler this allows callers to become.

Quoted 5 lines

builtin/commit.c +0 -0
index d75b3805e..de08c2594 100644

builtin/commit.c static int prepare_to_commit(const char *index_file, const char *prefix, · line 918

Quoted 9 lines

builtin/commit.c +2 -2

@@ -912,11 +912,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * submodules which were manually staged, which would
 			 * be really confusing.
 			 */
-			int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+			struct diff_flags flags = DIFF_FLAGS_INIT;
+			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;

This couldn't have been done in patches 1+2/3 because DIFF_OPT_SET() does not take a bare 'flags' but diffopt itself, which was a bit unfortunate, but the end result after this step becomes a lot more sensible.

builtin/commit.c static int prepare_to_commit(const char *index_file, const char *prefix, · line 920

Quoted 3 lines

builtin/commit.c +2 -1

-			commitable = index_differs_from(parent, diff_flags, 1);
+				flags.IGNORE_SUBMODULES = 1;
+			commitable = index_differs_from(parent, flags, 1);

OK.

builtin/log.c static void cmd_log_init_defaults(struct rev_info *rev) · line 137

Quoted 10 lines

builtin/log.c +1 -1

index d81a09051..780975ed4 100644
@@ -134,7 +134,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-	rev->diffopt.touched_flags = 0;
+	rev->diffopt.touched_flags = diff_flags_cleared;

So this structure assignment is a more kosher way to clear everything than memset(&touched_flags, '\0', sizeof(...)); I'd still prefer

	diff_flags_clear(&rev->diffopt.touched_flags);

tough, as it is easy to forget diff_flags_cleared is a singleton constant specifically created to be assigned for clearing another flags struct.

builtin/log.c int index_differs_from(const char *def, int diff_flags, · line 549

Quoted 6 lines

builtin/log.c +1 -1

@@ -546,7 +546,7 @@ int index_differs_from(const char *def, int diff_flags,
 	setup_revisions(0, NULL, &rev, &opt);
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-	rev.diffopt.flags |= diff_flags;
+	rev.diffopt.flags = diff_flags_or(rev.diffopt.flags, flags);

In a more general case, we cannot know what flags setup_revisions() gave to rev.diffopt, and because of that we cannot do

	rev.diffopt.flags = diff_flags;
	DIFF_OPT_SET(&rev.diffopt, QUICK);
	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);

without using the flags_or() thing. In this codepath, that currently is not a problem (because we give ac/av = 0/NULL), but it probably is a good idea to avoid depending on that.

I still haven't brought myself to like the structure being passed by value and the singleton diff_flags_cleared thing, but I suspect that we may get used to them once we start using these. I dunno.

Thanks.

Junio C Hamano

2017-10-30 00:30 UTC #7 lore raw

Re: [PATCH 3/3] diff: convert flags to be stored in bitfields

Quoted 3 lines · Junio C Hamano

I still haven't brought myself to like the structure being passed by value and the singleton diff_flags_cleared thing, but I suspect that we may get used to them once we start using these. I dunno.

Just bikeshedding, but I just had to prepare an evil merge to add a new use of diff_flags_cleared to a codepath that evolved in a topic still in flight, and realized that I really hate the name. Perhaps I wouldn't have hated it so much if it were named diff_flags_none or diff_flags_empty, I guess.

Brandon Williams

author

2017-10-30 19:39 UTC #8 lore raw

Re: [PATCH 3/3] diff: convert flags to be stored in bitfields

Quoted 11 lines · Junio C Hamano

Quoted 3 lines · Junio C Hamano
I still haven't brought myself to like the structure being passed by value and the singleton diff_flags_cleared thing, but I suspect that we may get used to them once we start using these. I dunno.
Just bikeshedding, but I just had to prepare an evil merge to add a new use of diff_flags_cleared to a codepath that evolved in a topic still in flight, and realized that I really hate the name. Perhaps I wouldn't have hated it so much if it were named diff_flags_none or diff_flags_empty, I guess.

I have a new version of the series I'll send out and i ended up dropping it entirely. Didn't even need a clear function because I was able to drop the touched stuff and it would have only been used inside of builtin/log.c to clear the touched flags.

Signature · 1 line

Brandon Williams

Junio C Hamano

2017-10-31 02:49 UTC #9 lore raw

Re: [PATCH 3/3] diff: convert flags to be stored in bitfields

In reply to Junio C Hamano

Quoted 11 lines · Junio C Hamano

Quoted 3 lines · Junio C Hamano
I still haven't brought myself to like the structure being passed by value and the singleton diff_flags_cleared thing, but I suspect that we may get used to them once we start using these. I dunno.
Just bikeshedding, but I just had to prepare an evil merge to add a new use of diff_flags_cleared to a codepath that evolved in a topic still in flight, and realized that I really hate the name. Perhaps I wouldn't have hated it so much if it were named diff_flags_none or diff_flags_empty, I guess.

As to the "passing by value" thing, as long as we have the _or() helper function, no sensible person will add anything but a bitfield into the structure, so I am fine with it.

Brandon Williams

author

2017-10-30 17:50 UTC #10 lore raw

Re: [PATCH 3/3] diff: convert flags to be stored in bitfields

In reply to Junio C Hamano

Quoted 9 lines · Junio C Hamano

Quoted 1 line · Brandon Williams
We have have reached the limit of the number of flags that can be stored
s/have have/have/; but bit #9 is unused.
"We cannot add many more flags even if we wanted to" would be more flexible and does not take this change hostage to whatever topic that tries to claim that bit, I would think.

I'll tweak the wording a bit.

Quoted 33 lines

in a single unsigned int. In order to allow for more flags to be added to the diff machinery in the future this patch converts the flags to be stored in bitfields in 'struct diff_flags'.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/commit.c |  7 +++--
 builtin/log.c    |  2 +-
 diff-lib.c       |  6 ++--
 diff.c           |  3 +-
 diff.h           | 96 +++++++++++++++++++++++++++++++++-----------------------
 sequencer.c      |  5 +--
 6 files changed, 70 insertions(+), 49 deletions(-)

diff.h +0 -9

index aca150ba2..d58f06106 100644
@@ -60,42 +60,59 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 
 #define DIFF_FORMAT_CALLBACK	0x1000
 
-#define DIFF_OPT_RECURSIVE           (1 <<  0)
-#define DIFF_OPT_TREE_IN_RECURSIVE   (1 <<  1)
-#define DIFF_OPT_BINARY              (1 <<  2)
-...
-#define DIFF_OPT_FUNCCONTEXT         (1 << 29)
-#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
-#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)
-
-#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)

Quoted 8 lines

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 64
Quoted 5 lines
diff.h +2 -3
-#define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)
-#define DIFF_OPT_SET(opts, flag)    (((opts)->flags |= DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
-#define DIFF_OPT_CLR(opts, flag)    (((opts)->flags &= ~DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
+#define DIFF_FLAGS_INIT { 0 }
+extern const struct diff_flags diff_flags_cleared;
This thing is curious. Seeing the scary diff_flags_or(), I would have expected we'd have diff_flags_clear(&flags).

This seemed to make things easier in practice because I was passing some structs by value, let me work with it a bit to see what it looks like when they are passed by reference instead.

Quoted 131 lines

Quoted 13 lines
diff.h +12 -0
+struct diff_flags {
+	unsigned RECURSIVE:1;
+	unsigned TREE_IN_RECURSIVE:1;
+	unsigned BINARY:1;
+...
+	unsigned FUNCCONTEXT:1;
+	unsigned PICKAXE_IGNORE_CASE:1;
+	unsigned DEFAULT_FOLLOW_RENAMES:1;
+};
+
+static inline struct diff_flags diff_flags_or(struct diff_flags a,
+					      struct diff_flags b)
diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 109
Quoted 10 lines
diff.h +10 -0
+{
+	char *tmp_a = (char *)&a;
+	char *tmp_b = (char *)&b;
+	int i;
+
+	for (i = 0; i < sizeof(struct diff_flags); i++)
+		tmp_a[i] |= tmp_b[i];
+
+	return a;
+}
This is doubly scary, but let's see why we need it by looking at the callers.
diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 114
Quoted 4 lines
diff.h +4 -0
+#define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
+#define DIFF_OPT_TOUCHED(opts, flag)	((opts)->touched_flags.flag)
+#define DIFF_OPT_SET(opts, flag)	(((opts)->flags.flag = 1),((opts)->touched_flags.flag = 1))
+#define DIFF_OPT_CLR(opts, flag)	(((opts)->flags.flag = 0),((opts)->touched_flags.flag = 1))
These are trivial and straight-forward.
Quoted 16 lines
diff.h +3 -2
+
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)
@@ -122,8 +139,8 @@ struct diff_options {
 	const char *a_prefix, *b_prefix;
 	const char *line_prefix;
 	size_t line_prefix_length;
-	unsigned flags;
-	unsigned touched_flags;
+	struct diff_flags flags;
+	struct diff_flags touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;
diff.h extern int diff_result_code(struct diff_options *, int); · line 409
Quoted 6 lines
diff.h +2 -1
@@ -388,7 +405,8 @@ extern int diff_result_code(struct diff_options *, int);
 
 extern void diff_no_index(struct rev_info *, int, const char **);
 
-extern int index_differs_from(const char *def, int diff_flags, int ita_invisible_in_index);
+extern int index_differs_from(const char *def, struct diff_flags flags,
+			      int ita_invisible_in_index);
OK. I tend to think twice before passing any struct by value (even something that starts its life as a small/single-word struct), but let's see how much simpler this allows callers to become.
Quoted 5 lines
builtin/commit.c +0 -0
index d75b3805e..de08c2594 100644
builtin/commit.c static int prepare_to_commit(const char *index_file, const char *prefix, · line 918
Quoted 9 lines
builtin/commit.c +2 -2
@@ -912,11 +912,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * submodules which were manually staged, which would
 			 * be really confusing.
 			 */
-			int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+			struct diff_flags flags = DIFF_FLAGS_INIT;
+			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
This couldn't have been done in patches 1+2/3 because DIFF_OPT_SET() does not take a bare 'flags' but diffopt itself, which was a bit unfortunate, but the end result after this step becomes a lot more sensible.
builtin/commit.c static int prepare_to_commit(const char *index_file, const char *prefix, · line 920
Quoted 3 lines
builtin/commit.c +2 -1
-			commitable = index_differs_from(parent, diff_flags, 1);
+				flags.IGNORE_SUBMODULES = 1;
+			commitable = index_differs_from(parent, flags, 1);
OK.
builtin/log.c static void cmd_log_init_defaults(struct rev_info *rev) · line 137
Quoted 10 lines
builtin/log.c +1 -1
index d81a09051..780975ed4 100644
@@ -134,7 +134,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-	rev->diffopt.touched_flags = 0;
+	rev->diffopt.touched_flags = diff_flags_cleared;
So this structure assignment is a more kosher way to clear everything than memset(&touched_flags, '\0', sizeof(...)); I'd still prefer
	diff_flags_clear(&rev->diffopt.touched_flags);
tough, as it is easy to forget diff_flags_cleared is a singleton constant specifically created to be assigned for clearing another flags struct.
builtin/log.c int index_differs_from(const char *def, int diff_flags, · line 549
Quoted 6 lines
builtin/log.c +1 -1
@@ -546,7 +546,7 @@ int index_differs_from(const char *def, int diff_flags,
 	setup_revisions(0, NULL, &rev, &opt);
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-	rev.diffopt.flags |= diff_flags;
+	rev.diffopt.flags = diff_flags_or(rev.diffopt.flags, flags);
In a more general case, we cannot know what flags setup_revisions() gave to rev.diffopt, and because of that we cannot do
	rev.diffopt.flags = diff_flags;
	DIFF_OPT_SET(&rev.diffopt, QUICK);
	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
without using the flags_or() thing. In this codepath, that currently is not a problem (because we give ac/av = 0/NULL), but it probably is a good idea to avoid depending on that.
I still haven't brought myself to like the structure being passed by value and the singleton diff_flags_cleared thing, but I suspect that we may get used to them once we start using these. I dunno.
Thanks.

Signature · 1 line

Brandon Williams

Junio C Hamano

2017-10-29 01:22 UTC #11 lore raw

In reply to Brandon Williams

Quoted 6 lines · Brandon Williams

There has be some desire to add additional flags to the diff machineery
(https://public-inbox.org/git/[email protected]/) 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).

Quoted 3 lines

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.

Quoted 3 lines

* 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.

Quoted 14 lines

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(-)

Stefan Beller

2017-10-29 04:37 UTC #12 lore raw

Quoted 6 lines · Junio C Hamano

Quoted 3 lines
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.

The .touched_flags is used just once in the code base for just one flag. I wonder if we could get rid of touched_flags and instead introduce a new flag (FORCE_TEXT_CONV instead of ALLOW_TEXT_CONV set by cmd_show specifically) that will enable the special casing that is currently done via the touched_flags.

Once we get rid of the special casing, we can get rid of the macros.

Brandon Williams

author

2017-10-30 19:46 UTC #13 lore raw

v2 cover convert diff flags to be stored in a struct

In reply to Brandon Williams

Changes in v2:
* removed the diff_flags_cleared singleton
* eliminated the 'touched' parallel flags
* pass structs by reference instead of by value

Now that the 'touched' flags have been removed it may be valuable to go ahead and remove the macros all together (including making the flags lower case). If that's what we want to do, I can go ahead and send those patches out as a follow on to these.

Brandon Williams (4):
add, reset: use DIFF_OPT_SET macro to set a diff flag
diff: convert flags to be stored in bitfields
diff: add flag to indicate textconv was set via cmdline
diff: remove touched flags

 builtin/add.c    |  2 +-
 builtin/commit.c |  7 +++--
 builtin/log.c    |  3 +-
 builtin/reset.c  |  2 +-
 diff-lib.c       |  7 +++--
 diff.c           | 10 +++---
 diff.h           | 96 +++++++++++++++++++++++++++++++++-----------------------
 sequencer.c      |  5 +--
 8 files changed, 77 insertions(+), 55 deletions(-)

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 19:46 UTC #14 lore raw

PATCH v2 1/4 add, reset: use DIFF_OPT_SET macro to set a diff flag

Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/add.c   | 2 +-
 builtin/reset.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

builtin/add.c +1 -1

index a648cf4c5..b70e8a779 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	rev.diffopt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

builtin/reset.c +1 -1

index 9cd89b230..ea2fad5a0 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 19:47 UTC #15 lore raw

PATCH v2 2/4 diff: convert flags to be stored in bitfields

In reply to Brandon Williams

We cannot add many more flags to the diff machinery due to the limitations of the number of flags that can be stored in a single unsigned int. In order to allow for more flags to be added to the diff machinery in the future this patch converts the flags to be stored in bitfields in 'struct diff_flags'.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/commit.c |  7 ++--
 builtin/log.c    |  3 +-
 diff-lib.c       |  7 ++--
 diff.c           |  2 +-
 diff.h           | 97 +++++++++++++++++++++++++++++++++-----------------------
 sequencer.c      |  5 +--
 6 files changed, 72 insertions(+), 49 deletions(-)

builtin/commit.c +4 -3

index d75b3805e..960e7ac08 100644
@@ -912,11 +912,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * submodules which were manually staged, which would
 			 * be really confusing.
 			 */
-			int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+			struct diff_flags flags = DIFF_FLAGS_INIT;
+			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
-			commitable = index_differs_from(parent, diff_flags, 1);
+				flags.IGNORE_SUBMODULES = 1;
+			commitable = index_differs_from(parent, &flags, 1);
 		}
 	}
 	strbuf_release(&committer_ident);

builtin/log.c +2 -1

index d81a09051..dc28d43eb 100644
@@ -134,7 +134,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-	rev->diffopt.touched_flags = 0;
+
+	memset(&rev->diffopt.touched_flags, 0, sizeof(struct diff_flags));
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

diff-lib.c +4 -3

index 4e0980caa..6c1c05c5b 100644
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 {
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
-		unsigned orig_flags = diffopt->flags;
+		struct diff_flags orig_flags = diffopt->flags;
 		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
 		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))

@@ -534,7 +534,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
 	return 0;
 }
 
-int index_differs_from(const char *def, int diff_flags,
+int index_differs_from(const char *def, const struct diff_flags *flags,
 		       int ita_invisible_in_index)
 {
 	struct rev_info rev;

@@ -546,7 +546,8 @@ int index_differs_from(const char *def, int diff_flags,
 	setup_revisions(0, NULL, &rev, &opt);
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-	rev.diffopt.flags |= diff_flags;
+	if (flags)
+		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);

diff.c +1 -1

index 6fd288420..3ad9c9b31 100644
@@ -5899,7 +5899,7 @@ int diff_can_quit_early(struct diff_options *opt)
 static int is_submodule_ignored(const char *path, struct diff_options *options)
 {
 	int ignored = 0;
-	unsigned orig_flags = options->flags;
+	struct diff_flags orig_flags = options->flags;
 	if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
 		set_diffopt_flags_from_submodule_config(options, path);
 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))

diff.h +58 ~~-39~~

115 lines not on this page · raw message

sequencer.c +3 -2

index f2a10cc4f..c0410e491 100644
@@ -959,7 +959,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		unborn = get_oid("HEAD", &head);
 		if (unborn)
 			oidcpy(&head, &empty_tree_oid);
-		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0, 0))
+		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD",
+				       NULL, 0))
 			return error_dirty_index(opts);
 	}
 	discard_cache();

@@ -2279,7 +2280,7 @@ int sequencer_continue(struct replay_opts *opts)
 			if (res)
 				goto release_todo_list;
 		}
-		if (index_differs_from("HEAD", 0, 0)) {
+		if (index_differs_from("HEAD", NULL, 0)) {
 			res = error_dirty_index(opts);
 			goto release_todo_list;
 		}

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Junio C Hamano

2017-10-31 04:41 UTC #16 lore raw

Re: [PATCH v2 2/4] diff: convert flags to be stored in bitfields

diff-lib.c int index_differs_from(const char *def, int diff_flags, · line 550

Quoted 2 lines · Brandon Williams

diff-lib.c +2 -0

+	if (flags)
+		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);

If we are avoiding from passing a struct (even if it is a small one) by value, then returning a struct as value defeats the point of the exercise, I would think. If that will be the calling cconvention, making diff_flags_or(&a, &b) to update &a by or'ing bits in &b would be more natural.

Having said that, as I said in a separate message, as long as we have this _or() thing, no sane person would add anything but bitfields to the structure which will guarantee that it will stay to be small set of flags and nothing else---so I personally am fine with pass by value (which in turn makes it OK to return as a value, too).

Other than that, this step looked reasonable to me.

Thanks.

Junio C Hamano

2017-10-31 05:23 UTC #17 lore raw

PATCH 1/1 diff: avoid returning a struct by value from diff_flags_or()

In reply to Brandon Williams

That is more in line with the design decision made in the previous step to pass struct by reference.

We may want to squash this into the previous patch eventually.

Signed-off-by Junio C Hamano <[email protected]>

---

* I am OK either way as long as things are consistent; as you took time to change the code to pass the struct by reference, let's unify the API in that direction.

 diff-lib.c |  2 +-
 diff.h     | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff-lib.c +1 -1

index 6c1c05c5b0..ed37f24c68 100644
@@ -547,7 +547,7 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
 	if (flags)
-		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);
+		diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);

diff.h +4 -8

index 47e6d43cbc..e512cf44d0 100644
@@ -94,19 +94,15 @@ struct diff_flags {
 	unsigned DEFAULT_FOLLOW_RENAMES:1;
 };
 
-static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
-					      const struct diff_flags *b)
+static inline void diff_flags_or(struct diff_flags *a,
+				 const struct diff_flags *b)
 {
-	struct diff_flags out;
 	char *tmp_a = (char *)a;
-	char *tmp_b = (char *)b;
-	char *tmp_out = (char *)&out;
+	const char *tmp_b = (const char *)b;
 	int i;
 
 	for (i = 0; i < sizeof(struct diff_flags); i++)
-		tmp_out[i] = tmp_a[i] | tmp_b[i];
-
-	return out;
+		tmp_a[i] |= tmp_b[i];
 }
 
 #define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)

Signature · 1 line

2.15.0-224-g5109123e6a

Brandon Williams

author

2017-10-31 17:51 UTC #18 lore raw

Re: [PATCH 2.5/4] diff: avoid returning a struct by value from diff_flags_or()

Quoted 4 lines · Junio C Hamano

That is more in line with the design decision made in the previous step to pass struct by reference.
We may want to squash this into the previous patch eventually.

Looks good to me. I was on the fence with what to do since I was already moving to pass by reference. This is probably the better move so I can squash this one into the previous.

Quoted 56 lines

Signed-off-by Junio C Hamano <[email protected]>

---

* I am OK either way as long as things are consistent; as you took time to change the code to pass the struct by reference, let's unify the API in that direction.

 diff-lib.c |  2 +-
 diff.h     | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff-lib.c +1 -1

index 6c1c05c5b0..ed37f24c68 100644
@@ -547,7 +547,7 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
 	if (flags)
-		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);
+		diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);

diff.h +4 -8

index 47e6d43cbc..e512cf44d0 100644
@@ -94,19 +94,15 @@ struct diff_flags {
 	unsigned DEFAULT_FOLLOW_RENAMES:1;
 };
 
-static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
-					      const struct diff_flags *b)
+static inline void diff_flags_or(struct diff_flags *a,
+				 const struct diff_flags *b)
 {
-	struct diff_flags out;
 	char *tmp_a = (char *)a;
-	char *tmp_b = (char *)b;
-	char *tmp_out = (char *)&out;
+	const char *tmp_b = (const char *)b;
 	int i;
 
 	for (i = 0; i < sizeof(struct diff_flags); i++)
-		tmp_out[i] = tmp_a[i] | tmp_b[i];
-
-	return out;
+		tmp_a[i] |= tmp_b[i];
 }
 
 #define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)

Signature · 1 line

2.15.0-224-g5109123e6a

Signature · 1 line

Brandon Williams

Brandon Williams

author

2017-10-30 19:47 UTC #19 lore raw

PATCH v2 3/4 diff: add flag to indicate textconv was set via cmdline

In reply to Brandon Williams

git-show is unique in that it wants to use textconv by default except for when it is showing blobs. When asked to show a blob, show doesn't want to use textconv unless the user explicitly requested that it be used by providing the command line flag '--textconv'.

Currently this is done by using a parallel set of 'touched' flags which get set every time a particular flag is set or cleared. In a future patch we want to eliminate this parallel set of flags so instead of relying on if the textconv flag has been touched, add a new flag 'TEXTCONV_SET_VIA_CMDLINE' which is only set if textconv is requested via the command line.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/log.c | 2 +-
 diff.c        | 8 +++++---
 diff.h        | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)

builtin/log.c +1 -1

index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+	if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
 	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
 		return stream_blob_to_fd(1, oid, NULL, 0);

diff.c +5 -3

index 3ad9c9b31..8b700b1bd 100644
@@ -4762,11 +4762,13 @@ int diff_opt_parse(struct diff_options *options,
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
-	else if (!strcmp(arg, "--textconv"))
+	else if (!strcmp(arg, "--textconv")) {
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--no-textconv"))
+		DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+	} else if (!strcmp(arg, "--no-textconv")) {
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--ignore-submodules")) {
+		DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+	} else if (!strcmp(arg, "--ignore-submodules")) {
 		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, "all");
 	} else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

diff.h +1 -0

index 47e6d43cb..4eaf9b370 100644
@@ -83,6 +83,7 @@ struct diff_flags {
 	unsigned DIRSTAT_CUMULATIVE:1;
 	unsigned DIRSTAT_BY_FILE:1;
 	unsigned ALLOW_TEXTCONV:1;
+	unsigned TEXTCONV_SET_VIA_CMDLINE:1;
 	unsigned DIFF_FROM_CONTENTS:1;
 	unsigned DIRTY_SUBMODULES:1;
 	unsigned IGNORE_UNTRACKED_IN_SUBMODULES:1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Stefan Beller

2017-10-30 20:41 UTC #20 lore raw

Re: [PATCH v2 3/4] diff: add flag to indicate textconv was set via cmdline

Quoted 11 lines · Brandon Williams

git-show is unique in that it wants to use textconv by default except for when it is showing blobs. When asked to show a blob, show doesn't want to use textconv unless the user explicitly requested that it be used by providing the command line flag '--textconv'.
Currently this is done by using a parallel set of 'touched' flags which get set every time a particular flag is set or cleared. In a future patch we want to eliminate this parallel set of flags so instead of relying on if the textconv flag has been touched, add a new flag 'TEXTCONV_SET_VIA_CMDLINE' which is only set if textconv is requested via the command line.

Is it worth mentioning 4197361e39 (Merge branch 'mg/more-textconv', 2013-10-23), that introduced the touched_flags?
(+cc Michael Gruber FYI)

Quoted 25 lines

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/log.c | 2 +-
 diff.c        | 8 +++++---
 diff.h        | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)

builtin/log.c +1 -1

index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
        unsigned long size;
 
        fflush(rev->diffopt.file);
-       if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+       if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
            !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
                return stream_blob_to_fd(1, oid, NULL, 0);

diff.c +0 -0

index 3ad9c9b31..8b700b1bd 100644

diff.c int diff_opt_parse(struct diff_options *options, · line 4769

Quoted 10 lines

diff.c +3 -2

@@ -4762,11 +4762,13 @@ int diff_opt_parse(struct diff_options *options,
                DIFF_OPT_SET(options, ALLOW_EXTERNAL);
        else if (!strcmp(arg, "--no-ext-diff"))
                DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
-       else if (!strcmp(arg, "--textconv"))
+       else if (!strcmp(arg, "--textconv")) {
                DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-       else if (!strcmp(arg, "--no-textconv"))
+               DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--no-textconv")) {
                DIFF_OPT_CLR(options, ALLOW_TEXTCONV);

Also clear TEXTCONV_SET_VIA_CMDLINE here? (git show --textconv --no-textconv might act funny?)

Quoted 21 lines

diff.c +2 -1

-       else if (!strcmp(arg, "--ignore-submodules")) {
+               DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--ignore-submodules")) {
                DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
                handle_ignore_submodules_arg(options, "all");
        } else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

diff.h +1 -1

index 47e6d43cb..4eaf9b370 100644
@@ -83,6 +83,7 @@ struct diff_flags {
        unsigned DIRSTAT_CUMULATIVE:1;
        unsigned DIRSTAT_BY_FILE:1;
        unsigned ALLOW_TEXTCONV:1;
+       unsigned TEXTCONV_SET_VIA_CMDLINE:1;
        unsigned DIFF_FROM_CONTENTS:1;
        unsigned DIRTY_SUBMODULES:1;
        unsigned IGNORE_UNTRACKED_IN_SUBMODULES:1;
--

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 20:44 UTC #21 lore raw

Re: [PATCH v2 3/4] diff: add flag to indicate textconv was set via cmdline

Quoted 47 lines · Stefan Beller

Quoted 11 lines · Brandon Williams
git-show is unique in that it wants to use textconv by default except for when it is showing blobs. When asked to show a blob, show doesn't want to use textconv unless the user explicitly requested that it be used by providing the command line flag '--textconv'.
Currently this is done by using a parallel set of 'touched' flags which get set every time a particular flag is set or cleared. In a future patch we want to eliminate this parallel set of flags so instead of relying on if the textconv flag has been touched, add a new flag 'TEXTCONV_SET_VIA_CMDLINE' which is only set if textconv is requested via the command line.
Is it worth mentioning 4197361e39 (Merge branch 'mg/more-textconv', 2013-10-23), that introduced the touched_flags?
(+cc Michael Gruber FYI)
Quoted 30 lines
Signed-off-by Brandon Williams <[email protected]>
---
 builtin/log.c | 2 +-
 diff.c        | 8 +++++---
 diff.h        | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)
builtin/log.c +1 -1
index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
        unsigned long size;
 
        fflush(rev->diffopt.file);
-       if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+       if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
            !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
                return stream_blob_to_fd(1, oid, NULL, 0);
 
diff.c +1 -1
index 3ad9c9b31..8b700b1bd 100644
@@ -4762,11 +4762,13 @@ int diff_opt_parse(struct diff_options *options,
                DIFF_OPT_SET(options, ALLOW_EXTERNAL);
        else if (!strcmp(arg, "--no-ext-diff"))
                DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
-       else if (!strcmp(arg, "--textconv"))
+       else if (!strcmp(arg, "--textconv")) {

Quoted 8 lines

diff.c int diff_opt_parse(struct diff_options *options, · line 4769
Quoted 5 lines
diff.c +2 -1
                DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-       else if (!strcmp(arg, "--no-textconv"))
+               DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--no-textconv")) {
                DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
Also clear TEXTCONV_SET_VIA_CMDLINE here? (git show --textconv --no-textconv might act funny?)

Oops, thanks for catching this, I added it in the wrong place! (as you can see below.

Quoted 22 lines

diff.c +2 -1

-       else if (!strcmp(arg, "--ignore-submodules")) {
+               DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--ignore-submodules")) {
                DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
                handle_ignore_submodules_arg(options, "all");
        } else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

diff.h +1 -1

index 47e6d43cb..4eaf9b370 100644
@@ -83,6 +83,7 @@ struct diff_flags {
        unsigned DIRSTAT_CUMULATIVE:1;
        unsigned DIRSTAT_BY_FILE:1;
        unsigned ALLOW_TEXTCONV:1;
+       unsigned TEXTCONV_SET_VIA_CMDLINE:1;
        unsigned DIFF_FROM_CONTENTS:1;
        unsigned DIRTY_SUBMODULES:1;
        unsigned IGNORE_UNTRACKED_IN_SUBMODULES:1;
--

2.15.0.403.gc27cc4dac6-goog

Signature · 1 line

Brandon Williams

Brandon Williams

author

2017-10-30 20:48 UTC #22 lore raw

Re: [PATCH v2 3/4] diff: add flag to indicate textconv was set via cmdline

Quoted 57 lines · Brandon Williams

Quoted 47 lines · Stefan Beller
Quoted 11 lines · Brandon Williams
git-show is unique in that it wants to use textconv by default except for when it is showing blobs. When asked to show a blob, show doesn't want to use textconv unless the user explicitly requested that it be used by providing the command line flag '--textconv'.
Currently this is done by using a parallel set of 'touched' flags which get set every time a particular flag is set or cleared. In a future patch we want to eliminate this parallel set of flags so instead of relying on if the textconv flag has been touched, add a new flag 'TEXTCONV_SET_VIA_CMDLINE' which is only set if textconv is requested via the command line.
Is it worth mentioning 4197361e39 (Merge branch 'mg/more-textconv', 2013-10-23), that introduced the touched_flags?
(+cc Michael Gruber FYI)
Quoted 30 lines
Signed-off-by Brandon Williams <[email protected]>
---
 builtin/log.c | 2 +-
 diff.c        | 8 +++++---
 diff.h        | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)
builtin/log.c +1 -1
index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
        unsigned long size;
 
        fflush(rev->diffopt.file);
-       if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+       if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
            !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
                return stream_blob_to_fd(1, oid, NULL, 0);
 
diff.c +1 -1
index 3ad9c9b31..8b700b1bd 100644
@@ -4762,11 +4762,13 @@ int diff_opt_parse(struct diff_options *options,
                DIFF_OPT_SET(options, ALLOW_EXTERNAL);
        else if (!strcmp(arg, "--no-ext-diff"))
                DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
-       else if (!strcmp(arg, "--textconv"))
+       else if (!strcmp(arg, "--textconv")) {
Quoted 8 lines
diff.c int diff_opt_parse(struct diff_options *options, · line 4769
Quoted 5 lines
diff.c +2 -1
                DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-       else if (!strcmp(arg, "--no-textconv"))
+               DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--no-textconv")) {
                DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
Also clear TEXTCONV_SET_VIA_CMDLINE here? (git show --textconv --no-textconv might act funny?)

Quoted 2 lines

Oops, thanks for catching this, I added it in the wrong place! (as you can see below.

wait nevermind, i overreacted. And this patch does correctly clear TEXTCONV_SET_VIA_CMDLINE.

Quoted 26 lines

Quoted 23 lines

diff.c +2 -1

-       else if (!strcmp(arg, "--ignore-submodules")) {
+               DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+       } else if (!strcmp(arg, "--ignore-submodules")) {
                DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
                handle_ignore_submodules_arg(options, "all");
        } else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

diff.h +1 -1

index 47e6d43cb..4eaf9b370 100644
@@ -83,6 +83,7 @@ struct diff_flags {
        unsigned DIRSTAT_CUMULATIVE:1;
        unsigned DIRSTAT_BY_FILE:1;
        unsigned ALLOW_TEXTCONV:1;
+       unsigned TEXTCONV_SET_VIA_CMDLINE:1;
        unsigned DIFF_FROM_CONTENTS:1;
        unsigned DIRTY_SUBMODULES:1;
        unsigned IGNORE_UNTRACKED_IN_SUBMODULES:1;
--

2.15.0.403.gc27cc4dac6-goog

Signature · 1 line

Brandon Williams

Signature · 1 line

Brandon Williams

Junio C Hamano

2017-10-31 05:02 UTC #23 lore raw

Re: [PATCH v2 3/4] diff: add flag to indicate textconv was set via cmdline

In reply to Brandon Williams

builtin/log.c static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c · line 490

Quoted 12 lines · Brandon Williams

builtin/log.c +1 -1

index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+	if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
 	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
 		return stream_blob_to_fd(1, oid, NULL, 0);

The original is equivalent to

	if (! (DIFF_OPT_TOUCHED() && DIFF_OPT_TST()))
		return stream_blob_to_fd();

which means that we must have used DIFF_OPT_SET() or DIFF_OPT_CLR() to touch the ALLOW_TEXTCONV bit, and ALLOW_TEXTCONV bit is currently set, in order for the flow to skip this "just stream it out".

And the way it implemented it was:

#define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) (((opts)->flags |= DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
#define DIFF_OPT_CLR(opts, flag) (((opts)->flags &= ~DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))

Notice that touched_flags is SET in both OPT_SET() and OPT_CLR(), because the point of _TOUCHED() was "did the user made an explicit request to affect the value of the bit from the command line?".

Quoted 8 lines

diff.c +0 -0

index 3ad9c9b31..8b700b1bd 100644
@@ -4762,11 +4762,13 @@ int diff_opt_parse(struct diff_options *options,
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);

diff.c int diff_opt_parse(struct diff_options *options, · line 4771

Quoted 10 lines

diff.c +5 -3

-	else if (!strcmp(arg, "--textconv"))
+	else if (!strcmp(arg, "--textconv")) {
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--no-textconv"))
+		DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+	} else if (!strcmp(arg, "--no-textconv")) {
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--ignore-submodules")) {
+		DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+	} else if (!strcmp(arg, "--ignore-submodules")) {

If we were aiming for faithful conversion, the above must be DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE), not CLR.

HOWEVER, I think it is fine to define TEXTCONV_SET_VIA_CMDLINE bit differently from what DIFF_OPT_TOUCHED(ALLOW_TEXTCONV) meant in the old code (i.e. "did the user made an explicit request to affect the value?"). That is, we can define the new one as "did the user explicitly SET the bit from the command line?", the conditional in show_blob_object() is prepared to take either interpretation. "User explicitly set the bit to true, and the bit is true" and "User explicitly set the bit to something, and the bit is true" are pretty much the same thing.

And that leads me to suggest dropping the last change here, to touch VIA_CMDLINE in response to "--no-textconv".

Other than that, looks good to me.

Thanks.

Junio C Hamano

2017-10-31 05:23 UTC #24 lore raw

PATCH 1/1 diff: set TEXTCONV_VIA_CMDLINE only when it is set to true

In reply to Brandon Williams

Change the meaning of the bit to "the user explicitly set the allow-textconv bit to true from the command line".

The "touched" mechanism in the old code meant to express "the user explicitly set the allow-textconv bit to something from the command line" and recorded that fact upon "--no-textconv", too, by setting the corresponding touched bit. The code in the previous step to clear the bit did not make much sense.

Again, this may want be squashed into the previous step, but its log message needs to be adjusted somewhat (e.g. "s/is requested via/is set to true via/").

Signed-off-by Junio C Hamano <[email protected]>

---
 diff.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff.c +2 -3

index 8b700b1bd2..11fccbd107 100644
@@ -4765,10 +4765,9 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--textconv")) {
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
 		DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
-	} else if (!strcmp(arg, "--no-textconv")) {
+	} else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-		DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
-	} else if (!strcmp(arg, "--ignore-submodules")) {
+	else if (!strcmp(arg, "--ignore-submodules")) {
 		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, "all");
 	} else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

Signature · 1 line

2.15.0-224-g5109123e6a

Brandon Williams

author

2017-10-31 17:55 UTC #25 lore raw

Re: [PATCH 3.5/4] diff: set TEXTCONV_VIA_CMDLINE only when it is set to true

Quoted 9 lines · Junio C Hamano

Change the meaning of the bit to "the user explicitly set the allow-textconv bit to true from the command line".
The "touched" mechanism in the old code meant to express "the user explicitly set the allow-textconv bit to something from the command line" and recorded that fact upon "--no-textconv", too, by setting the corresponding touched bit. The code in the previous step to clear the bit did not make much sense.

Quoted 3 lines

Again, this may want be squashed into the previous step, but its log message needs to be adjusted somewhat (e.g. "s/is requested via/is set to true via/").

I don't have any opinions on this, but I agree that if we want a more true conversion then we would wanted to squash this in, which I'll do and update the log message.

Quoted 26 lines

Signed-off-by Junio C Hamano <[email protected]>

---
 diff.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff.c +2 -3

index 8b700b1bd2..11fccbd107 100644
@@ -4765,10 +4765,9 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--textconv")) {
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
 		DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
-	} else if (!strcmp(arg, "--no-textconv")) {
+	} else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-		DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
-	} else if (!strcmp(arg, "--ignore-submodules")) {
+	else if (!strcmp(arg, "--ignore-submodules")) {
 		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, "all");
 	} else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {

Signature · 1 line

2.15.0-224-g5109123e6a

Signature · 1 line

Brandon Williams

Brandon Williams

author

2017-10-30 19:47 UTC #26 lore raw

PATCH v2 4/4 diff: remove touched flags

In reply to Brandon Williams

Now that the set of parallel touched flags are no longer being used, remove them.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/log.c | 2 --
 diff.h        | 6 ++----
 2 files changed, 2 insertions(+), 6 deletions(-)

builtin/log.c +0 -2

index 82131751d..9c0815270 100644
@@ -134,8 +134,6 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-
-	memset(&rev->diffopt.touched_flags, 0, sizeof(struct diff_flags));
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

diff.h +2 -4

index 4eaf9b370..7e567108e 100644
@@ -111,9 +111,8 @@ static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
 }
 
 #define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
-#define DIFF_OPT_TOUCHED(opts, flag)	((opts)->touched_flags.flag)
-#define DIFF_OPT_SET(opts, flag)	(((opts)->flags.flag = 1),((opts)->touched_flags.flag = 1))
-#define DIFF_OPT_CLR(opts, flag)	(((opts)->flags.flag = 0),((opts)->touched_flags.flag = 1))
+#define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
+#define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
 
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)

@@ -142,7 +141,6 @@ struct diff_options {
 	const char *line_prefix;
 	size_t line_prefix_length;
 	struct diff_flags flags;
-	struct diff_flags touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 22:20 UTC #27 lore raw

[PATCH v2 5/4] diff: remove DIFF_OPT_TST macro

In reply to Brandon Williams

Remove the DIFF_OPT_TST macro and instead access the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_TST(&E, fld)
	+ E.flags.fld

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_TST(ptr, fld)
	+ ptr->flags.fld

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c            |  8 +++---
 builtin/am.c       |  2 +-
 builtin/blame.c    |  4 +--
 builtin/diff.c     |  2 +-
 builtin/log.c      | 12 ++++-----
 builtin/rev-list.c |  2 +-
 combine-diff.c     |  6 ++---
 diff-lib.c         | 19 +++++++-------
 diff-no-index.c    |  2 +-
 diff.c             | 76 +++++++++++++++++++++++++++---------------------------
 diff.h             |  1 -
 diffcore-pickaxe.c |  8 +++---
 diffcore-rename.c  |  6 ++---
 log-tree.c         |  2 +-
 revision.c         |  4 +--
 submodule.c        |  2 +-
 tree-diff.c        | 12 ++++-----
 17 files changed, 84 insertions(+), 84 deletions(-)

blame.c +4 -4

index f575e9cbf..7c019bc7c 100644
@@ -209,7 +209,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
-			if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+			if (opt->flags.ALLOW_TEXTCONV &&
 			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
 				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
 			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)

@@ -293,7 +293,7 @@ static void fill_origin_blob(struct diff_options *opt,
 		unsigned long file_size;
 
 		(*num_read_blob)++;
-		if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+		if (opt->flags.ALLOW_TEXTCONV &&
 		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
 			;
 		else

@@ -1262,7 +1262,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 			      &target->commit->tree->object.oid,
 			      "", &diff_opts);
 
-	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
+	if (!diff_opts.flags.FIND_COPIES_HARDER)
 		diffcore_std(&diff_opts);
 
 	do {

@@ -1825,7 +1825,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 		if (fill_blob_sha1_and_mode(o))
 			die(_("no such path %s in %s"), path, final_commit_name);
 
-		if (DIFF_OPT_TST(&sb->revs->diffopt, ALLOW_TEXTCONV) &&
+		if (sb->revs->diffopt.flags.ALLOW_TEXTCONV &&
 		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
 				    &sb->final_buf_size))
 			;

builtin/am.c +1 -1

index d7513f537..fc54724cc 100644
@@ -1168,7 +1168,7 @@ static int index_has_changes(struct strbuf *sb)
 			strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
 		}
 		diff_flush(&opt);
-		return DIFF_OPT_TST(&opt, HAS_CHANGES) != 0;
+		return opt.flags.HAS_CHANGES != 0;
 	} else {
 		for (i = 0; sb && i < active_nr; i++) {
 			if (i)

builtin/blame.c +2 -2

index 67adaef4d..a29574984 100644
@@ -734,7 +734,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
-	no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
+	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
 	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
 	argc = parse_options_end(&ctx);

@@ -803,7 +803,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 	blame_date_width -= 1; /* strip the null */
 
-	if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
+	if (revs.diffopt.flags.FIND_COPIES_HARDER)
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);

builtin/diff.c +1 -1

index f5bbd4d75..8cbaf4538 100644
@@ -44,7 +44,7 @@ static void stuff_change(struct diff_options *opt,
 	    !oidcmp(old_oid, new_oid) && (old_mode == new_mode))
 		return;
 
-	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
+	if (opt->flags.REVERSE_DIFF) {
 		SWAP(old_mode, new_mode);
 		SWAP(old_oid, new_oid);
 		SWAP(old_path, new_path);

builtin/log.c +6 -6

index 9c0815270..2b2d36678 100644
@@ -181,7 +181,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		init_display_notes(&rev->notes_opt);
 
 	if (rev->diffopt.pickaxe || rev->diffopt.filter ||
-	    DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES))
+	    rev->diffopt.flags.FOLLOW_RENAMES)
 		rev->always_show_header = 0;
 
 	if (source)

@@ -391,7 +391,7 @@ static int cmd_log_walk(struct rev_info *rev)
 		fclose(rev->diffopt.file);
 
 	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
-	    DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) {
+	    rev->diffopt.flags.CHECK_FAILED) {
 		return 02;
 	}
 	return diff_result_code(&rev->diffopt, 0);

@@ -483,8 +483,8 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
-	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
+	if (!rev->diffopt.flags.TEXTCONV_SET_VIA_CMDLINE ||
+	    !rev->diffopt.flags.ALLOW_TEXTCONV)
 		return stream_blob_to_fd(1, oid, NULL, 0);
 
 	if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,

@@ -666,7 +666,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 static void log_setup_revisions_tweak(struct rev_info *rev,
 				      struct setup_revision_opt *opt)
 {
-	if (DIFF_OPT_TST(&rev->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
 	    rev->prune_data.nr == 1)
 		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);

@@ -1612,7 +1612,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.zero_commit = zero_commit;
 
-	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
+	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
 	if (rev.show_notes)

builtin/rev-list.c +1 -1

index c1c74d4a7..c62382171 100644
@@ -294,7 +294,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (revs.bisect)
 		bisect_list = 1;
 
-	if (DIFF_OPT_TST(&revs.diffopt, QUICK))
+	if (revs.diffopt.flags.QUICK)
 		info.flags |= REV_LIST_QUIET;
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];

combine-diff.c +3 -3

index 9e163d5ad..40cb4dd1a 100644
@@ -898,7 +898,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 				 int show_file_header)
 {
 	struct diff_options *opt = &rev->diffopt;
-	int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
+	int abbrev = opt->flags.FULL_INDEX ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
 	const char *a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
 	const char *b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
 	const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);

@@ -987,7 +987,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	userdiff = userdiff_find_by_path(elem->path);
 	if (!userdiff)
 		userdiff = userdiff_find_by_name("default");
-	if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV))
+	if (opt->flags.ALLOW_TEXTCONV)
 		textconv = userdiff_get_textconv(userdiff);
 
 	/* Read the result of merge first */

@@ -1435,7 +1435,7 @@ void diff_tree_combined(const struct object_id *oid,
 	 * NOTE please keep this semantically in sync with diffcore_std()
 	 */
 	need_generic_pathscan = opt->skip_stat_unmatch	||
-			DIFF_OPT_TST(opt, FOLLOW_RENAMES)	||
+			opt->flags.FOLLOW_RENAMES	||
 			opt->break_opt != -1	||
 			opt->detect_rename	||
 			opt->pickaxe		||

diff-lib.c +10 -9

index 6c1c05c5b..d6b711617 100644
@@ -72,13 +72,14 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct diff_flags orig_flags = diffopt->flags;
-		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
+		if (!diffopt->flags.OVERRIDE_SUBMODULE_CONFIG)
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
-		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))
+		if (diffopt->flags.IGNORE_SUBMODULES)
 			changed = 0;
-		else if (!DIFF_OPT_TST(diffopt, IGNORE_DIRTY_SUBMODULES)
-		    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES)))
-			*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
+		else if (!diffopt->flags.IGNORE_DIRTY_SUBMODULES &&
+			 (!changed || diffopt->flags.DIRTY_SUBMODULES))
+			*dirty_submodule = is_submodule_modified(ce->name,
+								 diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES);
 		diffopt->flags = orig_flags;
 	}
 	return changed;

@@ -228,7 +229,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 
 		if (!changed && !dirty_submodule) {
 			ce_mark_uptodate(ce);
-			if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+			if (!revs->diffopt.flags.FIND_COPIES_HARDER)
 				continue;
 		}
 		oldmode = ce->ce_mode;

@@ -362,7 +363,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !oidcmp(oid, &old->oid) && !dirty_submodule &&
-	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+	    !revs->diffopt.flags.FIND_COPIES_HARDER)
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,

@@ -493,7 +494,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.head_idx = 1;
 	opts.index_only = cached;
 	opts.diff_index_cached = (cached &&
-				  !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER));
+				  !revs->diffopt.flags.FIND_COPIES_HARDER);
 	opts.merge = 1;
 	opts.fn = oneway_diff;
 	opts.unpack_data = revs;

@@ -551,5 +552,5 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);
-	return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);
+	return (rev.diffopt.flags.HAS_CHANGES != 0);
 }

diff-no-index.c +1 -1

index 80ff17d46..40d17e42c 100644
@@ -184,7 +184,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		if (o->flags.REVERSE_DIFF) {
 			SWAP(mode1, mode2);
 			SWAP(name1, name2);
 		}

diff.c +38 ~~-38~~

260 lines not on this page · raw message

diff.h +0 -1

index 7e567108e..a3656ff5b 100644
@@ -110,7 +110,6 @@ static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
 	return out;
 }
 
-#define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
 #define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
 #define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)

diffcore-pickaxe.c +4 -4

index 341529b5a..6193c07f6 100644
@@ -131,7 +131,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	if (!DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two))
 		return 0;
 
-	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+	if (o->flags.ALLOW_TEXTCONV) {
 		textconv_one = get_textconv(p->one);
 		textconv_two = get_textconv(p->two);
 	}

@@ -222,11 +222,11 @@ void diffcore_pickaxe(struct diff_options *o)
 
 	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
 		int cflags = REG_EXTENDED | REG_NEWLINE;
-		if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
+		if (o->flags.PICKAXE_IGNORE_CASE)
 			cflags |= REG_ICASE;
 		regcomp_or_die(&regex, needle, cflags);
 		regexp = &regex;
-	} else if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) &&
+	} else if (o->flags.PICKAXE_IGNORE_CASE &&
 		   has_non_ascii(needle)) {
 		struct strbuf sb = STRBUF_INIT;
 		int cflags = REG_NEWLINE | REG_ICASE;

@@ -236,7 +236,7 @@ void diffcore_pickaxe(struct diff_options *o)
 		strbuf_release(&sb);
 		regexp = &regex;
 	} else {
-		kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
+		kws = kwsalloc(o->flags.PICKAXE_IGNORE_CASE
 			       ? tolower_trans_tbl : NULL);
 		kwsincr(kws, needle, strlen(needle));
 		kwsprep(kws);

diffcore-rename.c +3 -3

index 0d8c3d2ee..bd077ee11 100644
@@ -405,7 +405,7 @@ static int too_many_rename_candidates(int num_create,
 		num_src > num_create ? num_src : num_create;
 
 	/* Are we running under -C -C? */
-	if (!DIFF_OPT_TST(options, FIND_COPIES_HARDER))
+	if (!options->flags.FIND_COPIES_HARDER)
 		return 1;
 
 	/* Would we bust the limit if we were running under -C? */

@@ -463,7 +463,7 @@ void diffcore_rename(struct diff_options *options)
 			else if (options->single_follow &&
 				 strcmp(options->single_follow, p->two->path))
 				continue; /* not interested */
-			else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
+			else if (!options->flags.RENAME_EMPTY &&
 				 is_empty_blob_oid(&p->two->oid))
 				continue;
 			else if (add_rename_dst(p->two) < 0) {

@@ -473,7 +473,7 @@ void diffcore_rename(struct diff_options *options)
 				goto cleanup;
 			}
 		}
-		else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
+		else if (!options->flags.RENAME_EMPTY &&
 			 is_empty_blob_oid(&p->one->oid))
 			continue;
 		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {

log-tree.c +1 -1

index cea056234..460bb5498 100644
@@ -793,7 +793,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	struct commit_list *parents;
 	struct object_id *oid;
 
-	if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
+	if (!opt->diff && !opt->diffopt.flags.EXIT_WITH_STATUS)
 		return 0;
 
 	parse_commit_or_die(commit);

revision.c +2 -2

index d167223e6..c893d9166 100644
@@ -2399,7 +2399,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	/* Pickaxe, diff-filter and rename following need diffs */
 	if (revs->diffopt.pickaxe ||
 	    revs->diffopt.filter ||
-	    DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
+	    revs->diffopt.flags.FOLLOW_RENAMES)
 		revs->diff = 1;
 
 	if (revs->topo_order)

@@ -2408,7 +2408,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->prune_data.nr) {
 		copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
+		if (!revs->diffopt.flags.FOLLOW_RENAMES)
 			revs->prune = 1;
 		if (!revs->full_diff)
 			copy_pathspec(&revs->diffopt.pathspec,

submodule.c +1 -1

index 63e7094e1..58da9c668 100644
@@ -616,7 +616,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 	argv_array_pushf(&cp.args, "--color=%s", want_color(o->use_color) ?
 			 "always" : "never");
 
-	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+	if (o->flags.REVERSE_DIFF) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
 				 o->b_prefix, path);
 		argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",

tree-diff.c +6 -6

index 4bb93155b..ed0aa6a62 100644
@@ -212,9 +212,9 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 		mode = 0;
 	}
 
-	if (DIFF_OPT_TST(opt, RECURSIVE) && isdir) {
+	if (opt->flags.RECURSIVE && isdir) {
 		recurse = 1;
-		emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE);
+		emitthis = opt->flags.TREE_IN_RECURSIVE;
 	}
 
 	if (emitthis) {

@@ -425,7 +425,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	ttree = fill_tree_descriptor(&t, oid);
 
 	/* Enable recursion indefinitely */
-	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
+	opt->pathspec.recursive = opt->flags.RECURSIVE;
 
 	for (;;) {
 		int imin, cmp;

@@ -484,7 +484,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t = p[imin] */
 		if (cmp == 0) {
 			/* are either pi > p[imin] or diff(t,pi) != ø ? */
-			if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER)) {
+			if (!opt->flags.FIND_COPIES_HARDER) {
 				for (i = 0; i < nparent; ++i) {
 					/* p[i] > p[imin] */
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)

@@ -522,7 +522,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t > p[imin] */
 		else {
 			/* ∀i pi=p[imin] -> D += "-p[imin]" */
-			if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER)) {
+			if (!opt->flags.FIND_COPIES_HARDER) {
 				for (i = 0; i < nparent; ++i)
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)
 						goto skip_emit_tp;

@@ -706,7 +706,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 	strbuf_addstr(&base, base_str);
 
 	retval = ll_diff_tree_oid(old_oid, new_oid, &base, opt);
-	if (!*base_str && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
+	if (!*base_str && opt->flags.FOLLOW_RENAMES && diff_might_be_rename())
 		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
 	strbuf_release(&base);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 22:20 UTC #28 lore raw

[PATCH v2 7/4] diff: remove DIFF_OPT_CLR macro

Remove the DIFF_OPT_SET macro and instead set the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_CLR(&E, fld)
	+ E.flags.fld = 0

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_CLR(ptr, fld)
	+ ptr->flags.fld = 0

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/blame.c   |  2 +-
 combine-diff.c    |  2 +-
 diff.c            | 30 +++++++++++++++---------------
 diff.h            |  2 --
 merge-recursive.c |  2 +-
 revision.c        |  4 ++--
 submodule.c       |  6 +++---
 7 files changed, 23 insertions(+), 25 deletions(-)

builtin/blame.c +1 -1

index 76994aa64..79db9e849 100644
@@ -736,7 +736,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
-	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
+	revs.diffopt.flags.FOLLOW_RENAMES = 0;
 	argc = parse_options_end(&ctx);
 
 	if (incremental || (output_option & OUTPUT_PORCELAIN)) {

combine-diff.c +1 -1

index 204b0dfce..5a3a8b49b 100644
@@ -1414,7 +1414,7 @@ void diff_tree_combined(const struct object_id *oid,
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
 	diffopts.flags.RECURSIVE = 1;
-	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
+	diffopts.flags.ALLOW_EXTERNAL = 0;
 
 	/* find set of paths that everybody touches
 	 *

diff.c +15 ~~-15~~

index 16d33c319..0e5abb5ce 100644
@@ -124,16 +124,16 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
 	for (i = 0; i < params.nr; i++) {
 		const char *p = params.items[i].string;
 		if (!strcmp(p, "changes")) {
-			DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
-			DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
+			options->flags.DIRSTAT_BY_LINE = 0;
+			options->flags.DIRSTAT_BY_FILE = 0;
 		} else if (!strcmp(p, "lines")) {
 			options->flags.DIRSTAT_BY_LINE = 1;
-			DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
+			options->flags.DIRSTAT_BY_FILE = 0;
 		} else if (!strcmp(p, "files")) {
-			DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
+			options->flags.DIRSTAT_BY_LINE = 0;
 			options->flags.DIRSTAT_BY_FILE = 1;
 		} else if (!strcmp(p, "noncumulative")) {
-			DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
+			options->flags.DIRSTAT_CUMULATIVE = 0;
 		} else if (!strcmp(p, "cumulative")) {
 			options->flags.DIRSTAT_CUMULATIVE = 1;
 		} else if (isdigit(*p)) {

@@ -4205,7 +4205,7 @@ void diff_setup_done(struct diff_options *options)
 	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
 		options->flags.DIFF_FROM_CONTENTS = 1;
 	else
-		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
+		options->flags.DIFF_FROM_CONTENTS = 0;
 
 	if (options->flags.FIND_COPIES_HARDER)
 		options->detect_rename = DIFF_DETECT_COPY;

@@ -4640,7 +4640,7 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--rename-empty"))
 		options->flags.RENAME_EMPTY = 1;
 	else if (!strcmp(arg, "--no-rename-empty"))
-		DIFF_OPT_CLR(options, RENAME_EMPTY);
+		options->flags.RENAME_EMPTY = 0;
 	else if (!strcmp(arg, "--relative"))
 		options->flags.RELATIVE_NAME = 1;
 	else if (skip_prefix(arg, "--relative=", &arg)) {

@@ -4697,8 +4697,8 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--follow"))
 		options->flags.FOLLOW_RENAMES = 1;
 	else if (!strcmp(arg, "--no-follow")) {
-		DIFF_OPT_CLR(options, FOLLOW_RENAMES);
-		DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+		options->flags.FOLLOW_RENAMES = 0;
+		options->flags.DEFAULT_FOLLOW_RENAMES = 0;
 	} else if (!strcmp(arg, "--color"))
 		options->use_color = 1;
 	else if (skip_prefix(arg, "--color=", &arg)) {

@@ -4761,13 +4761,13 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--ext-diff"))
 		options->flags.ALLOW_EXTERNAL = 1;
 	else if (!strcmp(arg, "--no-ext-diff"))
-		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+		options->flags.ALLOW_EXTERNAL = 0;
 	else if (!strcmp(arg, "--textconv")) {
 		options->flags.ALLOW_TEXTCONV = 1;
 		options->flags.TEXTCONV_SET_VIA_CMDLINE = 1;
 	} else if (!strcmp(arg, "--no-textconv")) {
-		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-		DIFF_OPT_CLR(options, TEXTCONV_SET_VIA_CMDLINE);
+		options->flags.ALLOW_TEXTCONV = 0;
+		options->flags.TEXTCONV_SET_VIA_CMDLINE = 0;
 	} else if (!strcmp(arg, "--ignore-submodules")) {
 		options->flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 		handle_ignore_submodules_arg(options, "all");

@@ -4851,7 +4851,7 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--function-context"))
 		options->flags.FUNCCONTEXT = 1;
 	else if (!strcmp(arg, "--no-function-context"))
-		DIFF_OPT_CLR(options, FUNCCONTEXT);
+		options->flags.FUNCCONTEXT = 0;
 	else if ((argcount = parse_long_opt("output", av, &optarg))) {
 		char *path = prefix_filename(prefix, optarg);
 		options->file = xfopen(path, "w");

@@ -5689,7 +5689,7 @@ void diff_flush(struct diff_options *options)
 		if (options->found_changes)
 			options->flags.HAS_CHANGES = 1;
 		else
-			DIFF_OPT_CLR(options, HAS_CHANGES);
+			options->flags.HAS_CHANGES = 0;
 	}
 }

@@ -5861,7 +5861,7 @@ void diffcore_std(struct diff_options *options)
 	if (diff_queued_diff.nr && !options->flags.DIFF_FROM_CONTENTS)
 		options->flags.HAS_CHANGES = 1;
 	else
-		DIFF_OPT_CLR(options, HAS_CHANGES);
+		options->flags.HAS_CHANGES = 0;
 
 	options->found_follow = 0;
 }

diff.h +0 -2

index 4c1af4be6..f68fcf737 100644
@@ -110,8 +110,6 @@ static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
 	return out;
 }
 
-#define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
-
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)

merge-recursive.c +1 -1

index e7b3df45c..9752aba4e 100644
@@ -541,7 +541,7 @@ static struct string_list *get_renames(struct merge_options *o,
 
 	diff_setup(&opts);
 	opts.flags.RECURSIVE = 1;
-	DIFF_OPT_CLR(&opts, RENAME_EMPTY);
+	opts.flags.RENAME_EMPTY = 0;
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
 			    o->diff_rename_limit >= 0 ? o->diff_rename_limit :

revision.c +2 -2

index f019dd277..6bb873501 100644
@@ -455,7 +455,7 @@ static int rev_compare_tree(struct rev_info *revs,
 	}
 
 	tree_difference = REV_TREE_SAME;
-	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
+	revs->pruning.flags.HAS_CHANGES = 0;
 	if (diff_tree_oid(&t1->object.oid, &t2->object.oid, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;

@@ -471,7 +471,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 		return 0;
 
 	tree_difference = REV_TREE_SAME;
-	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
+	revs->pruning.flags.HAS_CHANGES = 0;
 	retval = diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
 
 	return retval >= 0 && (tree_difference == REV_TREE_SAME);

submodule.c +3 -3

index 7e7998592..62a93bb88 100644
@@ -402,9 +402,9 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
-	DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
-	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
+	diffopt->flags.IGNORE_SUBMODULES = 0;
+	diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 0;
+	diffopt->flags.IGNORE_DIRTY_SUBMODULES = 0;
 
 	if (!strcmp(arg, "all"))
 		diffopt->flags.IGNORE_SUBMODULES = 1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 22:20 UTC #29 lore raw

[PATCH v2 6/4] diff: remove DIFF_OPT_SET macro

In reply to Brandon Williams

Remove the DIFF_OPT_SET macro and instead set the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_SET(&E, fld)
	+ E.flags.fld = 1

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_SET(ptr, fld)
	+ ptr->flags.fld = 1

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c               |  8 +++----
 builtin/add.c         |  4 ++--
 builtin/am.c          |  8 +++----
 builtin/blame.c       |  4 ++--
 builtin/diff.c        |  6 ++---
 builtin/fast-export.c |  2 +-
 builtin/log.c         | 14 +++++------
 builtin/reset.c       |  2 +-
 combine-diff.c        |  2 +-
 diff-lib.c            |  4 ++--
 diff-no-index.c       |  6 ++---
 diff.c                | 66 +++++++++++++++++++++++++--------------------------
 diff.h                |  1 -
 merge-recursive.c     |  2 +-
 notes-merge.c         |  4 ++--
 patch-ids.c           |  2 +-
 revision.c            | 16 ++++++-------
 submodule.c           |  8 +++----
 tree-diff.c           |  4 ++--
 wt-status.c           | 18 +++++++-------
 20 files changed, 90 insertions(+), 91 deletions(-)

blame.c +4 -4

index 7c019bc7c..dc9cc237b 100644
@@ -541,7 +541,7 @@ static struct blame_origin *find_origin(struct commit *parent,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;

@@ -615,7 +615,7 @@ static struct blame_origin *find_rename(struct commit *parent,
 	int i;
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;

@@ -1238,7 +1238,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		return; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	diff_setup_done(&diff_opts);

@@ -1253,7 +1253,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
+		diff_opts.flags.FIND_COPIES_HARDER = 1;
 
 	if (is_null_oid(&target->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);

builtin/add.c +2 -2

index b70e8a779..e1d83b69a 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

@@ -218,7 +218,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev.diffopt.use_color = 0;
-	DIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES);
+	rev.diffopt.flags.IGNORE_DIRTY_SUBMODULES = 1;
 	out = open(file, O_CREAT | O_WRONLY, 0666);
 	if (out < 0)
 		die(_("Could not open '%s' for writing."), file);

builtin/am.c +4 -4

index fc54724cc..015425a0f 100644
@@ -1157,9 +1157,9 @@ static int index_has_changes(struct strbuf *sb)
 		struct diff_options opt;
 
 		diff_setup(&opt);
-		DIFF_OPT_SET(&opt, EXIT_WITH_STATUS);
+		opt.flags.EXIT_WITH_STATUS = 1;
 		if (!sb)
-			DIFF_OPT_SET(&opt, QUICK);
+			opt.flags.QUICK = 1;
 		do_diff_cache(&head, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {

@@ -1409,8 +1409,8 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
 	rev_info.show_root_diff = 1;
 	rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev_info.no_commit_id = 1;
-	DIFF_OPT_SET(&rev_info.diffopt, BINARY);
-	DIFF_OPT_SET(&rev_info.diffopt, FULL_INDEX);
+	rev_info.diffopt.flags.BINARY = 1;
+	rev_info.diffopt.flags.FULL_INDEX = 1;
 	rev_info.diffopt.use_color = 0;
 	rev_info.diffopt.file = fp;
 	rev_info.diffopt.close_file = 1;

builtin/blame.c +2 -2

index a29574984..76994aa64 100644
@@ -708,8 +708,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	git_config(git_blame_config, &output_option);
 	init_revisions(&revs, NULL);
 	revs.date_mode = blame_date_mode;
-	DIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);
-	DIFF_OPT_SET(&revs.diffopt, FOLLOW_RENAMES);
+	revs.diffopt.flags.ALLOW_TEXTCONV = 1;
+	revs.diffopt.flags.FOLLOW_RENAMES = 1;
 
 	save_commit_buffer = 0;
 	dashdash_pos = 0;

builtin/diff.c +3 -3

index 8cbaf4538..ed41eb5a5 100644
@@ -350,8 +350,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.stat_graph_width = -1;
 
 	/* Default to let external and textconv be used */
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+	rev.diffopt.flags.ALLOW_EXTERNAL = 1;
+	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
 
 	if (nongit)
 		die(_("Not a git repository"));

@@ -361,7 +361,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		diff_setup_done(&rev.diffopt);
 	}
 
-	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	rev.diffopt.flags.RECURSIVE = 1;
 
 	setup_diff_pager(&rev.diffopt);

builtin/fast-export.c +1 -1

index 2fb60d6d4..35c8fb65d 100644
@@ -1066,7 +1066,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
 	revs.diffopt.format_callback_data = &paths_of_changed_objects;
-	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
+	revs.diffopt.flags.RECURSIVE = 1;
 	while ((commit = get_revision(&revs))) {
 		if (has_unshown_parent(commit)) {
 			add_object_array(&commit->object, NULL, &commits);

builtin/log.c +7 -7

index 2b2d36678..4a24d3e48 100644
@@ -121,16 +121,16 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
 	if (default_follow)
-		DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
+		rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES = 1;
 	rev->verbose_header = 1;
-	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
+	rev->diffopt.flags.RECURSIVE = 1;
 	rev->diffopt.stat_width = -1; /* use full terminal width */
 	rev->diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
 	rev->abbrev_commit = default_abbrev_commit;
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
-	DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
+	rev->diffopt.flags.ALLOW_TEXTCONV = 1;
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);

@@ -668,7 +668,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
 {
 	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
 	    rev->prune_data.nr == 1)
-		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
+		rev->diffopt.flags.FOLLOW_RENAMES = 1;
 
 	/* Turn --cc/-c into -p --cc/-c when -p was not given */
 	if (!rev->diffopt.output_format && rev->combine_merges)

@@ -1340,7 +1340,7 @@ static void prepare_bases(struct base_tree_info *bases,
 		return;
 
 	diff_setup(&diffopt);
-	DIFF_OPT_SET(&diffopt, RECURSIVE);
+	diffopt.flags.RECURSIVE = 1;
 	diff_setup_done(&diffopt);
 
 	oidcpy(&bases->base_commit, &base->object.oid);

@@ -1511,7 +1511,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
-	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	rev.diffopt.flags.RECURSIVE = 1;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";

@@ -1613,7 +1613,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.zero_commit = zero_commit;
 
 	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
-		DIFF_OPT_SET(&rev.diffopt, BINARY);
+		rev.diffopt.flags.BINARY = 1;
 
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);

builtin/reset.c +1 -1

index ea2fad5a0..206819ef1 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
+	opt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

combine-diff.c +1 -1

index 40cb4dd1a..204b0dfce 100644
@@ -1413,7 +1413,7 @@ void diff_tree_combined(const struct object_id *oid,
 
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
-	DIFF_OPT_SET(&diffopts, RECURSIVE);
+	diffopts.flags.RECURSIVE = 1;
 	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
 
 	/* find set of paths that everybody touches

diff-lib.c +2 -2

index d6b711617..b1ec804fe 100644
@@ -545,8 +545,8 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	memset(&opt, 0, sizeof(opt));
 	opt.def = def;
 	setup_revisions(0, NULL, &rev, &opt);
-	DIFF_OPT_SET(&rev.diffopt, QUICK);
-	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
+	rev.diffopt.flags.QUICK = 1;
+	rev.diffopt.flags.EXIT_WITH_STATUS = 1;
 	if (flags)
 		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;

diff-no-index.c +3 -3

index 40d17e42c..a3e194340 100644
@@ -276,16 +276,16 @@ void diff_no_index(struct rev_info *revs,
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
+	revs->diffopt.flags.NO_INDEX = 1;
 
-	DIFF_OPT_SET(&revs->diffopt, RELATIVE_NAME);
+	revs->diffopt.flags.RELATIVE_NAME = 1;
 	revs->diffopt.prefix = prefix;
 
 	revs->max_count = -2;
 	diff_setup_done(&revs->diffopt);
 
 	setup_diff_pager(&revs->diffopt);
-	DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
+	revs->diffopt.flags.EXIT_WITH_STATUS = 1;
 
 	if (queue_diff(&revs->diffopt, paths[0], paths[1]))
 		exit(1);

diff.c +33 ~~-33~~

193 lines not on this page · raw message

diff.h +0 -1

index a3656ff5b..4c1af4be6 100644
@@ -110,7 +110,6 @@ static inline struct diff_flags diff_flags_or(const struct diff_flags *a,
 	return out;
 }
 
-#define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
 #define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
 
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)

merge-recursive.c +1 -1

index 1d3f8f0d2..e7b3df45c 100644
@@ -540,7 +540,7 @@ static struct string_list *get_renames(struct merge_options *o,
 		return renames;
 
 	diff_setup(&opts);
-	DIFF_OPT_SET(&opts, RECURSIVE);
+	opts.flags.RECURSIVE = 1;
 	DIFF_OPT_CLR(&opts, RENAME_EMPTY);
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :

notes-merge.c +2 -2

index 4352c34a6..b50111cf2 100644
@@ -125,7 +125,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	       oid_to_hex(base), oid_to_hex(remote));
 
 	diff_setup(&opt);
-	DIFF_OPT_SET(&opt, RECURSIVE);
+	opt.flags.RECURSIVE = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, remote, "", &opt);

@@ -188,7 +188,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	       len, oid_to_hex(base), oid_to_hex(local));
 
 	diff_setup(&opt);
-	DIFF_OPT_SET(&opt, RECURSIVE);
+	opt.flags.RECURSIVE = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, local, "", &opt);

patch-ids.c +1 -1

index 7a583b301..189869e57 100644
@@ -61,7 +61,7 @@ int init_patch_ids(struct patch_ids *ids)
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
 	ids->diffopts.detect_rename = 0;
-	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
+	ids->diffopts.flags.RECURSIVE = 1;
 	diff_setup_done(&ids->diffopts);
 	hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
 	return 0;

revision.c +8 -8

index c893d9166..f019dd277 100644
@@ -410,7 +410,7 @@ static void file_add_remove(struct diff_options *options,
 
 	tree_difference |= diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		DIFF_OPT_SET(options, HAS_CHANGES);
+		options->flags.HAS_CHANGES = 1;
 }
 
 static void file_change(struct diff_options *options,

@@ -422,7 +422,7 @@ static void file_change(struct diff_options *options,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	DIFF_OPT_SET(options, HAS_CHANGES);
+	options->flags.HAS_CHANGES = 1;
 }
 
 static int rev_compare_tree(struct rev_info *revs,

@@ -1403,8 +1403,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	DIFF_OPT_SET(&revs->pruning, RECURSIVE);
-	DIFF_OPT_SET(&revs->pruning, QUICK);
+	revs->pruning.flags.RECURSIVE = 1;
+	revs->pruning.flags.QUICK = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->sort_order = REV_SORT_IN_GRAPH_ORDER;

@@ -1917,11 +1917,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		die("--unpacked=<packfile> no longer supported.");
 	} else if (!strcmp(arg, "-r")) {
 		revs->diff = 1;
-		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+		revs->diffopt.flags.RECURSIVE = 1;
 	} else if (!strcmp(arg, "-t")) {
 		revs->diff = 1;
-		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
-		DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
+		revs->diffopt.flags.RECURSIVE = 1;
+		revs->diffopt.flags.TREE_IN_RECURSIVE = 1;
 	} else if (!strcmp(arg, "-m")) {
 		revs->ignore_merges = 0;
 	} else if (!strcmp(arg, "-c")) {

@@ -2066,7 +2066,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
 		revs->grep_filter.ignore_case = 1;
-		DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
+		revs->diffopt.flags.PICKAXE_IGNORE_CASE = 1;
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
 	} else if (!strcmp(arg, "--perl-regexp") || !strcmp(arg, "-P")) {

submodule.c +4 -4

index 58da9c668..7e7998592 100644
@@ -183,7 +183,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		if (ignore)
 			handle_ignore_submodules_arg(diffopt, ignore);
 		else if (is_gitmodules_unmerged(&the_index))
-			DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
+			diffopt->flags.IGNORE_SUBMODULES = 1;
 	}
 }

@@ -407,11 +407,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
 
 	if (!strcmp(arg, "all"))
-		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
+		diffopt->flags.IGNORE_SUBMODULES = 1;
 	else if (!strcmp(arg, "untracked"))
-		DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+		diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
 	else if (!strcmp(arg, "dirty"))
-		DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
+		diffopt->flags.IGNORE_DIRTY_SUBMODULES = 1;
 	else if (strcmp(arg, "none"))
 		die("bad --ignore-submodules argument: %s", arg);
 }

tree-diff.c +2 -2

index ed0aa6a62..b996a23bb 100644
@@ -608,8 +608,8 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
-	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
+	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.FIND_COPIES_HARDER = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->pathspec.items[0].match;
 	diff_opts.break_opt = opt->break_opt;

wt-status.c +9 -9

index 29bc64cc0..59f9f3a0b 100644
@@ -559,12 +559,12 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	rev.diffopt.flags.DIRTY_SUBMODULES = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (!s->show_untracked_files)
-		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+		rev.diffopt.flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
 	if (s->ignore_submodule_arg) {
-		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+		rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;

@@ -583,7 +583,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
-	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (s->ignore_submodule_arg) {
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);

@@ -949,7 +949,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	init_revisions(&rev, NULL);
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 
 	memset(&opt, 0, sizeof(opt));

@@ -2263,8 +2263,8 @@ int has_unstaged_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
+	rev_info.diffopt.flags.QUICK = 1;
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_files(&rev_info, 0);
 	return diff_result_code(&rev_info.diffopt, result);

@@ -2283,8 +2283,8 @@ int has_uncommitted_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
+	rev_info.diffopt.flags.QUICK = 1;
 	add_head_to_pending(&rev_info);
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_index(&rev_info, 1);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-30 22:20 UTC #30 lore raw

[PATCH v2 8/4] diff: make struct diff_flags members lowercase

In reply to Brandon Williams

Now that the flags stored in struct diff_flags are being accessed directly and not through macros, change all struct members from being uppercase to lowercase.
This conversion is done using the following semantic patch:

	@@
	expression E;
	@@
	- E.RECURSIVE
	+ E.recursive

	@@
	expression E;
	@@
	- E.TREE_IN_RECURSIVE
	+ E.tree_in_recursive

	@@
	expression E;
	@@
	- E.BINARY
	+ E.binary

	@@
	expression E;
	@@
	- E.TEXT
	+ E.text

	@@
	expression E;
	@@
	- E.FULL_INDEX
	+ E.full_index

	@@
	expression E;
	@@
	- E.SILENT_ON_REMOVE
	+ E.silent_on_remove

	@@
	expression E;
	@@
	- E.FIND_COPIES_HARDER
	+ E.find_copies_harder

	@@
	expression E;
	@@
	- E.FOLLOW_RENAMES
	+ E.follow_renames

	@@
	expression E;
	@@
	- E.RENAME_EMPTY
	+ E.rename_empty

	@@
	expression E;
	@@
	- E.HAS_CHANGES
	+ E.has_changes

	@@
	expression E;
	@@
	- E.QUICK
	+ E.quick

	@@
	expression E;
	@@
	- E.NO_INDEX
	+ E.no_index

	@@
	expression E;
	@@
	- E.ALLOW_EXTERNAL
	+ E.allow_external

	@@
	expression E;
	@@
	- E.EXIT_WITH_STATUS
	+ E.exit_with_status

	@@
	expression E;
	@@
	- E.REVERSE_DIFF
	+ E.reverse_diff

	@@
	expression E;
	@@
	- E.CHECK_FAILED
	+ E.check_failed

	@@
	expression E;
	@@
	- E.RELATIVE_NAME
	+ E.relative_name

	@@
	expression E;
	@@
	- E.IGNORE_SUBMODULES
	+ E.ignore_submodules

	@@
	expression E;
	@@
	- E.DIRSTAT_CUMULATIVE
	+ E.dirstat_cumulative

	@@
	expression E;
	@@
	- E.DIRSTAT_BY_FILE
	+ E.dirstat_by_file

	@@
	expression E;
	@@
	- E.ALLOW_TEXTCONV
	+ E.allow_textconv

	@@
	expression E;
	@@
	- E.TEXTCONV_SET_VIA_CMDLINE
	+ E.textconv_set_via_cmdline

	@@
	expression E;
	@@
	- E.DIFF_FROM_CONTENTS
	+ E.diff_from_contents

	@@
	expression E;
	@@
	- E.DIRTY_SUBMODULES
	+ E.dirty_submodules

	@@
	expression E;
	@@
	- E.IGNORE_UNTRACKED_IN_SUBMODULES
	+ E.ignore_untracked_in_submodules

	@@
	expression E;
	@@
	- E.IGNORE_DIRTY_SUBMODULES
	+ E.ignore_dirty_submodules

	@@
	expression E;
	@@
	- E.OVERRIDE_SUBMODULE_CONFIG
	+ E.override_submodule_config

	@@
	expression E;
	@@
	- E.DIRSTAT_BY_LINE
	+ E.dirstat_by_line

	@@
	expression E;
	@@
	- E.FUNCCONTEXT
	+ E.funccontext

	@@
	expression E;
	@@
	- E.PICKAXE_IGNORE_CASE
	+ E.pickaxe_ignore_case

	@@
	expression E;
	@@
	- E.DEFAULT_FOLLOW_RENAMES
	+ E.default_follow_renames

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c               |  16 ++---
 builtin/add.c         |   4 +-
 builtin/am.c          |  10 +--
 builtin/blame.c       |  10 +--
 builtin/commit.c      |   4 +-
 builtin/diff.c        |   8 +--
 builtin/fast-export.c |   2 +-
 builtin/log.c         |  26 ++++----
 builtin/reset.c       |   2 +-
 builtin/rev-list.c    |   2 +-
 combine-diff.c        |  10 +--
 diff-lib.c            |  22 +++----
 diff-no-index.c       |   8 +--
 diff.c                | 172 +++++++++++++++++++++++++-------------------------
 diff.h                |  62 +++++++++---------
 diffcore-pickaxe.c    |   8 +--
 diffcore-rename.c     |   6 +-
 log-tree.c            |   2 +-
 merge-recursive.c     |   4 +-
 notes-merge.c         |   4 +-
 patch-ids.c           |   2 +-
 revision.c            |  24 +++----
 submodule.c           |  16 ++---
 tree-diff.c           |  16 ++---
 wt-status.c           |  18 +++---
 25 files changed, 229 insertions(+), 229 deletions(-)

blame.c +8 -8

index dc9cc237b..28e03726f 100644
@@ -209,7 +209,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
-			if (opt->flags.ALLOW_TEXTCONV &&
+			if (opt->flags.allow_textconv &&
 			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
 				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
 			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)

@@ -293,7 +293,7 @@ static void fill_origin_blob(struct diff_options *opt,
 		unsigned long file_size;
 
 		(*num_read_blob)++;
-		if (opt->flags.ALLOW_TEXTCONV &&
+		if (opt->flags.allow_textconv &&
 		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
 			;
 		else

@@ -541,7 +541,7 @@ static struct blame_origin *find_origin(struct commit *parent,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;

@@ -615,7 +615,7 @@ static struct blame_origin *find_rename(struct commit *parent,
 	int i;
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;

@@ -1238,7 +1238,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		return; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	diff_setup_done(&diff_opts);

@@ -1253,7 +1253,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		diff_opts.flags.FIND_COPIES_HARDER = 1;
+		diff_opts.flags.find_copies_harder = 1;
 
 	if (is_null_oid(&target->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);

@@ -1262,7 +1262,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 			      &target->commit->tree->object.oid,
 			      "", &diff_opts);
 
-	if (!diff_opts.flags.FIND_COPIES_HARDER)
+	if (!diff_opts.flags.find_copies_harder)
 		diffcore_std(&diff_opts);
 
 	do {

@@ -1825,7 +1825,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 		if (fill_blob_sha1_and_mode(o))
 			die(_("no such path %s in %s"), path, final_commit_name);
 
-		if (sb->revs->diffopt.flags.ALLOW_TEXTCONV &&
+		if (sb->revs->diffopt.flags.allow_textconv &&
 		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
 				    &sb->final_buf_size))
 			;

builtin/add.c +2 -2

index e1d83b69a..8d08e99e9 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	rev.diffopt.flags.override_submodule_config = 1;
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

@@ -218,7 +218,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev.diffopt.use_color = 0;
-	rev.diffopt.flags.IGNORE_DIRTY_SUBMODULES = 1;
+	rev.diffopt.flags.ignore_dirty_submodules = 1;
 	out = open(file, O_CREAT | O_WRONLY, 0666);
 	if (out < 0)
 		die(_("Could not open '%s' for writing."), file);

builtin/am.c +5 -5

index 015425a0f..b281d58f3 100644
@@ -1157,9 +1157,9 @@ static int index_has_changes(struct strbuf *sb)
 		struct diff_options opt;
 
 		diff_setup(&opt);
-		opt.flags.EXIT_WITH_STATUS = 1;
+		opt.flags.exit_with_status = 1;
 		if (!sb)
-			opt.flags.QUICK = 1;
+			opt.flags.quick = 1;
 		do_diff_cache(&head, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {

@@ -1168,7 +1168,7 @@ static int index_has_changes(struct strbuf *sb)
 			strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
 		}
 		diff_flush(&opt);
-		return opt.flags.HAS_CHANGES != 0;
+		return opt.flags.has_changes != 0;
 	} else {
 		for (i = 0; sb && i < active_nr; i++) {
 			if (i)

@@ -1409,8 +1409,8 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
 	rev_info.show_root_diff = 1;
 	rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev_info.no_commit_id = 1;
-	rev_info.diffopt.flags.BINARY = 1;
-	rev_info.diffopt.flags.FULL_INDEX = 1;
+	rev_info.diffopt.flags.binary = 1;
+	rev_info.diffopt.flags.full_index = 1;
 	rev_info.diffopt.use_color = 0;
 	rev_info.diffopt.file = fp;
 	rev_info.diffopt.close_file = 1;

builtin/blame.c +5 -5

index 79db9e849..005f55aaa 100644
@@ -708,8 +708,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	git_config(git_blame_config, &output_option);
 	init_revisions(&revs, NULL);
 	revs.date_mode = blame_date_mode;
-	revs.diffopt.flags.ALLOW_TEXTCONV = 1;
-	revs.diffopt.flags.FOLLOW_RENAMES = 1;
+	revs.diffopt.flags.allow_textconv = 1;
+	revs.diffopt.flags.follow_renames = 1;
 
 	save_commit_buffer = 0;
 	dashdash_pos = 0;

@@ -734,9 +734,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
-	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
+	no_whole_file_rename = !revs.diffopt.flags.follow_renames;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
-	revs.diffopt.flags.FOLLOW_RENAMES = 0;
+	revs.diffopt.flags.follow_renames = 0;
 	argc = parse_options_end(&ctx);
 
 	if (incremental || (output_option & OUTPUT_PORCELAIN)) {

@@ -803,7 +803,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 	blame_date_width -= 1; /* strip the null */
 
-	if (revs.diffopt.flags.FIND_COPIES_HARDER)
+	if (revs.diffopt.flags.find_copies_harder)
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);

builtin/commit.c +2 -2

index 960e7ac08..0f368ad81 100644
@@ -913,10 +913,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * be really confusing.
 			 */
 			struct diff_flags flags = DIFF_FLAGS_INIT;
-			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+			flags.override_submodule_config = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				flags.IGNORE_SUBMODULES = 1;
+				flags.ignore_submodules = 1;
 			commitable = index_differs_from(parent, &flags, 1);
 		}
 	}

builtin/diff.c +4 -4

index ed41eb5a5..3476e95e1 100644
@@ -44,7 +44,7 @@ static void stuff_change(struct diff_options *opt,
 	    !oidcmp(old_oid, new_oid) && (old_mode == new_mode))
 		return;
 
-	if (opt->flags.REVERSE_DIFF) {
+	if (opt->flags.reverse_diff) {
 		SWAP(old_mode, new_mode);
 		SWAP(old_oid, new_oid);
 		SWAP(old_path, new_path);

@@ -350,8 +350,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.stat_graph_width = -1;
 
 	/* Default to let external and textconv be used */
-	rev.diffopt.flags.ALLOW_EXTERNAL = 1;
-	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev.diffopt.flags.allow_external = 1;
+	rev.diffopt.flags.allow_textconv = 1;
 
 	if (nongit)
 		die(_("Not a git repository"));

@@ -361,7 +361,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		diff_setup_done(&rev.diffopt);
 	}
 
-	rev.diffopt.flags.RECURSIVE = 1;
+	rev.diffopt.flags.recursive = 1;
 
 	setup_diff_pager(&rev.diffopt);

builtin/fast-export.c +1 -1

index 35c8fb65d..72672665b 100644
@@ -1066,7 +1066,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
 	revs.diffopt.format_callback_data = &paths_of_changed_objects;
-	revs.diffopt.flags.RECURSIVE = 1;
+	revs.diffopt.flags.recursive = 1;
 	while ((commit = get_revision(&revs))) {
 		if (has_unshown_parent(commit)) {
 			add_object_array(&commit->object, NULL, &commits);

builtin/log.c +13 ~~-13~~

index 4a24d3e48..f3d5a02be 100644
@@ -121,16 +121,16 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
 	if (default_follow)
-		rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES = 1;
+		rev->diffopt.flags.default_follow_renames = 1;
 	rev->verbose_header = 1;
-	rev->diffopt.flags.RECURSIVE = 1;
+	rev->diffopt.flags.recursive = 1;
 	rev->diffopt.stat_width = -1; /* use full terminal width */
 	rev->diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
 	rev->abbrev_commit = default_abbrev_commit;
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
-	rev->diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev->diffopt.flags.allow_textconv = 1;
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);

@@ -181,7 +181,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		init_display_notes(&rev->notes_opt);
 
 	if (rev->diffopt.pickaxe || rev->diffopt.filter ||
-	    rev->diffopt.flags.FOLLOW_RENAMES)
+	    rev->diffopt.flags.follow_renames)
 		rev->always_show_header = 0;
 
 	if (source)

@@ -391,7 +391,7 @@ static int cmd_log_walk(struct rev_info *rev)
 		fclose(rev->diffopt.file);
 
 	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
-	    rev->diffopt.flags.CHECK_FAILED) {
+	    rev->diffopt.flags.check_failed) {
 		return 02;
 	}
 	return diff_result_code(&rev->diffopt, 0);

@@ -483,8 +483,8 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!rev->diffopt.flags.TEXTCONV_SET_VIA_CMDLINE ||
-	    !rev->diffopt.flags.ALLOW_TEXTCONV)
+	if (!rev->diffopt.flags.textconv_set_via_cmdline ||
+	    !rev->diffopt.flags.allow_textconv)
 		return stream_blob_to_fd(1, oid, NULL, 0);
 
 	if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,

@@ -666,9 +666,9 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 static void log_setup_revisions_tweak(struct rev_info *rev,
 				      struct setup_revision_opt *opt)
 {
-	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
+	if (rev->diffopt.flags.default_follow_renames &&
 	    rev->prune_data.nr == 1)
-		rev->diffopt.flags.FOLLOW_RENAMES = 1;
+		rev->diffopt.flags.follow_renames = 1;
 
 	/* Turn --cc/-c into -p --cc/-c when -p was not given */
 	if (!rev->diffopt.output_format && rev->combine_merges)

@@ -1340,7 +1340,7 @@ static void prepare_bases(struct base_tree_info *bases,
 		return;
 
 	diff_setup(&diffopt);
-	diffopt.flags.RECURSIVE = 1;
+	diffopt.flags.recursive = 1;
 	diff_setup_done(&diffopt);
 
 	oidcpy(&bases->base_commit, &base->object.oid);

@@ -1511,7 +1511,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
-	rev.diffopt.flags.RECURSIVE = 1;
+	rev.diffopt.flags.recursive = 1;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";

@@ -1612,8 +1612,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.zero_commit = zero_commit;
 
-	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
-		rev.diffopt.flags.BINARY = 1;
+	if (!rev.diffopt.flags.text && !no_binary_diff)
+		rev.diffopt.flags.binary = 1;
 
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);

builtin/reset.c +1 -1

index 206819ef1..4b313a018 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	opt.flags.override_submodule_config = 1;
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

builtin/rev-list.c +1 -1

index c62382171..1d3b6b61b 100644
@@ -294,7 +294,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (revs.bisect)
 		bisect_list = 1;
 
-	if (revs.diffopt.flags.QUICK)
+	if (revs.diffopt.flags.quick)
 		info.flags |= REV_LIST_QUIET;
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];

combine-diff.c +5 -5

index 5a3a8b49b..23f3d25e2 100644
@@ -898,7 +898,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 				 int show_file_header)
 {
 	struct diff_options *opt = &rev->diffopt;
-	int abbrev = opt->flags.FULL_INDEX ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
+	int abbrev = opt->flags.full_index ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
 	const char *a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
 	const char *b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
 	const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);

@@ -987,7 +987,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	userdiff = userdiff_find_by_path(elem->path);
 	if (!userdiff)
 		userdiff = userdiff_find_by_name("default");
-	if (opt->flags.ALLOW_TEXTCONV)
+	if (opt->flags.allow_textconv)
 		textconv = userdiff_get_textconv(userdiff);
 
 	/* Read the result of merge first */

@@ -1413,8 +1413,8 @@ void diff_tree_combined(const struct object_id *oid,
 
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
-	diffopts.flags.RECURSIVE = 1;
-	diffopts.flags.ALLOW_EXTERNAL = 0;
+	diffopts.flags.recursive = 1;
+	diffopts.flags.allow_external = 0;
 
 	/* find set of paths that everybody touches
 	 *

@@ -1435,7 +1435,7 @@ void diff_tree_combined(const struct object_id *oid,
 	 * NOTE please keep this semantically in sync with diffcore_std()
 	 */
 	need_generic_pathscan = opt->skip_stat_unmatch	||
-			opt->flags.FOLLOW_RENAMES	||
+			opt->flags.follow_renames	||
 			opt->break_opt != -1	||
 			opt->detect_rename	||
 			opt->pickaxe		||

diff-lib.c +11 ~~-11~~

index b1ec804fe..9d04972b9 100644
@@ -72,14 +72,14 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct diff_flags orig_flags = diffopt->flags;
-		if (!diffopt->flags.OVERRIDE_SUBMODULE_CONFIG)
+		if (!diffopt->flags.override_submodule_config)
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
-		if (diffopt->flags.IGNORE_SUBMODULES)
+		if (diffopt->flags.ignore_submodules)
 			changed = 0;
-		else if (!diffopt->flags.IGNORE_DIRTY_SUBMODULES &&
-			 (!changed || diffopt->flags.DIRTY_SUBMODULES))
+		else if (!diffopt->flags.ignore_dirty_submodules &&
+			 (!changed || diffopt->flags.dirty_submodules))
 			*dirty_submodule = is_submodule_modified(ce->name,
-								 diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES);
+								 diffopt->flags.ignore_untracked_in_submodules);
 		diffopt->flags = orig_flags;
 	}
 	return changed;

@@ -229,7 +229,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 
 		if (!changed && !dirty_submodule) {
 			ce_mark_uptodate(ce);
-			if (!revs->diffopt.flags.FIND_COPIES_HARDER)
+			if (!revs->diffopt.flags.find_copies_harder)
 				continue;
 		}
 		oldmode = ce->ce_mode;

@@ -363,7 +363,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !oidcmp(oid, &old->oid) && !dirty_submodule &&
-	    !revs->diffopt.flags.FIND_COPIES_HARDER)
+	    !revs->diffopt.flags.find_copies_harder)
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,

@@ -494,7 +494,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.head_idx = 1;
 	opts.index_only = cached;
 	opts.diff_index_cached = (cached &&
-				  !revs->diffopt.flags.FIND_COPIES_HARDER);
+				  !revs->diffopt.flags.find_copies_harder);
 	opts.merge = 1;
 	opts.fn = oneway_diff;
 	opts.unpack_data = revs;

@@ -545,12 +545,12 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	memset(&opt, 0, sizeof(opt));
 	opt.def = def;
 	setup_revisions(0, NULL, &rev, &opt);
-	rev.diffopt.flags.QUICK = 1;
-	rev.diffopt.flags.EXIT_WITH_STATUS = 1;
+	rev.diffopt.flags.quick = 1;
+	rev.diffopt.flags.exit_with_status = 1;
 	if (flags)
 		rev.diffopt.flags = diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);
-	return (rev.diffopt.flags.HAS_CHANGES != 0);
+	return (rev.diffopt.flags.has_changes != 0);
 }

diff-no-index.c +4 -4

index a3e194340..0ed5f0f49 100644
@@ -184,7 +184,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (o->flags.REVERSE_DIFF) {
+		if (o->flags.reverse_diff) {
 			SWAP(mode1, mode2);
 			SWAP(name1, name2);
 		}

@@ -276,16 +276,16 @@ void diff_no_index(struct rev_info *revs,
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	revs->diffopt.flags.NO_INDEX = 1;
+	revs->diffopt.flags.no_index = 1;
 
-	revs->diffopt.flags.RELATIVE_NAME = 1;
+	revs->diffopt.flags.relative_name = 1;
 	revs->diffopt.prefix = prefix;
 
 	revs->max_count = -2;
 	diff_setup_done(&revs->diffopt);
 
 	setup_diff_pager(&revs->diffopt);
-	revs->diffopt.flags.EXIT_WITH_STATUS = 1;
+	revs->diffopt.flags.exit_with_status = 1;
 
 	if (queue_diff(&revs->diffopt, paths[0], paths[1]))
 		exit(1);

diff.c +86 ~~-86~~

450 lines not on this page · raw message

diff.h +31 ~~-31~~

68 lines not on this page · raw message

diffcore-pickaxe.c +4 -4

index 6193c07f6..9476bd210 100644
@@ -131,7 +131,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	if (!DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two))
 		return 0;
 
-	if (o->flags.ALLOW_TEXTCONV) {
+	if (o->flags.allow_textconv) {
 		textconv_one = get_textconv(p->one);
 		textconv_two = get_textconv(p->two);
 	}

@@ -222,11 +222,11 @@ void diffcore_pickaxe(struct diff_options *o)
 
 	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
 		int cflags = REG_EXTENDED | REG_NEWLINE;
-		if (o->flags.PICKAXE_IGNORE_CASE)
+		if (o->flags.pickaxe_ignore_case)
 			cflags |= REG_ICASE;
 		regcomp_or_die(&regex, needle, cflags);
 		regexp = &regex;
-	} else if (o->flags.PICKAXE_IGNORE_CASE &&
+	} else if (o->flags.pickaxe_ignore_case &&
 		   has_non_ascii(needle)) {
 		struct strbuf sb = STRBUF_INIT;
 		int cflags = REG_NEWLINE | REG_ICASE;

@@ -236,7 +236,7 @@ void diffcore_pickaxe(struct diff_options *o)
 		strbuf_release(&sb);
 		regexp = &regex;
 	} else {
-		kws = kwsalloc(o->flags.PICKAXE_IGNORE_CASE
+		kws = kwsalloc(o->flags.pickaxe_ignore_case
 			       ? tolower_trans_tbl : NULL);
 		kwsincr(kws, needle, strlen(needle));
 		kwsprep(kws);

diffcore-rename.c +3 -3

index bd077ee11..12dc2a056 100644
@@ -405,7 +405,7 @@ static int too_many_rename_candidates(int num_create,
 		num_src > num_create ? num_src : num_create;
 
 	/* Are we running under -C -C? */
-	if (!options->flags.FIND_COPIES_HARDER)
+	if (!options->flags.find_copies_harder)
 		return 1;
 
 	/* Would we bust the limit if we were running under -C? */

@@ -463,7 +463,7 @@ void diffcore_rename(struct diff_options *options)
 			else if (options->single_follow &&
 				 strcmp(options->single_follow, p->two->path))
 				continue; /* not interested */
-			else if (!options->flags.RENAME_EMPTY &&
+			else if (!options->flags.rename_empty &&
 				 is_empty_blob_oid(&p->two->oid))
 				continue;
 			else if (add_rename_dst(p->two) < 0) {

@@ -473,7 +473,7 @@ void diffcore_rename(struct diff_options *options)
 				goto cleanup;
 			}
 		}
-		else if (!options->flags.RENAME_EMPTY &&
+		else if (!options->flags.rename_empty &&
 			 is_empty_blob_oid(&p->one->oid))
 			continue;
 		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {

log-tree.c +1 -1

index 460bb5498..8dacccc0c 100644
@@ -793,7 +793,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	struct commit_list *parents;
 	struct object_id *oid;
 
-	if (!opt->diff && !opt->diffopt.flags.EXIT_WITH_STATUS)
+	if (!opt->diff && !opt->diffopt.flags.exit_with_status)
 		return 0;
 
 	parse_commit_or_die(commit);

merge-recursive.c +2 -2

index 9752aba4e..f6c03770b 100644
@@ -540,8 +540,8 @@ static struct string_list *get_renames(struct merge_options *o,
 		return renames;
 
 	diff_setup(&opts);
-	opts.flags.RECURSIVE = 1;
-	opts.flags.RENAME_EMPTY = 0;
+	opts.flags.recursive = 1;
+	opts.flags.rename_empty = 0;
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
 			    o->diff_rename_limit >= 0 ? o->diff_rename_limit :

notes-merge.c +2 -2

index b50111cf2..b3d4d44fc 100644
@@ -125,7 +125,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	       oid_to_hex(base), oid_to_hex(remote));
 
 	diff_setup(&opt);
-	opt.flags.RECURSIVE = 1;
+	opt.flags.recursive = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, remote, "", &opt);

@@ -188,7 +188,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	       len, oid_to_hex(base), oid_to_hex(local));
 
 	diff_setup(&opt);
-	opt.flags.RECURSIVE = 1;
+	opt.flags.recursive = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, local, "", &opt);

patch-ids.c +1 -1

index 189869e57..8f7c25d5d 100644
@@ -61,7 +61,7 @@ int init_patch_ids(struct patch_ids *ids)
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
 	ids->diffopts.detect_rename = 0;
-	ids->diffopts.flags.RECURSIVE = 1;
+	ids->diffopts.flags.recursive = 1;
 	diff_setup_done(&ids->diffopts);
 	hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
 	return 0;

revision.c +12 ~~-12~~

index 6bb873501..bfde5b0cd 100644
@@ -410,7 +410,7 @@ static void file_add_remove(struct diff_options *options,
 
 	tree_difference |= diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		options->flags.HAS_CHANGES = 1;
+		options->flags.has_changes = 1;
 }
 
 static void file_change(struct diff_options *options,

@@ -422,7 +422,7 @@ static void file_change(struct diff_options *options,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	options->flags.HAS_CHANGES = 1;
+	options->flags.has_changes = 1;
 }
 
 static int rev_compare_tree(struct rev_info *revs,

@@ -455,7 +455,7 @@ static int rev_compare_tree(struct rev_info *revs,
 	}
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.flags.HAS_CHANGES = 0;
+	revs->pruning.flags.has_changes = 0;
 	if (diff_tree_oid(&t1->object.oid, &t2->object.oid, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;

@@ -471,7 +471,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 		return 0;
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.flags.HAS_CHANGES = 0;
+	revs->pruning.flags.has_changes = 0;
 	retval = diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
 
 	return retval >= 0 && (tree_difference == REV_TREE_SAME);

@@ -1403,8 +1403,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	revs->pruning.flags.RECURSIVE = 1;
-	revs->pruning.flags.QUICK = 1;
+	revs->pruning.flags.recursive = 1;
+	revs->pruning.flags.quick = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->sort_order = REV_SORT_IN_GRAPH_ORDER;

@@ -1917,11 +1917,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		die("--unpacked=<packfile> no longer supported.");
 	} else if (!strcmp(arg, "-r")) {
 		revs->diff = 1;
-		revs->diffopt.flags.RECURSIVE = 1;
+		revs->diffopt.flags.recursive = 1;
 	} else if (!strcmp(arg, "-t")) {
 		revs->diff = 1;
-		revs->diffopt.flags.RECURSIVE = 1;
-		revs->diffopt.flags.TREE_IN_RECURSIVE = 1;
+		revs->diffopt.flags.recursive = 1;
+		revs->diffopt.flags.tree_in_recursive = 1;
 	} else if (!strcmp(arg, "-m")) {
 		revs->ignore_merges = 0;
 	} else if (!strcmp(arg, "-c")) {

@@ -2066,7 +2066,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
 		revs->grep_filter.ignore_case = 1;
-		revs->diffopt.flags.PICKAXE_IGNORE_CASE = 1;
+		revs->diffopt.flags.pickaxe_ignore_case = 1;
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
 	} else if (!strcmp(arg, "--perl-regexp") || !strcmp(arg, "-P")) {

@@ -2399,7 +2399,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	/* Pickaxe, diff-filter and rename following need diffs */
 	if (revs->diffopt.pickaxe ||
 	    revs->diffopt.filter ||
-	    revs->diffopt.flags.FOLLOW_RENAMES)
+	    revs->diffopt.flags.follow_renames)
 		revs->diff = 1;
 
 	if (revs->topo_order)

@@ -2408,7 +2408,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->prune_data.nr) {
 		copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!revs->diffopt.flags.FOLLOW_RENAMES)
+		if (!revs->diffopt.flags.follow_renames)
 			revs->prune = 1;
 		if (!revs->full_diff)
 			copy_pathspec(&revs->diffopt.pathspec,

submodule.c +8 -8

index 62a93bb88..9b16adc96 100644
@@ -183,7 +183,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		if (ignore)
 			handle_ignore_submodules_arg(diffopt, ignore);
 		else if (is_gitmodules_unmerged(&the_index))
-			diffopt->flags.IGNORE_SUBMODULES = 1;
+			diffopt->flags.ignore_submodules = 1;
 	}
 }

@@ -402,16 +402,16 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
-	diffopt->flags.IGNORE_SUBMODULES = 0;
-	diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 0;
-	diffopt->flags.IGNORE_DIRTY_SUBMODULES = 0;
+	diffopt->flags.ignore_submodules = 0;
+	diffopt->flags.ignore_untracked_in_submodules = 0;
+	diffopt->flags.ignore_dirty_submodules = 0;
 
 	if (!strcmp(arg, "all"))
-		diffopt->flags.IGNORE_SUBMODULES = 1;
+		diffopt->flags.ignore_submodules = 1;
 	else if (!strcmp(arg, "untracked"))
-		diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
+		diffopt->flags.ignore_untracked_in_submodules = 1;
 	else if (!strcmp(arg, "dirty"))
-		diffopt->flags.IGNORE_DIRTY_SUBMODULES = 1;
+		diffopt->flags.ignore_dirty_submodules = 1;
 	else if (strcmp(arg, "none"))
 		die("bad --ignore-submodules argument: %s", arg);
 }

@@ -616,7 +616,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 	argv_array_pushf(&cp.args, "--color=%s", want_color(o->use_color) ?
 			 "always" : "never");
 
-	if (o->flags.REVERSE_DIFF) {
+	if (o->flags.reverse_diff) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
 				 o->b_prefix, path);
 		argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",

tree-diff.c +8 -8

index b996a23bb..fe2e466ac 100644
@@ -212,9 +212,9 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 		mode = 0;
 	}
 
-	if (opt->flags.RECURSIVE && isdir) {
+	if (opt->flags.recursive && isdir) {
 		recurse = 1;
-		emitthis = opt->flags.TREE_IN_RECURSIVE;
+		emitthis = opt->flags.tree_in_recursive;
 	}
 
 	if (emitthis) {

@@ -425,7 +425,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	ttree = fill_tree_descriptor(&t, oid);
 
 	/* Enable recursion indefinitely */
-	opt->pathspec.recursive = opt->flags.RECURSIVE;
+	opt->pathspec.recursive = opt->flags.recursive;
 
 	for (;;) {
 		int imin, cmp;

@@ -484,7 +484,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t = p[imin] */
 		if (cmp == 0) {
 			/* are either pi > p[imin] or diff(t,pi) != ø ? */
-			if (!opt->flags.FIND_COPIES_HARDER) {
+			if (!opt->flags.find_copies_harder) {
 				for (i = 0; i < nparent; ++i) {
 					/* p[i] > p[imin] */
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)

@@ -522,7 +522,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t > p[imin] */
 		else {
 			/* ∀i pi=p[imin] -> D += "-p[imin]" */
-			if (!opt->flags.FIND_COPIES_HARDER) {
+			if (!opt->flags.find_copies_harder) {
 				for (i = 0; i < nparent; ++i)
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)
 						goto skip_emit_tp;

@@ -608,8 +608,8 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
-	diff_opts.flags.FIND_COPIES_HARDER = 1;
+	diff_opts.flags.recursive = 1;
+	diff_opts.flags.find_copies_harder = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->pathspec.items[0].match;
 	diff_opts.break_opt = opt->break_opt;

@@ -706,7 +706,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 	strbuf_addstr(&base, base_str);
 
 	retval = ll_diff_tree_oid(old_oid, new_oid, &base, opt);
-	if (!*base_str && opt->flags.FOLLOW_RENAMES && diff_might_be_rename())
+	if (!*base_str && opt->flags.follow_renames && diff_might_be_rename())
 		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
 	strbuf_release(&base);

wt-status.c +9 -9

index 59f9f3a0b..4f76e19d3 100644
@@ -559,12 +559,12 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	rev.diffopt.flags.DIRTY_SUBMODULES = 1;
+	rev.diffopt.flags.dirty_submodules = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (!s->show_untracked_files)
-		rev.diffopt.flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
+		rev.diffopt.flags.ignore_untracked_in_submodules = 1;
 	if (s->ignore_submodule_arg) {
-		rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+		rev.diffopt.flags.override_submodule_config = 1;
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;

@@ -583,7 +583,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
-	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	rev.diffopt.flags.override_submodule_config = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (s->ignore_submodule_arg) {
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);

@@ -949,7 +949,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	init_revisions(&rev, NULL);
-	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev.diffopt.flags.allow_textconv = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 
 	memset(&opt, 0, sizeof(opt));

@@ -2263,8 +2263,8 @@ int has_unstaged_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
-	rev_info.diffopt.flags.QUICK = 1;
+		rev_info.diffopt.flags.ignore_submodules = 1;
+	rev_info.diffopt.flags.quick = 1;
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_files(&rev_info, 0);
 	return diff_result_code(&rev_info.diffopt, result);

@@ -2283,8 +2283,8 @@ int has_uncommitted_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
-	rev_info.diffopt.flags.QUICK = 1;
+		rev_info.diffopt.flags.ignore_submodules = 1;
+	rev_info.diffopt.flags.quick = 1;
 	add_head_to_pending(&rev_info);
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_index(&rev_info, 1);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #31 lore raw

v3 cover convert diff flags to be stored in a struct

In reply to Brandon Williams

Changes in v3:
* Now always pass struct diff_flags by reference and don't return the struct but rather modify the passed in struct.
* Don't clear TEXTCONV_SET_VIA_CMDLINE when --no-textconv is passed
* added additional patches (set out separately before) to remove the macros and change the struct members to lowercase

Brandon Williams (8):
add, reset: use DIFF_OPT_SET macro to set a diff flag
diff: convert flags to be stored in bitfields
diff: add flag to indicate textconv was set via cmdline
diff: remove touched flags
diff: remove DIFF_OPT_TST macro
diff: remove DIFF_OPT_SET macro
diff: remove DIFF_OPT_CLR macro
diff: make struct diff_flags members lowercase

 blame.c               |  16 ++---
 builtin/add.c         |   4 +-
 builtin/am.c          |  10 +--
 builtin/blame.c       |  10 +--
 builtin/commit.c      |   7 +-
 builtin/diff.c        |   8 +--
 builtin/fast-export.c |   2 +-
 builtin/log.c         |  27 ++++----
 builtin/reset.c       |   2 +-
 builtin/rev-list.c    |   2 +-
 combine-diff.c        |  10 +--
 diff-lib.c            |  30 +++++----
 diff-no-index.c       |   8 +--
 diff.c                | 175 +++++++++++++++++++++++++-------------------------
 diff.h                |  88 ++++++++++++++-----------
 diffcore-pickaxe.c    |   8 +--
 diffcore-rename.c     |   6 +-
 log-tree.c            |   2 +-
 merge-recursive.c     |   4 +-
 notes-merge.c         |   4 +-
 patch-ids.c           |   2 +-
 revision.c            |  24 +++----
 sequencer.c           |   5 +-
 submodule.c           |  16 ++---
 tree-diff.c           |  16 ++---
 wt-status.c           |  18 +++---
 26 files changed, 259 insertions(+), 245 deletions(-)

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #32 lore raw

PATCH v3 1/8 add, reset: use DIFF_OPT_SET macro to set a diff flag

Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG' flag, use the 'DIFF_OPT_SET' macro.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/add.c   | 2 +-
 builtin/reset.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

builtin/add.c +1 -1

index a648cf4c5..b70e8a779 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	rev.diffopt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

builtin/reset.c +1 -1

index 9cd89b230..ea2fad5a0 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #33 lore raw

PATCH v3 2/8 diff: convert flags to be stored in bitfields

In reply to Brandon Williams

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/commit.c |  7 +++--
 builtin/log.c    |  3 +-
 diff-lib.c       |  7 +++--
 diff.c           |  2 +-
 diff.h           | 93 ++++++++++++++++++++++++++++++++------------------------
 sequencer.c      |  5 +--
 6 files changed, 68 insertions(+), 49 deletions(-)

builtin/commit.c +4 -3

index d75b3805e..960e7ac08 100644
@@ -912,11 +912,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * submodules which were manually staged, which would
 			 * be really confusing.
 			 */
-			int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+			struct diff_flags flags = DIFF_FLAGS_INIT;
+			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
-			commitable = index_differs_from(parent, diff_flags, 1);
+				flags.IGNORE_SUBMODULES = 1;
+			commitable = index_differs_from(parent, &flags, 1);
 		}
 	}
 	strbuf_release(&committer_ident);

builtin/log.c +2 -1

index d81a09051..dc28d43eb 100644
@@ -134,7 +134,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-	rev->diffopt.touched_flags = 0;
+
+	memset(&rev->diffopt.touched_flags, 0, sizeof(struct diff_flags));
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

diff-lib.c +4 -3

index 4e0980caa..ed37f24c6 100644
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 {
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
-		unsigned orig_flags = diffopt->flags;
+		struct diff_flags orig_flags = diffopt->flags;
 		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
 		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))

@@ -534,7 +534,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
 	return 0;
 }
 
-int index_differs_from(const char *def, int diff_flags,
+int index_differs_from(const char *def, const struct diff_flags *flags,
 		       int ita_invisible_in_index)
 {
 	struct rev_info rev;

@@ -546,7 +546,8 @@ int index_differs_from(const char *def, int diff_flags,
 	setup_revisions(0, NULL, &rev, &opt);
 	DIFF_OPT_SET(&rev.diffopt, QUICK);
 	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-	rev.diffopt.flags |= diff_flags;
+	if (flags)
+		diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);

diff.c +1 -1

index 6fd288420..3ad9c9b31 100644
@@ -5899,7 +5899,7 @@ int diff_can_quit_early(struct diff_options *opt)
 static int is_submodule_ignored(const char *path, struct diff_options *options)
 {
 	int ignored = 0;
-	unsigned orig_flags = options->flags;
+	struct diff_flags orig_flags = options->flags;
 	if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
 		set_diffopt_flags_from_submodule_config(options, path);
 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))

diff.h +54 ~~-39~~

111 lines not on this page · raw message

sequencer.c +3 -2

index f2a10cc4f..c0410e491 100644
@@ -959,7 +959,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		unborn = get_oid("HEAD", &head);
 		if (unborn)
 			oidcpy(&head, &empty_tree_oid);
-		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0, 0))
+		if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD",
+				       NULL, 0))
 			return error_dirty_index(opts);
 	}
 	discard_cache();

@@ -2279,7 +2280,7 @@ int sequencer_continue(struct replay_opts *opts)
 			if (res)
 				goto release_todo_list;
 		}
-		if (index_differs_from("HEAD", 0, 0)) {
+		if (index_differs_from("HEAD", NULL, 0)) {
 			res = error_dirty_index(opts);
 			goto release_todo_list;
 		}

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Stefan Beller

2017-10-31 21:32 UTC #34 lore raw

Re: [PATCH v3 2/8] diff: convert flags to be stored in bitfields

Quoted 7 lines · Brandon Williams

We cannot add many more flags to the diff machinery due to the limitations of the number of flags that can be stored in a single unsigned int. In order to allow for more flags to be added to the diff machinery in the future this patch converts the flags to be stored in bitfields in 'struct diff_flags'.
Signed-off-by Brandon Williams <[email protected]>

Thanks for this cleanup series!
Stefan

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 65

Quoted 2 lines

diff.h +2 -0

+struct diff_flags {
+       unsigned RECURSIVE:1;

After some quick research our coding style on bit fields is twofold:
Most older code is this way and more recent code seems to prefer

    unsigned <FLAGNAME> SP : SP ;

If this turns out to be the only nit, I would ignore it for the sake of faster settlement of the series.

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 104

Quoted 8 lines

diff.h +8 -0

+static inline void diff_flags_or(struct diff_flags *a,
+                                const struct diff_flags *b)
+{
+       char *tmp_a = (char *)a;
+       const char *tmp_b = (const char *)b;
+       int i;
+
+       for (i = 0; i < sizeof(struct diff_flags); i++)

I think most of the code prefers to put the variable into the sizeof argument i.e. 'sizeof(*a)', as that is presumably more maintainable. (If the type of 'a' changes, then we don't forget to adapt this place, but the compiler can take care of it.

Junio C Hamano

2017-11-01 01:26 UTC #35 lore raw

Re: [PATCH v3 2/8] diff: convert flags to be stored in bitfields

Quoted 4 lines · Stefan Beller

After some quick research our coding style on bit fields is twofold:
Most older code is this way and more recent code seems to prefer
    unsigned <FLAGNAME> SP : SP ;

Yes, we are very inconsistent. What does the clang format rules Brandon came up with have to say on this? FWIW, checkpatch.pl is unhappy without spaces on both side.

Quoted 5 lines

diff.h +5 -0

+static inline void diff_flags_or(struct diff_flags *a,
+                                const struct diff_flags *b)
+{
+       char *tmp_a = (char *)a;
+       const char *tmp_b = (const char *)b;

Quoted 8 lines

diff.h typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) · line 104
Quoted 3 lines
diff.h +3 -0
+       int i;
+
+       for (i = 0; i < sizeof(struct diff_flags); i++)
I think most of the code prefers to put the variable into the sizeof argument i.e. 'sizeof(*a)', as that is presumably more maintainable. (If the type of 'a' changes, then we don't forget to adapt this place, but the compiler can take care of it.

Yup, but in this case we won't change the type, no?

Stefan Beller

2017-11-01 17:11 UTC #36 lore raw

Re: [PATCH v3 2/8] diff: convert flags to be stored in bitfields

Quoted 10 lines · Junio C Hamano

Quoted 4 lines · Stefan Beller
After some quick research our coding style on bit fields is twofold:
Most older code is this way and more recent code seems to prefer
    unsigned <FLAGNAME> SP : SP ;
Yes, we are very inconsistent. What does the clang format rules Brandon came up with have to say on this? FWIW, checkpatch.pl is unhappy without spaces on both side.

clang-format --style file diff.h

...
#define DIFF_FLAGS_INIT \

{               \
                0       \
}
struct diff_flags {
        unsigned recursive : 1;
        unsigned tree_in_recursive : 1;
        unsigned binary : 1;
        unsigned text : 1;

...

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
+ for (i = 0; i < sizeof(struct diff_flags); i++)
I think most of the code prefers to put the variable into the sizeof argument i.e. 'sizeof(*a)', as that is presumably more maintainable. (If the type of 'a' changes, then we don't forget to adapt this place, but the compiler can take care of it.
Yup, but in this case we won't change the type, no?

Most likely. If we were to change the type we'd have to rename the function and probably rewrite the body, too. I just mentioned it from a consistency point of view. (exceptions are both a mental burden to humans as well as to machines using .clang-format et al. The fewer "this time is different" calls we have, the better)

Brandon Williams

author

2017-10-31 18:19 UTC #37 lore raw

PATCH v3 3/8 diff: add flag to indicate textconv was set via cmdline

In reply to Brandon Williams

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/log.c | 2 +-
 diff.c        | 5 +++--
 diff.h        | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

builtin/log.c +1 -1

index dc28d43eb..82131751d 100644
@@ -485,7 +485,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+	if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
 	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
 		return stream_blob_to_fd(1, oid, NULL, 0);

diff.c +3 -2

index 3ad9c9b31..11fccbd10 100644
@@ -4762,9 +4762,10 @@ int diff_opt_parse(struct diff_options *options,
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
-	else if (!strcmp(arg, "--textconv"))
+	else if (!strcmp(arg, "--textconv")) {
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--no-textconv"))
+		DIFF_OPT_SET(options, TEXTCONV_SET_VIA_CMDLINE);
+	} else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--ignore-submodules")) {
 		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);

diff.h +1 -0

index e512cf44d..d077d3c10 100644
@@ -83,6 +83,7 @@ struct diff_flags {
 	unsigned DIRSTAT_CUMULATIVE:1;
 	unsigned DIRSTAT_BY_FILE:1;
 	unsigned ALLOW_TEXTCONV:1;
+	unsigned TEXTCONV_SET_VIA_CMDLINE:1;
 	unsigned DIFF_FROM_CONTENTS:1;
 	unsigned DIRTY_SUBMODULES:1;
 	unsigned IGNORE_UNTRACKED_IN_SUBMODULES:1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #38 lore raw

PATCH v3 4/8 diff: remove touched flags

In reply to Brandon Williams

Now that the set of parallel touched flags are no longer being used, remove them.

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/log.c | 2 --
 diff.h        | 6 ++----
 2 files changed, 2 insertions(+), 6 deletions(-)

builtin/log.c +0 -2

index 82131751d..9c0815270 100644
@@ -134,8 +134,6 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
-
-	memset(&rev->diffopt.touched_flags, 0, sizeof(struct diff_flags));
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

diff.h +2 -4

index d077d3c10..26a1cab87 100644
@@ -107,9 +107,8 @@ static inline void diff_flags_or(struct diff_flags *a,
 }
 
 #define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
-#define DIFF_OPT_TOUCHED(opts, flag)	((opts)->touched_flags.flag)
-#define DIFF_OPT_SET(opts, flag)	(((opts)->flags.flag = 1),((opts)->touched_flags.flag = 1))
-#define DIFF_OPT_CLR(opts, flag)	(((opts)->flags.flag = 0),((opts)->touched_flags.flag = 1))
+#define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
+#define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
 
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)

@@ -138,7 +137,6 @@ struct diff_options {
 	const char *line_prefix;
 	size_t line_prefix_length;
 	struct diff_flags flags;
-	struct diff_flags touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #39 lore raw

PATCH v3 5/8 diff: remove DIFF_OPT_TST macro

In reply to Brandon Williams

Remove the DIFF_OPT_TST macro and instead access the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_TST(&E, fld)
	+ E.flags.fld

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_TST(ptr, fld)
	+ ptr->flags.fld

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c            |  8 +++---
 builtin/am.c       |  2 +-
 builtin/blame.c    |  4 +--
 builtin/diff.c     |  2 +-
 builtin/log.c      | 12 ++++-----
 builtin/rev-list.c |  2 +-
 combine-diff.c     |  6 ++---
 diff-lib.c         | 19 +++++++-------
 diff-no-index.c    |  2 +-
 diff.c             | 76 +++++++++++++++++++++++++++---------------------------
 diff.h             |  1 -
 diffcore-pickaxe.c |  8 +++---
 diffcore-rename.c  |  6 ++---
 log-tree.c         |  2 +-
 revision.c         |  4 +--
 submodule.c        |  2 +-
 tree-diff.c        | 12 ++++-----
 17 files changed, 84 insertions(+), 84 deletions(-)

blame.c +4 -4

index f575e9cbf..7c019bc7c 100644
@@ -209,7 +209,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
-			if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+			if (opt->flags.ALLOW_TEXTCONV &&
 			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
 				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
 			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)

@@ -293,7 +293,7 @@ static void fill_origin_blob(struct diff_options *opt,
 		unsigned long file_size;
 
 		(*num_read_blob)++;
-		if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+		if (opt->flags.ALLOW_TEXTCONV &&
 		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
 			;
 		else

@@ -1262,7 +1262,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 			      &target->commit->tree->object.oid,
 			      "", &diff_opts);
 
-	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
+	if (!diff_opts.flags.FIND_COPIES_HARDER)
 		diffcore_std(&diff_opts);
 
 	do {

@@ -1825,7 +1825,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 		if (fill_blob_sha1_and_mode(o))
 			die(_("no such path %s in %s"), path, final_commit_name);
 
-		if (DIFF_OPT_TST(&sb->revs->diffopt, ALLOW_TEXTCONV) &&
+		if (sb->revs->diffopt.flags.ALLOW_TEXTCONV &&
 		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
 				    &sb->final_buf_size))
 			;

builtin/am.c +1 -1

index d7513f537..fc54724cc 100644
@@ -1168,7 +1168,7 @@ static int index_has_changes(struct strbuf *sb)
 			strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
 		}
 		diff_flush(&opt);
-		return DIFF_OPT_TST(&opt, HAS_CHANGES) != 0;
+		return opt.flags.HAS_CHANGES != 0;
 	} else {
 		for (i = 0; sb && i < active_nr; i++) {
 			if (i)

builtin/blame.c +2 -2

index 67adaef4d..a29574984 100644
@@ -734,7 +734,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
-	no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
+	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
 	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
 	argc = parse_options_end(&ctx);

@@ -803,7 +803,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 	blame_date_width -= 1; /* strip the null */
 
-	if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
+	if (revs.diffopt.flags.FIND_COPIES_HARDER)
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);

builtin/diff.c +1 -1

index f5bbd4d75..8cbaf4538 100644
@@ -44,7 +44,7 @@ static void stuff_change(struct diff_options *opt,
 	    !oidcmp(old_oid, new_oid) && (old_mode == new_mode))
 		return;
 
-	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
+	if (opt->flags.REVERSE_DIFF) {
 		SWAP(old_mode, new_mode);
 		SWAP(old_oid, new_oid);
 		SWAP(old_path, new_path);

builtin/log.c +6 -6

index 9c0815270..2b2d36678 100644
@@ -181,7 +181,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		init_display_notes(&rev->notes_opt);
 
 	if (rev->diffopt.pickaxe || rev->diffopt.filter ||
-	    DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES))
+	    rev->diffopt.flags.FOLLOW_RENAMES)
 		rev->always_show_header = 0;
 
 	if (source)

@@ -391,7 +391,7 @@ static int cmd_log_walk(struct rev_info *rev)
 		fclose(rev->diffopt.file);
 
 	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
-	    DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) {
+	    rev->diffopt.flags.CHECK_FAILED) {
 		return 02;
 	}
 	return diff_result_code(&rev->diffopt, 0);

@@ -483,8 +483,8 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!DIFF_OPT_TST(&rev->diffopt, TEXTCONV_SET_VIA_CMDLINE) ||
-	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
+	if (!rev->diffopt.flags.TEXTCONV_SET_VIA_CMDLINE ||
+	    !rev->diffopt.flags.ALLOW_TEXTCONV)
 		return stream_blob_to_fd(1, oid, NULL, 0);
 
 	if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,

@@ -666,7 +666,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 static void log_setup_revisions_tweak(struct rev_info *rev,
 				      struct setup_revision_opt *opt)
 {
-	if (DIFF_OPT_TST(&rev->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
 	    rev->prune_data.nr == 1)
 		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);

@@ -1612,7 +1612,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.zero_commit = zero_commit;
 
-	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
+	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
 	if (rev.show_notes)

builtin/rev-list.c +1 -1

index c1c74d4a7..c62382171 100644
@@ -294,7 +294,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (revs.bisect)
 		bisect_list = 1;
 
-	if (DIFF_OPT_TST(&revs.diffopt, QUICK))
+	if (revs.diffopt.flags.QUICK)
 		info.flags |= REV_LIST_QUIET;
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];

combine-diff.c +3 -3

index 9e163d5ad..40cb4dd1a 100644
@@ -898,7 +898,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 				 int show_file_header)
 {
 	struct diff_options *opt = &rev->diffopt;
-	int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
+	int abbrev = opt->flags.FULL_INDEX ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
 	const char *a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
 	const char *b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
 	const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);

@@ -987,7 +987,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	userdiff = userdiff_find_by_path(elem->path);
 	if (!userdiff)
 		userdiff = userdiff_find_by_name("default");
-	if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV))
+	if (opt->flags.ALLOW_TEXTCONV)
 		textconv = userdiff_get_textconv(userdiff);
 
 	/* Read the result of merge first */

@@ -1435,7 +1435,7 @@ void diff_tree_combined(const struct object_id *oid,
 	 * NOTE please keep this semantically in sync with diffcore_std()
 	 */
 	need_generic_pathscan = opt->skip_stat_unmatch	||
-			DIFF_OPT_TST(opt, FOLLOW_RENAMES)	||
+			opt->flags.FOLLOW_RENAMES	||
 			opt->break_opt != -1	||
 			opt->detect_rename	||
 			opt->pickaxe		||

diff-lib.c +10 -9

index ed37f24c6..456971b2c 100644
@@ -72,13 +72,14 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct diff_flags orig_flags = diffopt->flags;
-		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
+		if (!diffopt->flags.OVERRIDE_SUBMODULE_CONFIG)
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
-		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))
+		if (diffopt->flags.IGNORE_SUBMODULES)
 			changed = 0;
-		else if (!DIFF_OPT_TST(diffopt, IGNORE_DIRTY_SUBMODULES)
-		    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES)))
-			*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
+		else if (!diffopt->flags.IGNORE_DIRTY_SUBMODULES &&
+			 (!changed || diffopt->flags.DIRTY_SUBMODULES))
+			*dirty_submodule = is_submodule_modified(ce->name,
+								 diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES);
 		diffopt->flags = orig_flags;
 	}
 	return changed;

@@ -228,7 +229,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 
 		if (!changed && !dirty_submodule) {
 			ce_mark_uptodate(ce);
-			if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+			if (!revs->diffopt.flags.FIND_COPIES_HARDER)
 				continue;
 		}
 		oldmode = ce->ce_mode;

@@ -362,7 +363,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !oidcmp(oid, &old->oid) && !dirty_submodule &&
-	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+	    !revs->diffopt.flags.FIND_COPIES_HARDER)
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,

@@ -493,7 +494,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.head_idx = 1;
 	opts.index_only = cached;
 	opts.diff_index_cached = (cached &&
-				  !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER));
+				  !revs->diffopt.flags.FIND_COPIES_HARDER);
 	opts.merge = 1;
 	opts.fn = oneway_diff;
 	opts.unpack_data = revs;

@@ -551,5 +552,5 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);
-	return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);
+	return (rev.diffopt.flags.HAS_CHANGES != 0);
 }

diff-no-index.c +1 -1

index 80ff17d46..40d17e42c 100644
@@ -184,7 +184,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		if (o->flags.REVERSE_DIFF) {
 			SWAP(mode1, mode2);
 			SWAP(name1, name2);
 		}

diff.c +38 ~~-38~~

260 lines not on this page · raw message

diff.h +0 -1

index 26a1cab87..c3798a104 100644
@@ -106,7 +106,6 @@ static inline void diff_flags_or(struct diff_flags *a,
 		tmp_a[i] |= tmp_b[i];
 }
 
-#define DIFF_OPT_TST(opts, flag)	((opts)->flags.flag)
 #define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
 #define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)

diffcore-pickaxe.c +4 -4

index 341529b5a..6193c07f6 100644
@@ -131,7 +131,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	if (!DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two))
 		return 0;
 
-	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+	if (o->flags.ALLOW_TEXTCONV) {
 		textconv_one = get_textconv(p->one);
 		textconv_two = get_textconv(p->two);
 	}

@@ -222,11 +222,11 @@ void diffcore_pickaxe(struct diff_options *o)
 
 	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
 		int cflags = REG_EXTENDED | REG_NEWLINE;
-		if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
+		if (o->flags.PICKAXE_IGNORE_CASE)
 			cflags |= REG_ICASE;
 		regcomp_or_die(&regex, needle, cflags);
 		regexp = &regex;
-	} else if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) &&
+	} else if (o->flags.PICKAXE_IGNORE_CASE &&
 		   has_non_ascii(needle)) {
 		struct strbuf sb = STRBUF_INIT;
 		int cflags = REG_NEWLINE | REG_ICASE;

@@ -236,7 +236,7 @@ void diffcore_pickaxe(struct diff_options *o)
 		strbuf_release(&sb);
 		regexp = &regex;
 	} else {
-		kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
+		kws = kwsalloc(o->flags.PICKAXE_IGNORE_CASE
 			       ? tolower_trans_tbl : NULL);
 		kwsincr(kws, needle, strlen(needle));
 		kwsprep(kws);

diffcore-rename.c +3 -3

index 0d8c3d2ee..bd077ee11 100644
@@ -405,7 +405,7 @@ static int too_many_rename_candidates(int num_create,
 		num_src > num_create ? num_src : num_create;
 
 	/* Are we running under -C -C? */
-	if (!DIFF_OPT_TST(options, FIND_COPIES_HARDER))
+	if (!options->flags.FIND_COPIES_HARDER)
 		return 1;
 
 	/* Would we bust the limit if we were running under -C? */

@@ -463,7 +463,7 @@ void diffcore_rename(struct diff_options *options)
 			else if (options->single_follow &&
 				 strcmp(options->single_follow, p->two->path))
 				continue; /* not interested */
-			else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
+			else if (!options->flags.RENAME_EMPTY &&
 				 is_empty_blob_oid(&p->two->oid))
 				continue;
 			else if (add_rename_dst(p->two) < 0) {

@@ -473,7 +473,7 @@ void diffcore_rename(struct diff_options *options)
 				goto cleanup;
 			}
 		}
-		else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
+		else if (!options->flags.RENAME_EMPTY &&
 			 is_empty_blob_oid(&p->one->oid))
 			continue;
 		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {

log-tree.c +1 -1

index cea056234..460bb5498 100644
@@ -793,7 +793,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	struct commit_list *parents;
 	struct object_id *oid;
 
-	if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
+	if (!opt->diff && !opt->diffopt.flags.EXIT_WITH_STATUS)
 		return 0;
 
 	parse_commit_or_die(commit);

revision.c +2 -2

index d167223e6..c893d9166 100644
@@ -2399,7 +2399,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	/* Pickaxe, diff-filter and rename following need diffs */
 	if (revs->diffopt.pickaxe ||
 	    revs->diffopt.filter ||
-	    DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
+	    revs->diffopt.flags.FOLLOW_RENAMES)
 		revs->diff = 1;
 
 	if (revs->topo_order)

@@ -2408,7 +2408,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->prune_data.nr) {
 		copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
+		if (!revs->diffopt.flags.FOLLOW_RENAMES)
 			revs->prune = 1;
 		if (!revs->full_diff)
 			copy_pathspec(&revs->diffopt.pathspec,

submodule.c +1 -1

index 63e7094e1..58da9c668 100644
@@ -616,7 +616,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 	argv_array_pushf(&cp.args, "--color=%s", want_color(o->use_color) ?
 			 "always" : "never");
 
-	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+	if (o->flags.REVERSE_DIFF) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
 				 o->b_prefix, path);
 		argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",

tree-diff.c +6 -6

index 4bb93155b..ed0aa6a62 100644
@@ -212,9 +212,9 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 		mode = 0;
 	}
 
-	if (DIFF_OPT_TST(opt, RECURSIVE) && isdir) {
+	if (opt->flags.RECURSIVE && isdir) {
 		recurse = 1;
-		emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE);
+		emitthis = opt->flags.TREE_IN_RECURSIVE;
 	}
 
 	if (emitthis) {

@@ -425,7 +425,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	ttree = fill_tree_descriptor(&t, oid);
 
 	/* Enable recursion indefinitely */
-	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
+	opt->pathspec.recursive = opt->flags.RECURSIVE;
 
 	for (;;) {
 		int imin, cmp;

@@ -484,7 +484,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t = p[imin] */
 		if (cmp == 0) {
 			/* are either pi > p[imin] or diff(t,pi) != ø ? */
-			if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER)) {
+			if (!opt->flags.FIND_COPIES_HARDER) {
 				for (i = 0; i < nparent; ++i) {
 					/* p[i] > p[imin] */
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)

@@ -522,7 +522,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t > p[imin] */
 		else {
 			/* ∀i pi=p[imin] -> D += "-p[imin]" */
-			if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER)) {
+			if (!opt->flags.FIND_COPIES_HARDER) {
 				for (i = 0; i < nparent; ++i)
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)
 						goto skip_emit_tp;

@@ -706,7 +706,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 	strbuf_addstr(&base, base_str);
 
 	retval = ll_diff_tree_oid(old_oid, new_oid, &base, opt);
-	if (!*base_str && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
+	if (!*base_str && opt->flags.FOLLOW_RENAMES && diff_might_be_rename())
 		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
 	strbuf_release(&base);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #40 lore raw

PATCH v3 6/8 diff: remove DIFF_OPT_SET macro

In reply to Brandon Williams

Remove the DIFF_OPT_SET macro and instead set the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_SET(&E, fld)
	+ E.flags.fld = 1

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_SET(ptr, fld)
	+ ptr->flags.fld = 1

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c               |  8 +++----
 builtin/add.c         |  4 ++--
 builtin/am.c          |  8 +++----
 builtin/blame.c       |  4 ++--
 builtin/diff.c        |  6 ++---
 builtin/fast-export.c |  2 +-
 builtin/log.c         | 14 +++++------
 builtin/reset.c       |  2 +-
 combine-diff.c        |  2 +-
 diff-lib.c            |  4 ++--
 diff-no-index.c       |  6 ++---
 diff.c                | 66 +++++++++++++++++++++++++--------------------------
 diff.h                |  1 -
 merge-recursive.c     |  2 +-
 notes-merge.c         |  4 ++--
 patch-ids.c           |  2 +-
 revision.c            | 16 ++++++-------
 submodule.c           |  8 +++----
 tree-diff.c           |  4 ++--
 wt-status.c           | 18 +++++++-------
 20 files changed, 90 insertions(+), 91 deletions(-)

blame.c +4 -4

index 7c019bc7c..dc9cc237b 100644
@@ -541,7 +541,7 @@ static struct blame_origin *find_origin(struct commit *parent,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;

@@ -615,7 +615,7 @@ static struct blame_origin *find_rename(struct commit *parent,
 	int i;
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;

@@ -1238,7 +1238,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		return; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.flags.RECURSIVE = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	diff_setup_done(&diff_opts);

@@ -1253,7 +1253,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
+		diff_opts.flags.FIND_COPIES_HARDER = 1;
 
 	if (is_null_oid(&target->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);

builtin/add.c +2 -2

index b70e8a779..e1d83b69a 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

@@ -218,7 +218,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev.diffopt.use_color = 0;
-	DIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES);
+	rev.diffopt.flags.IGNORE_DIRTY_SUBMODULES = 1;
 	out = open(file, O_CREAT | O_WRONLY, 0666);
 	if (out < 0)
 		die(_("Could not open '%s' for writing."), file);

builtin/am.c +4 -4

index fc54724cc..015425a0f 100644
@@ -1157,9 +1157,9 @@ static int index_has_changes(struct strbuf *sb)
 		struct diff_options opt;
 
 		diff_setup(&opt);
-		DIFF_OPT_SET(&opt, EXIT_WITH_STATUS);
+		opt.flags.EXIT_WITH_STATUS = 1;
 		if (!sb)
-			DIFF_OPT_SET(&opt, QUICK);
+			opt.flags.QUICK = 1;
 		do_diff_cache(&head, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {

@@ -1409,8 +1409,8 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
 	rev_info.show_root_diff = 1;
 	rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev_info.no_commit_id = 1;
-	DIFF_OPT_SET(&rev_info.diffopt, BINARY);
-	DIFF_OPT_SET(&rev_info.diffopt, FULL_INDEX);
+	rev_info.diffopt.flags.BINARY = 1;
+	rev_info.diffopt.flags.FULL_INDEX = 1;
 	rev_info.diffopt.use_color = 0;
 	rev_info.diffopt.file = fp;
 	rev_info.diffopt.close_file = 1;

builtin/blame.c +2 -2

index a29574984..76994aa64 100644
@@ -708,8 +708,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	git_config(git_blame_config, &output_option);
 	init_revisions(&revs, NULL);
 	revs.date_mode = blame_date_mode;
-	DIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);
-	DIFF_OPT_SET(&revs.diffopt, FOLLOW_RENAMES);
+	revs.diffopt.flags.ALLOW_TEXTCONV = 1;
+	revs.diffopt.flags.FOLLOW_RENAMES = 1;
 
 	save_commit_buffer = 0;
 	dashdash_pos = 0;

builtin/diff.c +3 -3

index 8cbaf4538..ed41eb5a5 100644
@@ -350,8 +350,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.stat_graph_width = -1;
 
 	/* Default to let external and textconv be used */
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+	rev.diffopt.flags.ALLOW_EXTERNAL = 1;
+	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
 
 	if (nongit)
 		die(_("Not a git repository"));

@@ -361,7 +361,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		diff_setup_done(&rev.diffopt);
 	}
 
-	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	rev.diffopt.flags.RECURSIVE = 1;
 
 	setup_diff_pager(&rev.diffopt);

builtin/fast-export.c +1 -1

index 2fb60d6d4..35c8fb65d 100644
@@ -1066,7 +1066,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
 	revs.diffopt.format_callback_data = &paths_of_changed_objects;
-	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
+	revs.diffopt.flags.RECURSIVE = 1;
 	while ((commit = get_revision(&revs))) {
 		if (has_unshown_parent(commit)) {
 			add_object_array(&commit->object, NULL, &commits);

builtin/log.c +7 -7

index 2b2d36678..4a24d3e48 100644
@@ -121,16 +121,16 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
 	if (default_follow)
-		DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
+		rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES = 1;
 	rev->verbose_header = 1;
-	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
+	rev->diffopt.flags.RECURSIVE = 1;
 	rev->diffopt.stat_width = -1; /* use full terminal width */
 	rev->diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
 	rev->abbrev_commit = default_abbrev_commit;
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
-	DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
+	rev->diffopt.flags.ALLOW_TEXTCONV = 1;
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);

@@ -668,7 +668,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
 {
 	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
 	    rev->prune_data.nr == 1)
-		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
+		rev->diffopt.flags.FOLLOW_RENAMES = 1;
 
 	/* Turn --cc/-c into -p --cc/-c when -p was not given */
 	if (!rev->diffopt.output_format && rev->combine_merges)

@@ -1340,7 +1340,7 @@ static void prepare_bases(struct base_tree_info *bases,
 		return;
 
 	diff_setup(&diffopt);
-	DIFF_OPT_SET(&diffopt, RECURSIVE);
+	diffopt.flags.RECURSIVE = 1;
 	diff_setup_done(&diffopt);
 
 	oidcpy(&bases->base_commit, &base->object.oid);

@@ -1511,7 +1511,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
-	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	rev.diffopt.flags.RECURSIVE = 1;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";

@@ -1613,7 +1613,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.zero_commit = zero_commit;
 
 	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
-		DIFF_OPT_SET(&rev.diffopt, BINARY);
+		rev.diffopt.flags.BINARY = 1;
 
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);

builtin/reset.c +1 -1

index ea2fad5a0..206819ef1 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
+	opt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

combine-diff.c +1 -1

index 40cb4dd1a..204b0dfce 100644
@@ -1413,7 +1413,7 @@ void diff_tree_combined(const struct object_id *oid,
 
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
-	DIFF_OPT_SET(&diffopts, RECURSIVE);
+	diffopts.flags.RECURSIVE = 1;
 	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
 
 	/* find set of paths that everybody touches

diff-lib.c +2 -2

index 456971b2c..f1cc3fd51 100644
@@ -545,8 +545,8 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	memset(&opt, 0, sizeof(opt));
 	opt.def = def;
 	setup_revisions(0, NULL, &rev, &opt);
-	DIFF_OPT_SET(&rev.diffopt, QUICK);
-	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
+	rev.diffopt.flags.QUICK = 1;
+	rev.diffopt.flags.EXIT_WITH_STATUS = 1;
 	if (flags)
 		diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;

diff-no-index.c +3 -3

index 40d17e42c..a3e194340 100644
@@ -276,16 +276,16 @@ void diff_no_index(struct rev_info *revs,
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
+	revs->diffopt.flags.NO_INDEX = 1;
 
-	DIFF_OPT_SET(&revs->diffopt, RELATIVE_NAME);
+	revs->diffopt.flags.RELATIVE_NAME = 1;
 	revs->diffopt.prefix = prefix;
 
 	revs->max_count = -2;
 	diff_setup_done(&revs->diffopt);
 
 	setup_diff_pager(&revs->diffopt);
-	DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
+	revs->diffopt.flags.EXIT_WITH_STATUS = 1;
 
 	if (queue_diff(&revs->diffopt, paths[0], paths[1]))
 		exit(1);

diff.c +33 ~~-33~~

192 lines not on this page · raw message

diff.h +0 -1

index c3798a104..d53e5a2da 100644
@@ -106,7 +106,6 @@ static inline void diff_flags_or(struct diff_flags *a,
 		tmp_a[i] |= tmp_b[i];
 }
 
-#define DIFF_OPT_SET(opts, flag)	((opts)->flags.flag = 1)
 #define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
 
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)

merge-recursive.c +1 -1

index 1d3f8f0d2..e7b3df45c 100644
@@ -540,7 +540,7 @@ static struct string_list *get_renames(struct merge_options *o,
 		return renames;
 
 	diff_setup(&opts);
-	DIFF_OPT_SET(&opts, RECURSIVE);
+	opts.flags.RECURSIVE = 1;
 	DIFF_OPT_CLR(&opts, RENAME_EMPTY);
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :

notes-merge.c +2 -2

index 4352c34a6..b50111cf2 100644
@@ -125,7 +125,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	       oid_to_hex(base), oid_to_hex(remote));
 
 	diff_setup(&opt);
-	DIFF_OPT_SET(&opt, RECURSIVE);
+	opt.flags.RECURSIVE = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, remote, "", &opt);

@@ -188,7 +188,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	       len, oid_to_hex(base), oid_to_hex(local));
 
 	diff_setup(&opt);
-	DIFF_OPT_SET(&opt, RECURSIVE);
+	opt.flags.RECURSIVE = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, local, "", &opt);

patch-ids.c +1 -1

index 7a583b301..189869e57 100644
@@ -61,7 +61,7 @@ int init_patch_ids(struct patch_ids *ids)
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
 	ids->diffopts.detect_rename = 0;
-	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
+	ids->diffopts.flags.RECURSIVE = 1;
 	diff_setup_done(&ids->diffopts);
 	hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
 	return 0;

revision.c +8 -8

index c893d9166..f019dd277 100644
@@ -410,7 +410,7 @@ static void file_add_remove(struct diff_options *options,
 
 	tree_difference |= diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		DIFF_OPT_SET(options, HAS_CHANGES);
+		options->flags.HAS_CHANGES = 1;
 }
 
 static void file_change(struct diff_options *options,

@@ -422,7 +422,7 @@ static void file_change(struct diff_options *options,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	DIFF_OPT_SET(options, HAS_CHANGES);
+	options->flags.HAS_CHANGES = 1;
 }
 
 static int rev_compare_tree(struct rev_info *revs,

@@ -1403,8 +1403,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	DIFF_OPT_SET(&revs->pruning, RECURSIVE);
-	DIFF_OPT_SET(&revs->pruning, QUICK);
+	revs->pruning.flags.RECURSIVE = 1;
+	revs->pruning.flags.QUICK = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->sort_order = REV_SORT_IN_GRAPH_ORDER;

@@ -1917,11 +1917,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		die("--unpacked=<packfile> no longer supported.");
 	} else if (!strcmp(arg, "-r")) {
 		revs->diff = 1;
-		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+		revs->diffopt.flags.RECURSIVE = 1;
 	} else if (!strcmp(arg, "-t")) {
 		revs->diff = 1;
-		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
-		DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
+		revs->diffopt.flags.RECURSIVE = 1;
+		revs->diffopt.flags.TREE_IN_RECURSIVE = 1;
 	} else if (!strcmp(arg, "-m")) {
 		revs->ignore_merges = 0;
 	} else if (!strcmp(arg, "-c")) {

@@ -2066,7 +2066,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
 		revs->grep_filter.ignore_case = 1;
-		DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
+		revs->diffopt.flags.PICKAXE_IGNORE_CASE = 1;
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
 	} else if (!strcmp(arg, "--perl-regexp") || !strcmp(arg, "-P")) {

submodule.c +4 -4

index 58da9c668..7e7998592 100644
@@ -183,7 +183,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		if (ignore)
 			handle_ignore_submodules_arg(diffopt, ignore);
 		else if (is_gitmodules_unmerged(&the_index))
-			DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
+			diffopt->flags.IGNORE_SUBMODULES = 1;
 	}
 }

@@ -407,11 +407,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
 
 	if (!strcmp(arg, "all"))
-		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
+		diffopt->flags.IGNORE_SUBMODULES = 1;
 	else if (!strcmp(arg, "untracked"))
-		DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+		diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
 	else if (!strcmp(arg, "dirty"))
-		DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
+		diffopt->flags.IGNORE_DIRTY_SUBMODULES = 1;
 	else if (strcmp(arg, "none"))
 		die("bad --ignore-submodules argument: %s", arg);
 }

tree-diff.c +2 -2

index ed0aa6a62..b996a23bb 100644
@@ -608,8 +608,8 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
-	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
+	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.FIND_COPIES_HARDER = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->pathspec.items[0].match;
 	diff_opts.break_opt = opt->break_opt;

wt-status.c +9 -9

index 29bc64cc0..59f9f3a0b 100644
@@ -559,12 +559,12 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	rev.diffopt.flags.DIRTY_SUBMODULES = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (!s->show_untracked_files)
-		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+		rev.diffopt.flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
 	if (s->ignore_submodule_arg) {
-		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+		rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;

@@ -583,7 +583,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
-	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (s->ignore_submodule_arg) {
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);

@@ -949,7 +949,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	init_revisions(&rev, NULL);
-	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 
 	memset(&opt, 0, sizeof(opt));

@@ -2263,8 +2263,8 @@ int has_unstaged_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
+	rev_info.diffopt.flags.QUICK = 1;
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_files(&rev_info, 0);
 	return diff_result_code(&rev_info.diffopt, result);

@@ -2283,8 +2283,8 @@ int has_uncommitted_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
+	rev_info.diffopt.flags.QUICK = 1;
 	add_head_to_pending(&rev_info);
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_index(&rev_info, 1);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Brandon Williams

author

2017-10-31 18:19 UTC #41 lore raw

PATCH v3 7/8 diff: remove DIFF_OPT_CLR macro

In reply to Brandon Williams

Remove the DIFF_OPT_SET macro and instead set the flags directly. This conversion is done using the following semantic patch:

	@@
	expression E;
	identifier fld;
	@@
	- DIFF_OPT_CLR(&E, fld)
	+ E.flags.fld = 0

	@@
	type T;
	T *ptr;
	identifier fld;
	@@
	- DIFF_OPT_CLR(ptr, fld)
	+ ptr->flags.fld = 0

Signed-off-by Brandon Williams <[email protected]>

---
 builtin/blame.c   |  2 +-
 combine-diff.c    |  2 +-
 diff.c            | 28 ++++++++++++++--------------
 diff.h            |  2 --
 merge-recursive.c |  2 +-
 revision.c        |  4 ++--
 submodule.c       |  6 +++---
 7 files changed, 22 insertions(+), 24 deletions(-)

builtin/blame.c +1 -1

index 76994aa64..79db9e849 100644
@@ -736,7 +736,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
-	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
+	revs.diffopt.flags.FOLLOW_RENAMES = 0;
 	argc = parse_options_end(&ctx);
 
 	if (incremental || (output_option & OUTPUT_PORCELAIN)) {

combine-diff.c +1 -1

index 204b0dfce..5a3a8b49b 100644
@@ -1414,7 +1414,7 @@ void diff_tree_combined(const struct object_id *oid,
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
 	diffopts.flags.RECURSIVE = 1;
-	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
+	diffopts.flags.ALLOW_EXTERNAL = 0;
 
 	/* find set of paths that everybody touches
 	 *

diff.c +14 ~~-14~~

index 6dea186d8..e5f9d3078 100644
@@ -124,16 +124,16 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
 	for (i = 0; i < params.nr; i++) {
 		const char *p = params.items[i].string;
 		if (!strcmp(p, "changes")) {
-			DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
-			DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
+			options->flags.DIRSTAT_BY_LINE = 0;
+			options->flags.DIRSTAT_BY_FILE = 0;
 		} else if (!strcmp(p, "lines")) {
 			options->flags.DIRSTAT_BY_LINE = 1;
-			DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
+			options->flags.DIRSTAT_BY_FILE = 0;
 		} else if (!strcmp(p, "files")) {
-			DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
+			options->flags.DIRSTAT_BY_LINE = 0;
 			options->flags.DIRSTAT_BY_FILE = 1;
 		} else if (!strcmp(p, "noncumulative")) {
-			DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
+			options->flags.DIRSTAT_CUMULATIVE = 0;
 		} else if (!strcmp(p, "cumulative")) {
 			options->flags.DIRSTAT_CUMULATIVE = 1;
 		} else if (isdigit(*p)) {

@@ -4205,7 +4205,7 @@ void diff_setup_done(struct diff_options *options)
 	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
 		options->flags.DIFF_FROM_CONTENTS = 1;
 	else
-		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
+		options->flags.DIFF_FROM_CONTENTS = 0;
 
 	if (options->flags.FIND_COPIES_HARDER)
 		options->detect_rename = DIFF_DETECT_COPY;

@@ -4640,7 +4640,7 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--rename-empty"))
 		options->flags.RENAME_EMPTY = 1;
 	else if (!strcmp(arg, "--no-rename-empty"))
-		DIFF_OPT_CLR(options, RENAME_EMPTY);
+		options->flags.RENAME_EMPTY = 0;
 	else if (!strcmp(arg, "--relative"))
 		options->flags.RELATIVE_NAME = 1;
 	else if (skip_prefix(arg, "--relative=", &arg)) {

@@ -4697,8 +4697,8 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--follow"))
 		options->flags.FOLLOW_RENAMES = 1;
 	else if (!strcmp(arg, "--no-follow")) {
-		DIFF_OPT_CLR(options, FOLLOW_RENAMES);
-		DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+		options->flags.FOLLOW_RENAMES = 0;
+		options->flags.DEFAULT_FOLLOW_RENAMES = 0;
 	} else if (!strcmp(arg, "--color"))
 		options->use_color = 1;
 	else if (skip_prefix(arg, "--color=", &arg)) {

@@ -4761,12 +4761,12 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--ext-diff"))
 		options->flags.ALLOW_EXTERNAL = 1;
 	else if (!strcmp(arg, "--no-ext-diff"))
-		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+		options->flags.ALLOW_EXTERNAL = 0;
 	else if (!strcmp(arg, "--textconv")) {
 		options->flags.ALLOW_TEXTCONV = 1;
 		options->flags.TEXTCONV_SET_VIA_CMDLINE = 1;
 	} else if (!strcmp(arg, "--no-textconv"))
-		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
+		options->flags.ALLOW_TEXTCONV = 0;
 	else if (!strcmp(arg, "--ignore-submodules")) {
 		options->flags.OVERRIDE_SUBMODULE_CONFIG = 1;
 		handle_ignore_submodules_arg(options, "all");

@@ -4850,7 +4850,7 @@ int diff_opt_parse(struct diff_options *options,
 	else if (!strcmp(arg, "--function-context"))
 		options->flags.FUNCCONTEXT = 1;
 	else if (!strcmp(arg, "--no-function-context"))
-		DIFF_OPT_CLR(options, FUNCCONTEXT);
+		options->flags.FUNCCONTEXT = 0;
 	else if ((argcount = parse_long_opt("output", av, &optarg))) {
 		char *path = prefix_filename(prefix, optarg);
 		options->file = xfopen(path, "w");

@@ -5688,7 +5688,7 @@ void diff_flush(struct diff_options *options)
 		if (options->found_changes)
 			options->flags.HAS_CHANGES = 1;
 		else
-			DIFF_OPT_CLR(options, HAS_CHANGES);
+			options->flags.HAS_CHANGES = 0;
 	}
 }

@@ -5860,7 +5860,7 @@ void diffcore_std(struct diff_options *options)
 	if (diff_queued_diff.nr && !options->flags.DIFF_FROM_CONTENTS)
 		options->flags.HAS_CHANGES = 1;
 	else
-		DIFF_OPT_CLR(options, HAS_CHANGES);
+		options->flags.HAS_CHANGES = 0;
 
 	options->found_follow = 0;
 }

diff.h +0 -2

index d53e5a2da..258d735c1 100644
@@ -106,8 +106,6 @@ static inline void diff_flags_or(struct diff_flags *a,
 		tmp_a[i] |= tmp_b[i];
 }
 
-#define DIFF_OPT_CLR(opts, flag)	((opts)->flags.flag = 0)
-
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)

merge-recursive.c +1 -1

index e7b3df45c..9752aba4e 100644
@@ -541,7 +541,7 @@ static struct string_list *get_renames(struct merge_options *o,
 
 	diff_setup(&opts);
 	opts.flags.RECURSIVE = 1;
-	DIFF_OPT_CLR(&opts, RENAME_EMPTY);
+	opts.flags.RENAME_EMPTY = 0;
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
 			    o->diff_rename_limit >= 0 ? o->diff_rename_limit :

revision.c +2 -2

index f019dd277..6bb873501 100644
@@ -455,7 +455,7 @@ static int rev_compare_tree(struct rev_info *revs,
 	}
 
 	tree_difference = REV_TREE_SAME;
-	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
+	revs->pruning.flags.HAS_CHANGES = 0;
 	if (diff_tree_oid(&t1->object.oid, &t2->object.oid, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;

@@ -471,7 +471,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 		return 0;
 
 	tree_difference = REV_TREE_SAME;
-	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
+	revs->pruning.flags.HAS_CHANGES = 0;
 	retval = diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
 
 	return retval >= 0 && (tree_difference == REV_TREE_SAME);

submodule.c +3 -3

index 7e7998592..62a93bb88 100644
@@ -402,9 +402,9 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
-	DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
-	DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
-	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
+	diffopt->flags.IGNORE_SUBMODULES = 0;
+	diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 0;
+	diffopt->flags.IGNORE_DIRTY_SUBMODULES = 0;
 
 	if (!strcmp(arg, "all"))
 		diffopt->flags.IGNORE_SUBMODULES = 1;

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Stefan Beller

2017-10-31 21:44 UTC #42 lore raw

Re: [PATCH v3 7/8] diff: remove DIFF_OPT_CLR macro

Quoted 1 line · Brandon Williams

Remove the DIFF_OPT_SET macro and instead set the flags directly.

_CLR here

Junio C Hamano

2017-11-01 02:52 UTC #43 lore raw

Re: [PATCH v3 7/8] diff: remove DIFF_OPT_CLR macro

Quoted 4 lines · Stefan Beller

Quoted 1 line · Brandon Williams
Remove the DIFF_OPT_SET macro and instead set the flags directly.
_CLR here

Will squash in. Thanks.

Brandon Williams

author

2017-10-31 18:19 UTC #44 lore raw

PATCH v3 8/8 diff: make struct diff_flags members lowercase

In reply to Brandon Williams

	@@
	expression E;
	@@
	- E.RECURSIVE
	+ E.recursive

	@@
	expression E;
	@@
	- E.TREE_IN_RECURSIVE
	+ E.tree_in_recursive

	@@
	expression E;
	@@
	- E.BINARY
	+ E.binary

	@@
	expression E;
	@@
	- E.TEXT
	+ E.text

	@@
	expression E;
	@@
	- E.FULL_INDEX
	+ E.full_index

	@@
	expression E;
	@@
	- E.SILENT_ON_REMOVE
	+ E.silent_on_remove

	@@
	expression E;
	@@
	- E.FIND_COPIES_HARDER
	+ E.find_copies_harder

	@@
	expression E;
	@@
	- E.FOLLOW_RENAMES
	+ E.follow_renames

	@@
	expression E;
	@@
	- E.RENAME_EMPTY
	+ E.rename_empty

	@@
	expression E;
	@@
	- E.HAS_CHANGES
	+ E.has_changes

	@@
	expression E;
	@@
	- E.QUICK
	+ E.quick

	@@
	expression E;
	@@
	- E.NO_INDEX
	+ E.no_index

	@@
	expression E;
	@@
	- E.ALLOW_EXTERNAL
	+ E.allow_external

	@@
	expression E;
	@@
	- E.EXIT_WITH_STATUS
	+ E.exit_with_status

	@@
	expression E;
	@@
	- E.REVERSE_DIFF
	+ E.reverse_diff

	@@
	expression E;
	@@
	- E.CHECK_FAILED
	+ E.check_failed

	@@
	expression E;
	@@
	- E.RELATIVE_NAME
	+ E.relative_name

	@@
	expression E;
	@@
	- E.IGNORE_SUBMODULES
	+ E.ignore_submodules

	@@
	expression E;
	@@
	- E.DIRSTAT_CUMULATIVE
	+ E.dirstat_cumulative

	@@
	expression E;
	@@
	- E.DIRSTAT_BY_FILE
	+ E.dirstat_by_file

	@@
	expression E;
	@@
	- E.ALLOW_TEXTCONV
	+ E.allow_textconv

	@@
	expression E;
	@@
	- E.TEXTCONV_SET_VIA_CMDLINE
	+ E.textconv_set_via_cmdline

	@@
	expression E;
	@@
	- E.DIFF_FROM_CONTENTS
	+ E.diff_from_contents

	@@
	expression E;
	@@
	- E.DIRTY_SUBMODULES
	+ E.dirty_submodules

	@@
	expression E;
	@@
	- E.IGNORE_UNTRACKED_IN_SUBMODULES
	+ E.ignore_untracked_in_submodules

	@@
	expression E;
	@@
	- E.IGNORE_DIRTY_SUBMODULES
	+ E.ignore_dirty_submodules

	@@
	expression E;
	@@
	- E.OVERRIDE_SUBMODULE_CONFIG
	+ E.override_submodule_config

	@@
	expression E;
	@@
	- E.DIRSTAT_BY_LINE
	+ E.dirstat_by_line

	@@
	expression E;
	@@
	- E.FUNCCONTEXT
	+ E.funccontext

	@@
	expression E;
	@@
	- E.PICKAXE_IGNORE_CASE
	+ E.pickaxe_ignore_case

	@@
	expression E;
	@@
	- E.DEFAULT_FOLLOW_RENAMES
	+ E.default_follow_renames

Signed-off-by Brandon Williams <[email protected]>

---
 blame.c               |  16 ++---
 builtin/add.c         |   4 +-
 builtin/am.c          |  10 +--
 builtin/blame.c       |  10 +--
 builtin/commit.c      |   4 +-
 builtin/diff.c        |   8 +--
 builtin/fast-export.c |   2 +-
 builtin/log.c         |  26 ++++----
 builtin/reset.c       |   2 +-
 builtin/rev-list.c    |   2 +-
 combine-diff.c        |  10 +--
 diff-lib.c            |  22 +++----
 diff-no-index.c       |   8 +--
 diff.c                | 170 +++++++++++++++++++++++++-------------------------
 diff.h                |  62 +++++++++---------
 diffcore-pickaxe.c    |   8 +--
 diffcore-rename.c     |   6 +-
 log-tree.c            |   2 +-
 merge-recursive.c     |   4 +-
 notes-merge.c         |   4 +-
 patch-ids.c           |   2 +-
 revision.c            |  24 +++----
 submodule.c           |  16 ++---
 tree-diff.c           |  16 ++---
 wt-status.c           |  18 +++---
 25 files changed, 228 insertions(+), 228 deletions(-)

blame.c +8 -8

index dc9cc237b..28e03726f 100644
@@ -209,7 +209,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
-			if (opt->flags.ALLOW_TEXTCONV &&
+			if (opt->flags.allow_textconv &&
 			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
 				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
 			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)

@@ -293,7 +293,7 @@ static void fill_origin_blob(struct diff_options *opt,
 		unsigned long file_size;
 
 		(*num_read_blob)++;
-		if (opt->flags.ALLOW_TEXTCONV &&
+		if (opt->flags.allow_textconv &&
 		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
 			;
 		else

@@ -541,7 +541,7 @@ static struct blame_origin *find_origin(struct commit *parent,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;

@@ -615,7 +615,7 @@ static struct blame_origin *find_rename(struct commit *parent,
 	int i;
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;

@@ -1238,7 +1238,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		return; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
+	diff_opts.flags.recursive = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	diff_setup_done(&diff_opts);

@@ -1253,7 +1253,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		diff_opts.flags.FIND_COPIES_HARDER = 1;
+		diff_opts.flags.find_copies_harder = 1;
 
 	if (is_null_oid(&target->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);

@@ -1262,7 +1262,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 			      &target->commit->tree->object.oid,
 			      "", &diff_opts);
 
-	if (!diff_opts.flags.FIND_COPIES_HARDER)
+	if (!diff_opts.flags.find_copies_harder)
 		diffcore_std(&diff_opts);
 
 	do {

@@ -1825,7 +1825,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 		if (fill_blob_sha1_and_mode(o))
 			die(_("no such path %s in %s"), path, final_commit_name);
 
-		if (sb->revs->diffopt.flags.ALLOW_TEXTCONV &&
+		if (sb->revs->diffopt.flags.allow_textconv &&
 		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
 				    &sb->final_buf_size))
 			;

builtin/add.c +2 -2

index e1d83b69a..8d08e99e9 100644
@@ -116,7 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
-	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	rev.diffopt.flags.override_submodule_config = 1;
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	clear_pathspec(&rev.prune_data);

@@ -218,7 +218,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev.diffopt.use_color = 0;
-	rev.diffopt.flags.IGNORE_DIRTY_SUBMODULES = 1;
+	rev.diffopt.flags.ignore_dirty_submodules = 1;
 	out = open(file, O_CREAT | O_WRONLY, 0666);
 	if (out < 0)
 		die(_("Could not open '%s' for writing."), file);

builtin/am.c +5 -5

index 015425a0f..b281d58f3 100644
@@ -1157,9 +1157,9 @@ static int index_has_changes(struct strbuf *sb)
 		struct diff_options opt;
 
 		diff_setup(&opt);
-		opt.flags.EXIT_WITH_STATUS = 1;
+		opt.flags.exit_with_status = 1;
 		if (!sb)
-			opt.flags.QUICK = 1;
+			opt.flags.quick = 1;
 		do_diff_cache(&head, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {

@@ -1168,7 +1168,7 @@ static int index_has_changes(struct strbuf *sb)
 			strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
 		}
 		diff_flush(&opt);
-		return opt.flags.HAS_CHANGES != 0;
+		return opt.flags.has_changes != 0;
 	} else {
 		for (i = 0; sb && i < active_nr; i++) {
 			if (i)

@@ -1409,8 +1409,8 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
 	rev_info.show_root_diff = 1;
 	rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
 	rev_info.no_commit_id = 1;
-	rev_info.diffopt.flags.BINARY = 1;
-	rev_info.diffopt.flags.FULL_INDEX = 1;
+	rev_info.diffopt.flags.binary = 1;
+	rev_info.diffopt.flags.full_index = 1;
 	rev_info.diffopt.use_color = 0;
 	rev_info.diffopt.file = fp;
 	rev_info.diffopt.close_file = 1;

builtin/blame.c +5 -5

index 79db9e849..005f55aaa 100644
@@ -708,8 +708,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	git_config(git_blame_config, &output_option);
 	init_revisions(&revs, NULL);
 	revs.date_mode = blame_date_mode;
-	revs.diffopt.flags.ALLOW_TEXTCONV = 1;
-	revs.diffopt.flags.FOLLOW_RENAMES = 1;
+	revs.diffopt.flags.allow_textconv = 1;
+	revs.diffopt.flags.follow_renames = 1;
 
 	save_commit_buffer = 0;
 	dashdash_pos = 0;

@@ -734,9 +734,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
-	no_whole_file_rename = !revs.diffopt.flags.FOLLOW_RENAMES;
+	no_whole_file_rename = !revs.diffopt.flags.follow_renames;
 	xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
-	revs.diffopt.flags.FOLLOW_RENAMES = 0;
+	revs.diffopt.flags.follow_renames = 0;
 	argc = parse_options_end(&ctx);
 
 	if (incremental || (output_option & OUTPUT_PORCELAIN)) {

@@ -803,7 +803,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 	blame_date_width -= 1; /* strip the null */
 
-	if (revs.diffopt.flags.FIND_COPIES_HARDER)
+	if (revs.diffopt.flags.find_copies_harder)
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);

builtin/commit.c +2 -2

index 960e7ac08..0f368ad81 100644
@@ -913,10 +913,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			 * be really confusing.
 			 */
 			struct diff_flags flags = DIFF_FLAGS_INIT;
-			flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+			flags.override_submodule_config = 1;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
-				flags.IGNORE_SUBMODULES = 1;
+				flags.ignore_submodules = 1;
 			commitable = index_differs_from(parent, &flags, 1);
 		}
 	}

builtin/diff.c +4 -4

index ed41eb5a5..3476e95e1 100644
@@ -44,7 +44,7 @@ static void stuff_change(struct diff_options *opt,
 	    !oidcmp(old_oid, new_oid) && (old_mode == new_mode))
 		return;
 
-	if (opt->flags.REVERSE_DIFF) {
+	if (opt->flags.reverse_diff) {
 		SWAP(old_mode, new_mode);
 		SWAP(old_oid, new_oid);
 		SWAP(old_path, new_path);

@@ -350,8 +350,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.stat_graph_width = -1;
 
 	/* Default to let external and textconv be used */
-	rev.diffopt.flags.ALLOW_EXTERNAL = 1;
-	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev.diffopt.flags.allow_external = 1;
+	rev.diffopt.flags.allow_textconv = 1;
 
 	if (nongit)
 		die(_("Not a git repository"));

@@ -361,7 +361,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		diff_setup_done(&rev.diffopt);
 	}
 
-	rev.diffopt.flags.RECURSIVE = 1;
+	rev.diffopt.flags.recursive = 1;
 
 	setup_diff_pager(&rev.diffopt);

builtin/fast-export.c +1 -1

index 35c8fb65d..72672665b 100644
@@ -1066,7 +1066,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
 	revs.diffopt.format_callback_data = &paths_of_changed_objects;
-	revs.diffopt.flags.RECURSIVE = 1;
+	revs.diffopt.flags.recursive = 1;
 	while ((commit = get_revision(&revs))) {
 		if (has_unshown_parent(commit)) {
 			add_object_array(&commit->object, NULL, &commits);

builtin/log.c +13 ~~-13~~

index 4a24d3e48..f3d5a02be 100644
@@ -121,16 +121,16 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
 	if (default_follow)
-		rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES = 1;
+		rev->diffopt.flags.default_follow_renames = 1;
 	rev->verbose_header = 1;
-	rev->diffopt.flags.RECURSIVE = 1;
+	rev->diffopt.flags.recursive = 1;
 	rev->diffopt.stat_width = -1; /* use full terminal width */
 	rev->diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
 	rev->abbrev_commit = default_abbrev_commit;
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
-	rev->diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev->diffopt.flags.allow_textconv = 1;
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);

@@ -181,7 +181,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		init_display_notes(&rev->notes_opt);
 
 	if (rev->diffopt.pickaxe || rev->diffopt.filter ||
-	    rev->diffopt.flags.FOLLOW_RENAMES)
+	    rev->diffopt.flags.follow_renames)
 		rev->always_show_header = 0;
 
 	if (source)

@@ -391,7 +391,7 @@ static int cmd_log_walk(struct rev_info *rev)
 		fclose(rev->diffopt.file);
 
 	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
-	    rev->diffopt.flags.CHECK_FAILED) {
+	    rev->diffopt.flags.check_failed) {
 		return 02;
 	}
 	return diff_result_code(&rev->diffopt, 0);

@@ -483,8 +483,8 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	unsigned long size;
 
 	fflush(rev->diffopt.file);
-	if (!rev->diffopt.flags.TEXTCONV_SET_VIA_CMDLINE ||
-	    !rev->diffopt.flags.ALLOW_TEXTCONV)
+	if (!rev->diffopt.flags.textconv_set_via_cmdline ||
+	    !rev->diffopt.flags.allow_textconv)
 		return stream_blob_to_fd(1, oid, NULL, 0);
 
 	if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,

@@ -666,9 +666,9 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 static void log_setup_revisions_tweak(struct rev_info *rev,
 				      struct setup_revision_opt *opt)
 {
-	if (rev->diffopt.flags.DEFAULT_FOLLOW_RENAMES &&
+	if (rev->diffopt.flags.default_follow_renames &&
 	    rev->prune_data.nr == 1)
-		rev->diffopt.flags.FOLLOW_RENAMES = 1;
+		rev->diffopt.flags.follow_renames = 1;
 
 	/* Turn --cc/-c into -p --cc/-c when -p was not given */
 	if (!rev->diffopt.output_format && rev->combine_merges)

@@ -1340,7 +1340,7 @@ static void prepare_bases(struct base_tree_info *bases,
 		return;
 
 	diff_setup(&diffopt);
-	diffopt.flags.RECURSIVE = 1;
+	diffopt.flags.recursive = 1;
 	diff_setup_done(&diffopt);
 
 	oidcpy(&bases->base_commit, &base->object.oid);

@@ -1511,7 +1511,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
-	rev.diffopt.flags.RECURSIVE = 1;
+	rev.diffopt.flags.recursive = 1;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";

@@ -1612,8 +1612,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.zero_commit = zero_commit;
 
-	if (!rev.diffopt.flags.TEXT && !no_binary_diff)
-		rev.diffopt.flags.BINARY = 1;
+	if (!rev.diffopt.flags.text && !no_binary_diff)
+		rev.diffopt.flags.binary = 1;
 
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);

builtin/reset.c +1 -1

index 206819ef1..4b313a018 100644
@@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
-	opt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	opt.flags.override_submodule_config = 1;
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;

builtin/rev-list.c +1 -1

index c62382171..1d3b6b61b 100644
@@ -294,7 +294,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (revs.bisect)
 		bisect_list = 1;
 
-	if (revs.diffopt.flags.QUICK)
+	if (revs.diffopt.flags.quick)
 		info.flags |= REV_LIST_QUIET;
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];

combine-diff.c +5 -5

index 5a3a8b49b..23f3d25e2 100644
@@ -898,7 +898,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 				 int show_file_header)
 {
 	struct diff_options *opt = &rev->diffopt;
-	int abbrev = opt->flags.FULL_INDEX ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
+	int abbrev = opt->flags.full_index ? GIT_SHA1_HEXSZ : DEFAULT_ABBREV;
 	const char *a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
 	const char *b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
 	const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);

@@ -987,7 +987,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	userdiff = userdiff_find_by_path(elem->path);
 	if (!userdiff)
 		userdiff = userdiff_find_by_name("default");
-	if (opt->flags.ALLOW_TEXTCONV)
+	if (opt->flags.allow_textconv)
 		textconv = userdiff_get_textconv(userdiff);
 
 	/* Read the result of merge first */

@@ -1413,8 +1413,8 @@ void diff_tree_combined(const struct object_id *oid,
 
 	diffopts = *opt;
 	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
-	diffopts.flags.RECURSIVE = 1;
-	diffopts.flags.ALLOW_EXTERNAL = 0;
+	diffopts.flags.recursive = 1;
+	diffopts.flags.allow_external = 0;
 
 	/* find set of paths that everybody touches
 	 *

@@ -1435,7 +1435,7 @@ void diff_tree_combined(const struct object_id *oid,
 	 * NOTE please keep this semantically in sync with diffcore_std()
 	 */
 	need_generic_pathscan = opt->skip_stat_unmatch	||
-			opt->flags.FOLLOW_RENAMES	||
+			opt->flags.follow_renames	||
 			opt->break_opt != -1	||
 			opt->detect_rename	||
 			opt->pickaxe		||

diff-lib.c +11 ~~-11~~

index f1cc3fd51..d373358a5 100644
@@ -72,14 +72,14 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct diff_flags orig_flags = diffopt->flags;
-		if (!diffopt->flags.OVERRIDE_SUBMODULE_CONFIG)
+		if (!diffopt->flags.override_submodule_config)
 			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
-		if (diffopt->flags.IGNORE_SUBMODULES)
+		if (diffopt->flags.ignore_submodules)
 			changed = 0;
-		else if (!diffopt->flags.IGNORE_DIRTY_SUBMODULES &&
-			 (!changed || diffopt->flags.DIRTY_SUBMODULES))
+		else if (!diffopt->flags.ignore_dirty_submodules &&
+			 (!changed || diffopt->flags.dirty_submodules))
 			*dirty_submodule = is_submodule_modified(ce->name,
-								 diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES);
+								 diffopt->flags.ignore_untracked_in_submodules);
 		diffopt->flags = orig_flags;
 	}
 	return changed;

@@ -229,7 +229,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 
 		if (!changed && !dirty_submodule) {
 			ce_mark_uptodate(ce);
-			if (!revs->diffopt.flags.FIND_COPIES_HARDER)
+			if (!revs->diffopt.flags.find_copies_harder)
 				continue;
 		}
 		oldmode = ce->ce_mode;

@@ -363,7 +363,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !oidcmp(oid, &old->oid) && !dirty_submodule &&
-	    !revs->diffopt.flags.FIND_COPIES_HARDER)
+	    !revs->diffopt.flags.find_copies_harder)
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,

@@ -494,7 +494,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.head_idx = 1;
 	opts.index_only = cached;
 	opts.diff_index_cached = (cached &&
-				  !revs->diffopt.flags.FIND_COPIES_HARDER);
+				  !revs->diffopt.flags.find_copies_harder);
 	opts.merge = 1;
 	opts.fn = oneway_diff;
 	opts.unpack_data = revs;

@@ -545,12 +545,12 @@ int index_differs_from(const char *def, const struct diff_flags *flags,
 	memset(&opt, 0, sizeof(opt));
 	opt.def = def;
 	setup_revisions(0, NULL, &rev, &opt);
-	rev.diffopt.flags.QUICK = 1;
-	rev.diffopt.flags.EXIT_WITH_STATUS = 1;
+	rev.diffopt.flags.quick = 1;
+	rev.diffopt.flags.exit_with_status = 1;
 	if (flags)
 		diff_flags_or(&rev.diffopt.flags, flags);
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
 	object_array_clear(&rev.pending);
-	return (rev.diffopt.flags.HAS_CHANGES != 0);
+	return (rev.diffopt.flags.has_changes != 0);
 }

diff-no-index.c +4 -4

index a3e194340..0ed5f0f49 100644
@@ -184,7 +184,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (o->flags.REVERSE_DIFF) {
+		if (o->flags.reverse_diff) {
 			SWAP(mode1, mode2);
 			SWAP(name1, name2);
 		}

@@ -276,16 +276,16 @@ void diff_no_index(struct rev_info *revs,
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	revs->diffopt.flags.NO_INDEX = 1;
+	revs->diffopt.flags.no_index = 1;
 
-	revs->diffopt.flags.RELATIVE_NAME = 1;
+	revs->diffopt.flags.relative_name = 1;
 	revs->diffopt.prefix = prefix;
 
 	revs->max_count = -2;
 	diff_setup_done(&revs->diffopt);
 
 	setup_diff_pager(&revs->diffopt);
-	revs->diffopt.flags.EXIT_WITH_STATUS = 1;
+	revs->diffopt.flags.exit_with_status = 1;
 
 	if (queue_diff(&revs->diffopt, paths[0], paths[1]))
 		exit(1);

diff.c +85 ~~-85~~

448 lines not on this page · raw message

diff.h +31 ~~-31~~

68 lines not on this page · raw message

diffcore-pickaxe.c +4 -4

index 6193c07f6..9476bd210 100644
@@ -131,7 +131,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	if (!DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two))
 		return 0;
 
-	if (o->flags.ALLOW_TEXTCONV) {
+	if (o->flags.allow_textconv) {
 		textconv_one = get_textconv(p->one);
 		textconv_two = get_textconv(p->two);
 	}

@@ -222,11 +222,11 @@ void diffcore_pickaxe(struct diff_options *o)
 
 	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
 		int cflags = REG_EXTENDED | REG_NEWLINE;
-		if (o->flags.PICKAXE_IGNORE_CASE)
+		if (o->flags.pickaxe_ignore_case)
 			cflags |= REG_ICASE;
 		regcomp_or_die(&regex, needle, cflags);
 		regexp = &regex;
-	} else if (o->flags.PICKAXE_IGNORE_CASE &&
+	} else if (o->flags.pickaxe_ignore_case &&
 		   has_non_ascii(needle)) {
 		struct strbuf sb = STRBUF_INIT;
 		int cflags = REG_NEWLINE | REG_ICASE;

@@ -236,7 +236,7 @@ void diffcore_pickaxe(struct diff_options *o)
 		strbuf_release(&sb);
 		regexp = &regex;
 	} else {
-		kws = kwsalloc(o->flags.PICKAXE_IGNORE_CASE
+		kws = kwsalloc(o->flags.pickaxe_ignore_case
 			       ? tolower_trans_tbl : NULL);
 		kwsincr(kws, needle, strlen(needle));
 		kwsprep(kws);

diffcore-rename.c +3 -3

index bd077ee11..12dc2a056 100644
@@ -405,7 +405,7 @@ static int too_many_rename_candidates(int num_create,
 		num_src > num_create ? num_src : num_create;
 
 	/* Are we running under -C -C? */
-	if (!options->flags.FIND_COPIES_HARDER)
+	if (!options->flags.find_copies_harder)
 		return 1;
 
 	/* Would we bust the limit if we were running under -C? */

@@ -463,7 +463,7 @@ void diffcore_rename(struct diff_options *options)
 			else if (options->single_follow &&
 				 strcmp(options->single_follow, p->two->path))
 				continue; /* not interested */
-			else if (!options->flags.RENAME_EMPTY &&
+			else if (!options->flags.rename_empty &&
 				 is_empty_blob_oid(&p->two->oid))
 				continue;
 			else if (add_rename_dst(p->two) < 0) {

@@ -473,7 +473,7 @@ void diffcore_rename(struct diff_options *options)
 				goto cleanup;
 			}
 		}
-		else if (!options->flags.RENAME_EMPTY &&
+		else if (!options->flags.rename_empty &&
 			 is_empty_blob_oid(&p->one->oid))
 			continue;
 		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {

log-tree.c +1 -1

index 460bb5498..8dacccc0c 100644
@@ -793,7 +793,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	struct commit_list *parents;
 	struct object_id *oid;
 
-	if (!opt->diff && !opt->diffopt.flags.EXIT_WITH_STATUS)
+	if (!opt->diff && !opt->diffopt.flags.exit_with_status)
 		return 0;
 
 	parse_commit_or_die(commit);

merge-recursive.c +2 -2

index 9752aba4e..f6c03770b 100644
@@ -540,8 +540,8 @@ static struct string_list *get_renames(struct merge_options *o,
 		return renames;
 
 	diff_setup(&opts);
-	opts.flags.RECURSIVE = 1;
-	opts.flags.RENAME_EMPTY = 0;
+	opts.flags.recursive = 1;
+	opts.flags.rename_empty = 0;
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
 			    o->diff_rename_limit >= 0 ? o->diff_rename_limit :

notes-merge.c +2 -2

index b50111cf2..b3d4d44fc 100644
@@ -125,7 +125,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	       oid_to_hex(base), oid_to_hex(remote));
 
 	diff_setup(&opt);
-	opt.flags.RECURSIVE = 1;
+	opt.flags.recursive = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, remote, "", &opt);

@@ -188,7 +188,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	       len, oid_to_hex(base), oid_to_hex(local));
 
 	diff_setup(&opt);
-	opt.flags.RECURSIVE = 1;
+	opt.flags.recursive = 1;
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
 	diff_tree_oid(base, local, "", &opt);

patch-ids.c +1 -1

index 189869e57..8f7c25d5d 100644
@@ -61,7 +61,7 @@ int init_patch_ids(struct patch_ids *ids)
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
 	ids->diffopts.detect_rename = 0;
-	ids->diffopts.flags.RECURSIVE = 1;
+	ids->diffopts.flags.recursive = 1;
 	diff_setup_done(&ids->diffopts);
 	hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
 	return 0;

revision.c +12 ~~-12~~

index 6bb873501..bfde5b0cd 100644
@@ -410,7 +410,7 @@ static void file_add_remove(struct diff_options *options,
 
 	tree_difference |= diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		options->flags.HAS_CHANGES = 1;
+		options->flags.has_changes = 1;
 }
 
 static void file_change(struct diff_options *options,

@@ -422,7 +422,7 @@ static void file_change(struct diff_options *options,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	options->flags.HAS_CHANGES = 1;
+	options->flags.has_changes = 1;
 }
 
 static int rev_compare_tree(struct rev_info *revs,

@@ -455,7 +455,7 @@ static int rev_compare_tree(struct rev_info *revs,
 	}
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.flags.HAS_CHANGES = 0;
+	revs->pruning.flags.has_changes = 0;
 	if (diff_tree_oid(&t1->object.oid, &t2->object.oid, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;

@@ -471,7 +471,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 		return 0;
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.flags.HAS_CHANGES = 0;
+	revs->pruning.flags.has_changes = 0;
 	retval = diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
 
 	return retval >= 0 && (tree_difference == REV_TREE_SAME);

@@ -1403,8 +1403,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	revs->pruning.flags.RECURSIVE = 1;
-	revs->pruning.flags.QUICK = 1;
+	revs->pruning.flags.recursive = 1;
+	revs->pruning.flags.quick = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->sort_order = REV_SORT_IN_GRAPH_ORDER;

@@ -1917,11 +1917,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		die("--unpacked=<packfile> no longer supported.");
 	} else if (!strcmp(arg, "-r")) {
 		revs->diff = 1;
-		revs->diffopt.flags.RECURSIVE = 1;
+		revs->diffopt.flags.recursive = 1;
 	} else if (!strcmp(arg, "-t")) {
 		revs->diff = 1;
-		revs->diffopt.flags.RECURSIVE = 1;
-		revs->diffopt.flags.TREE_IN_RECURSIVE = 1;
+		revs->diffopt.flags.recursive = 1;
+		revs->diffopt.flags.tree_in_recursive = 1;
 	} else if (!strcmp(arg, "-m")) {
 		revs->ignore_merges = 0;
 	} else if (!strcmp(arg, "-c")) {

@@ -2066,7 +2066,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
 		revs->grep_filter.ignore_case = 1;
-		revs->diffopt.flags.PICKAXE_IGNORE_CASE = 1;
+		revs->diffopt.flags.pickaxe_ignore_case = 1;
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
 	} else if (!strcmp(arg, "--perl-regexp") || !strcmp(arg, "-P")) {

@@ -2399,7 +2399,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	/* Pickaxe, diff-filter and rename following need diffs */
 	if (revs->diffopt.pickaxe ||
 	    revs->diffopt.filter ||
-	    revs->diffopt.flags.FOLLOW_RENAMES)
+	    revs->diffopt.flags.follow_renames)
 		revs->diff = 1;
 
 	if (revs->topo_order)

@@ -2408,7 +2408,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->prune_data.nr) {
 		copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!revs->diffopt.flags.FOLLOW_RENAMES)
+		if (!revs->diffopt.flags.follow_renames)
 			revs->prune = 1;
 		if (!revs->full_diff)
 			copy_pathspec(&revs->diffopt.pathspec,

submodule.c +8 -8

index 62a93bb88..9b16adc96 100644
@@ -183,7 +183,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		if (ignore)
 			handle_ignore_submodules_arg(diffopt, ignore);
 		else if (is_gitmodules_unmerged(&the_index))
-			diffopt->flags.IGNORE_SUBMODULES = 1;
+			diffopt->flags.ignore_submodules = 1;
 	}
 }

@@ -402,16 +402,16 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
-	diffopt->flags.IGNORE_SUBMODULES = 0;
-	diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 0;
-	diffopt->flags.IGNORE_DIRTY_SUBMODULES = 0;
+	diffopt->flags.ignore_submodules = 0;
+	diffopt->flags.ignore_untracked_in_submodules = 0;
+	diffopt->flags.ignore_dirty_submodules = 0;
 
 	if (!strcmp(arg, "all"))
-		diffopt->flags.IGNORE_SUBMODULES = 1;
+		diffopt->flags.ignore_submodules = 1;
 	else if (!strcmp(arg, "untracked"))
-		diffopt->flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
+		diffopt->flags.ignore_untracked_in_submodules = 1;
 	else if (!strcmp(arg, "dirty"))
-		diffopt->flags.IGNORE_DIRTY_SUBMODULES = 1;
+		diffopt->flags.ignore_dirty_submodules = 1;
 	else if (strcmp(arg, "none"))
 		die("bad --ignore-submodules argument: %s", arg);
 }

@@ -616,7 +616,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 	argv_array_pushf(&cp.args, "--color=%s", want_color(o->use_color) ?
 			 "always" : "never");
 
-	if (o->flags.REVERSE_DIFF) {
+	if (o->flags.reverse_diff) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
 				 o->b_prefix, path);
 		argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",

tree-diff.c +8 -8

index b996a23bb..fe2e466ac 100644
@@ -212,9 +212,9 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 		mode = 0;
 	}
 
-	if (opt->flags.RECURSIVE && isdir) {
+	if (opt->flags.recursive && isdir) {
 		recurse = 1;
-		emitthis = opt->flags.TREE_IN_RECURSIVE;
+		emitthis = opt->flags.tree_in_recursive;
 	}
 
 	if (emitthis) {

@@ -425,7 +425,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	ttree = fill_tree_descriptor(&t, oid);
 
 	/* Enable recursion indefinitely */
-	opt->pathspec.recursive = opt->flags.RECURSIVE;
+	opt->pathspec.recursive = opt->flags.recursive;
 
 	for (;;) {
 		int imin, cmp;

@@ -484,7 +484,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t = p[imin] */
 		if (cmp == 0) {
 			/* are either pi > p[imin] or diff(t,pi) != ø ? */
-			if (!opt->flags.FIND_COPIES_HARDER) {
+			if (!opt->flags.find_copies_harder) {
 				for (i = 0; i < nparent; ++i) {
 					/* p[i] > p[imin] */
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)

@@ -522,7 +522,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 		/* t > p[imin] */
 		else {
 			/* ∀i pi=p[imin] -> D += "-p[imin]" */
-			if (!opt->flags.FIND_COPIES_HARDER) {
+			if (!opt->flags.find_copies_harder) {
 				for (i = 0; i < nparent; ++i)
 					if (tp[i].entry.mode & S_IFXMIN_NEQ)
 						goto skip_emit_tp;

@@ -608,8 +608,8 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	diff_opts.flags.RECURSIVE = 1;
-	diff_opts.flags.FIND_COPIES_HARDER = 1;
+	diff_opts.flags.recursive = 1;
+	diff_opts.flags.find_copies_harder = 1;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->pathspec.items[0].match;
 	diff_opts.break_opt = opt->break_opt;

@@ -706,7 +706,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 	strbuf_addstr(&base, base_str);
 
 	retval = ll_diff_tree_oid(old_oid, new_oid, &base, opt);
-	if (!*base_str && opt->flags.FOLLOW_RENAMES && diff_might_be_rename())
+	if (!*base_str && opt->flags.follow_renames && diff_might_be_rename())
 		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
 	strbuf_release(&base);

wt-status.c +9 -9

index 59f9f3a0b..4f76e19d3 100644
@@ -559,12 +559,12 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	rev.diffopt.flags.DIRTY_SUBMODULES = 1;
+	rev.diffopt.flags.dirty_submodules = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (!s->show_untracked_files)
-		rev.diffopt.flags.IGNORE_UNTRACKED_IN_SUBMODULES = 1;
+		rev.diffopt.flags.ignore_untracked_in_submodules = 1;
 	if (s->ignore_submodule_arg) {
-		rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+		rev.diffopt.flags.override_submodule_config = 1;
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;

@@ -583,7 +583,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
-	rev.diffopt.flags.OVERRIDE_SUBMODULE_CONFIG = 1;
+	rev.diffopt.flags.override_submodule_config = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 	if (s->ignore_submodule_arg) {
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);

@@ -949,7 +949,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	init_revisions(&rev, NULL);
-	rev.diffopt.flags.ALLOW_TEXTCONV = 1;
+	rev.diffopt.flags.allow_textconv = 1;
 	rev.diffopt.ita_invisible_in_index = 1;
 
 	memset(&opt, 0, sizeof(opt));

@@ -2263,8 +2263,8 @@ int has_unstaged_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
-	rev_info.diffopt.flags.QUICK = 1;
+		rev_info.diffopt.flags.ignore_submodules = 1;
+	rev_info.diffopt.flags.quick = 1;
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_files(&rev_info, 0);
 	return diff_result_code(&rev_info.diffopt, result);

@@ -2283,8 +2283,8 @@ int has_uncommitted_changes(int ignore_submodules)
 
 	init_revisions(&rev_info, NULL);
 	if (ignore_submodules)
-		rev_info.diffopt.flags.IGNORE_SUBMODULES = 1;
-	rev_info.diffopt.flags.QUICK = 1;
+		rev_info.diffopt.flags.ignore_submodules = 1;
+	rev_info.diffopt.flags.quick = 1;
 	add_head_to_pending(&rev_info);
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_index(&rev_info, 1);

Signature · 1 line

2.15.0.403.gc27cc4dac6-goog

Stefan Beller

2017-10-31 21:46 UTC #45 lore raw

In reply to Brandon Williams

Quoted 6 lines · Brandon Williams

Changes in v3:
* Now always pass struct diff_flags by reference and don't return the struct but rather modify the passed in struct.
* Don't clear TEXTCONV_SET_VIA_CMDLINE when --no-textconv is passed
* added additional patches (set out separately before) to remove the macros and change the struct members to lowercase

I have reviewed this version and the functionality introduced looks good to me. My only nits were regarding style and typos.

Thanks,
Stefan

Junio C Hamano

2017-11-01 06:23 UTC #46 lore raw

In reply to Brandon Williams

Quoted 6 lines · Brandon Williams

Changes in v3:
* Now always pass struct diff_flags by reference and don't return the struct but rather modify the passed in struct.
* Don't clear TEXTCONV_SET_VIA_CMDLINE when --no-textconv is passed
* added additional patches (set out separately before) to remove the macros and change the struct members to lowercase

The result looks pleasant overall, although I suspect I'd need some time to get used to them being all explicit bitfields.

Queued.