[PATCH/RFC 0/4] i-t-a entries again...

STALE3632d

9 messages, 3 authors, 2016-09-27 · open the first message on its own page

[PATCH/RFC 0/4] i-t-a entries again...

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:45

The first two patches bring back a patch does fixes "git status" when
i-t-a entries are present (i.e. "git diff" should show new files while
"git diff --cached" should show no changes).

The third commit fixes "git commit" creating empty commits when i-t-a
entries are the only changes compared to HEAD.

The last one brings back the old behavior before 3f6d56d (commit:
ignore intent-to-add entries instead of refusing - 2012-02-07). I
still believe that current behavior has its uses. So instead of
reverting the behavior unconditonally, the user can choose to switch
back to the old behavior via config key.

The last two do not have have proper tests added because I just
created them in the last hours. This is mainly to bring back the old
discussion to see what should be the way forward.

Nguyễn Thái Ngọc Duy (4):
  diff.h: extend "flags" field to 64 bits because we're out of bits
  Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
  commit: don't count i-t-a entries when checking if the new commit is empty
  commit: reinstate commit behavior before 3f6d56d via a config option

 Documentation/diff-options.txt |  6 ++++++
 builtin/commit.c               | 14 ++++++++++----
 cache-tree.c                   | 14 +++++++++++---
 diff-lib.c                     | 18 ++++++++++++++++--
 diff.c                         |  4 +++-
 diff.h                         |  7 ++++---
 t/t2203-add-intent.sh          | 20 ++++++++++++++++++--
 wt-status.c                    |  5 +++++
 8 files changed, 73 insertions(+), 15 deletions(-)

-- 
2.8.2.524.g6ff3d78

[PATCH 1/4] diff.h: extend "flags" field to 64 bits because we're out of bits

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:45

Current flags field is 32-bits, all used except one bit and we need one
more bit is needed for to toggle i-t-a behavior. The 9th bit could be
reused for this, but we could just extend it to 64 bits now to give room
for more future flags.

gcc -Wconversion is used to catch assignments that truncate bits. No new
warning was introduced (in fact one in index_differs_from() was
eliminated),

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/commit.c | 2 +-
 diff-lib.c       | 4 ++--
 diff.c           | 2 +-
 diff.h           | 6 +++---
 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 443ff91..fcfaa2b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -906,7 +906,7 @@ 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;
+			uint64_t diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
 				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
diff --git a/diff-lib.c b/diff-lib.c
index bc49c70..27887d0 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -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;
+		uint64_t 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))
@@ -516,7 +516,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
 	return 0;
 }
 
-int index_differs_from(const char *def, int diff_flags)
+int index_differs_from(const char *def, uint64_t diff_flags)
 {
 	struct rev_info rev;
 	struct setup_revision_opt opt;
diff --git a/diff.c b/diff.c
index d3734d3..f70425f 100644
--- a/diff.c
+++ b/diff.c
@@ -4936,7 +4936,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;
+	uint64_t 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 --git a/diff.h b/diff.h
index 125447b..b497078 100644
--- a/diff.h
+++ b/diff.h
@@ -115,8 +115,8 @@ struct diff_options {
 	const char *pickaxe;
 	const char *single_follow;
 	const char *a_prefix, *b_prefix;
-	unsigned flags;
-	unsigned touched_flags;
+	uint64_t flags;
+	uint64_t touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;
@@ -348,7 +348,7 @@ 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);
+extern int index_differs_from(const char *def, uint64_t diff_flags);
 
 /*
  * Fill the contents of the filespec "df", respecting any textconv defined by
-- 
2.8.2.524.g6ff3d78

[PATCH 3/4] commit: don't count i-t-a entries when checking if the new commit is empty

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:45

i-t-a entries are excluded from tree building. Relying solely on active_nr
(or diff without --shift-ita) may lead to empty commits sometimes, when
i-t-a entries are the only ones "changed" in the index.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/commit.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index fcfaa2b..e98ca8a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		if (amend)
 			parent = "HEAD^1";
 
-		if (get_sha1(parent, sha1))
-			commitable = !!active_nr;
-		else {
+		if (get_sha1(parent, sha1)) {
+			int i, ita_nr = 0;
+
+			for (i = 0; i < active_nr; i++)
+				if (ce_intent_to_add(active_cache[i]))
+					ita_nr++;
+			commitable = active_nr - ita_nr == 0;
+		} else {
 			/*
 			 * Unless the user did explicitly request a submodule
 			 * ignore mode by passing a command line option we do
@@ -910,6 +915,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
 				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
+			diff_flags |= DIFF_OPT_SHIFT_INTENT_TO_ADD;
 			commitable = index_differs_from(parent, diff_flags);
 		}
 	}
-- 
2.8.2.524.g6ff3d78

[PATCH 2/4] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:45

The original commit d95d728aba06a34394d15466045cbdabdada58a2 was
reverted in commit 78cc1a540ba127b13f2f3fd531777b57f3a9cd46 because we
were (and still are) not ready for a new world order. A lot more
investigation must be done to see what is impacted. See the 78cc1a5 for
details.

This patch takes a smaller and safer step. The new behavior is
controlled by SHIFT_INTENT_TO_ADD flag. We can gradually move more diff
users to the new behavior after we are sure it's safe to do so. This
flag is exposed to outside as "--shift-ita".

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/diff-options.txt |  6 ++++++
 diff-lib.c                     | 14 ++++++++++++++
 diff.c                         |  2 ++
 diff.h                         |  1 +
 t/t2203-add-intent.sh          | 20 ++++++++++++++++++--
 wt-status.c                    |  5 +++++
 6 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 3cb3015..904438b 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -559,5 +559,11 @@ endif::git-format-patch[]
 --no-prefix::
 	Do not show any source or destination prefix.
 
+--shift-ita::
+	By default entries added by "git add -N" appear as an existing
+	empty file in "git diff" and a new file in "git diff --cached".
+	This option makes the entry appear as a new file in "git diff"
+	and non-existent in "git diff --cached".
+
 For more detailed explanation on these common options, see also
 linkgit:gitdiffcore[7].
diff --git a/diff-lib.c b/diff-lib.c
index 27887d0..1248970 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -212,6 +212,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 					       ce->sha1, !is_null_sha1(ce->sha1),
 					       ce->name, 0);
 				continue;
+			} else if (DIFF_OPT_TST(&revs->diffopt, SHIFT_INTENT_TO_ADD) &&
+				   ce_intent_to_add(ce)) {
+				diff_addremove(&revs->diffopt, '+', ce->ce_mode,
+					       EMPTY_BLOB_SHA1_BIN, 0,
+					       ce->name, 0);
+				continue;
 			}
 
 			changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
@@ -376,6 +382,14 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 	struct rev_info *revs = o->unpack_data;
 	int match_missing, cached;
 
+	/* i-t-a entries do not actually exist in the index */
+	if (DIFF_OPT_TST(&revs->diffopt, SHIFT_INTENT_TO_ADD) &&
+	    idx && ce_intent_to_add(idx)) {
+		idx = NULL;
+		if (!tree)
+			return;	/* nothing to diff.. */
+	}
+
 	/* if the entry is not checked out, don't examine work tree */
 	cached = o->index_only ||
 		(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));
diff --git a/diff.c b/diff.c
index f70425f..b624e64 100644
--- a/diff.c
+++ b/diff.c
@@ -3910,6 +3910,8 @@ int diff_opt_parse(struct diff_options *options,
 		return parse_submodule_opt(options, arg);
 	else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
 		return parse_ws_error_highlight(options, arg);
+	else if (!strcmp(arg, "--shift-ita"))
+		DIFF_OPT_SET(options, SHIFT_INTENT_TO_ADD);
 
 	/* misc options */
 	else if (!strcmp(arg, "-z"))
diff --git a/diff.h b/diff.h
index b497078..9e42556 100644
--- a/diff.h
+++ b/diff.h
@@ -92,6 +92,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_SHIFT_INTENT_TO_ADD (1UL << 32)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2a4a749..4ae98f4 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -5,10 +5,24 @@ test_description='Intent to add'
 . ./test-lib.sh
 
 test_expect_success 'intent to add' '
+	test_commit 1 &&
+	git rm 1.t &&
+	echo hello >1.t &&
 	echo hello >file &&
 	echo hello >elif &&
 	git add -N file &&
-	git add elif
+	git add elif &&
+	git add -N 1.t
+'
+
+test_expect_success 'git status' '
+	git status --porcelain | grep -v actual >actual &&
+	cat >expect <<-\EOF &&
+	DA 1.t
+	A  elif
+	 A file
+	EOF
+	test_cmp expect actual
 '
 
 test_expect_success 'check result of "add -N"' '
@@ -43,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' '
 	git add -N nitfol &&
 	git commit -m second &&
 	test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
-	test $(git diff --name-only HEAD -- nitfol | wc -l) = 1
+	test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
+	test $(git diff --name-only --shift-ita HEAD -- nitfol | wc -l) = 0 &&
+	test $(git diff --name-only --shift-ita -- nitfol | wc -l) = 1
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
diff --git a/wt-status.c b/wt-status.c
index 4f27bd6..cb89005 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -497,6 +497,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	DIFF_OPT_SET(&rev.diffopt, SHIFT_INTENT_TO_ADD);
 	if (!s->show_untracked_files)
 		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	if (s->ignore_submodule_arg) {
@@ -520,6 +521,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, &opt);
 
 	DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+	DIFF_OPT_SET(&rev.diffopt, SHIFT_INTENT_TO_ADD);
 	if (s->ignore_submodule_arg) {
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	} else {
@@ -555,6 +557,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
 
 		if (!ce_path_match(ce, &s->pathspec, NULL))
 			continue;
+		if (ce_intent_to_add(ce))
+			continue;
 		it = string_list_insert(&s->change, ce->name);
 		d = it->util;
 		if (!d) {
@@ -853,6 +857,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 
 	init_revisions(&rev, NULL);
 	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+	DIFF_OPT_SET(&rev.diffopt, SHIFT_INTENT_TO_ADD);
 
 	memset(&opt, 0, sizeof(opt));
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
-- 
2.8.2.524.g6ff3d78

[PATCH/RFC 4/4] commit: reinstate commit behavior before 3f6d56d via a config option

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:45

There are two groups of users with regards to "git add -N". One uses
i-t-a as a reminder that certain untracked files must be added before
the next commit. Erroring when i-t-a entries are still present
(i.e. real content not added) out gives the user a chance to add real
content.

The other group that sees i-t-a entries as a way to make Git aware of
some untracked paths so that "git diff" and "git status" show them
correctly. If they decide that some content should be staged for the
next commit, they "git add" again for real. If content in these paths is
never added, the paths are simply ignored.

3f6d56d (commit: ignore intent-to-add entries instead of refusing -
2012-02-07) changes the behavior from the former to the latter. This
patch brings bach the former behavior if core.errorCommitIta is true.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 cache-tree.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index ddf0cc9..b87bd3c 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -155,20 +155,28 @@ static int verify_cache(struct cache_entry **cache,
 {
 	int i, funny;
 	int silent = flags & WRITE_TREE_SILENT;
+	int core_error_commit_ita = 0;
+
+	git_config_get_bool("core.errorcommitita", &core_error_commit_ita);
 
 	/* Verify that the tree is merged */
 	funny = 0;
 	for (i = 0; i < entries; i++) {
 		const struct cache_entry *ce = cache[i];
-		if (ce_stage(ce)) {
+		if (ce_stage(ce) ||
+		    (core_error_commit_ita && ce_intent_to_add(ce))) {
 			if (silent)
 				return -1;
 			if (10 < ++funny) {
 				fprintf(stderr, "...\n");
 				break;
 			}
-			fprintf(stderr, "%s: unmerged (%s)\n",
-				ce->name, sha1_to_hex(ce->sha1));
+			if (ce_stage(ce))
+				fprintf(stderr, "%s: unmerged (%s)\n",
+					ce->name, sha1_to_hex(ce->sha1));
+			else
+				fprintf(stderr, "%s: not added yet\n",
+					ce->name);
 		}
 	}
 	if (funny)
-- 
2.8.2.524.g6ff3d78

Re: [PATCH 1/4] diff.h: extend "flags" field to 64 bits because we're out of bits

From: <hidden>
Date: 2016-06-16 02:19:46

Am 06.06.2016 um 13:16 schrieb Nguyễn Thái Ngọc Duy:
Current flags field is 32-bits, all used except one bit and we need one
more bit is needed for to toggle i-t-a behavior. The 9th bit could be
Something's wrong here. Either drop the "is needed" or the "we need".
quoted hunk
reused for this, but we could just extend it to 64 bits now to give room
for more future flags.

gcc -Wconversion is used to catch assignments that truncate bits. No new
warning was introduced (in fact one in index_differs_from() was
eliminated),

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/commit.c | 2 +-
 diff-lib.c       | 4 ++--
 diff.c           | 2 +-
 diff.h           | 6 +++---
 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 443ff91..fcfaa2b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -906,7 +906,7 @@ 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;
+			uint64_t diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
 			if (ignore_submodule_arg &&
 			    !strcmp(ignore_submodule_arg, "all"))
 				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
diff --git a/diff-lib.c b/diff-lib.c
index bc49c70..27887d0 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -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;
+		uint64_t 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))
@@ -516,7 +516,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
 	return 0;
 }
 
-int index_differs_from(const char *def, int diff_flags)
+int index_differs_from(const char *def, uint64_t diff_flags)
 {
 	struct rev_info rev;
 	struct setup_revision_opt opt;
diff --git a/diff.c b/diff.c
index d3734d3..f70425f 100644
--- a/diff.c
+++ b/diff.c
@@ -4936,7 +4936,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;
+	uint64_t 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 --git a/diff.h b/diff.h
index 125447b..b497078 100644
--- a/diff.h
+++ b/diff.h
@@ -115,8 +115,8 @@ struct diff_options {
 	const char *pickaxe;
 	const char *single_follow;
 	const char *a_prefix, *b_prefix;
-	unsigned flags;
-	unsigned touched_flags;
+	uint64_t flags;
+	uint64_t touched_flags;
 
 	/* diff-filter bits */
 	unsigned int filter;
@@ -348,7 +348,7 @@ 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);
+extern int index_differs_from(const char *def, uint64_t diff_flags);
 
 /*
  * Fill the contents of the filespec "df", respecting any textconv defined by
Stefan
-- 
----------------------------------------------------------------
/dev/random says: Make Headlines..use a corduroy pillow....
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

Re: [PATCH 2/4] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Johannes Schindelin <hidden>
Date: 2016-06-16 02:19:49

Hi Duy,

On Mon, 6 Jun 2016, Nguyễn Thái Ngọc Duy wrote:
quoted hunk
diff --git a/diff.h b/diff.h
index b497078..9e42556 100644
--- a/diff.h
+++ b/diff.h
@@ -92,6 +92,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_SHIFT_INTENT_TO_ADD (1UL << 32)
I am afraid that this is not enough. On Windows, sizeof(unsigned long) is
4 (and it is perfectly legal). That means that you would have to use at
least (1ULL << 32), but then you also have to change the type of xdl_opts
to uint64_t, which in turn means that you will have to change the
definition of xpparam_t's "flags" field from unsigned long to uint64_t.

Maybe this can be avoided?

Ciao,
Johannes

Re: [PATCH 2/4] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Duy Nguyen <hidden>
Date: 2016-09-27 10:59:15

(sorry for a very late reply, I'm just picking this series up again)

On Thu, Jun 9, 2016 at 11:18 PM, Johannes Schindelin
[off-list ref] wrote:
Hi Duy,

On Mon, 6 Jun 2016, Nguyễn Thái Ngọc Duy wrote:
quoted
diff --git a/diff.h b/diff.h
index b497078..9e42556 100644
--- a/diff.h
+++ b/diff.h
@@ -92,6 +92,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_SHIFT_INTENT_TO_ADD (1UL << 32)
I am afraid that this is not enough. On Windows, sizeof(unsigned long) is
4 (and it is perfectly legal). That means that you would have to use at
least (1ULL << 32),
OK.
but then you also have to change the type of xdl_opts
to uint64_t, which in turn means that you will have to change the
definition of xpparam_t's "flags" field from unsigned long to uint64_t.
I miss a connection here. This new flag is intended to be used in
"flags" field in struct diff_options. Is there any chance it can be
set on xdl_opts (of the same struct, I assume)?
Maybe this can be avoided?
I don't see a good way to avoid it. We normally enable or disable diff
features as bit flags and now we run out of bits. Adding something
like "flags2" works, but not pretty. Any suggestion is welcome.
-- 
Duy

Re: [PATCH 2/4] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Duy Nguyen <hidden>
Date: 2016-09-27 12:28:24

On Tue, Sep 27, 2016 at 5:58 PM, Duy Nguyen [off-list ref] wrote:
quoted
but then you also have to change the type of xdl_opts
to uint64_t, which in turn means that you will have to change the
definition of xpparam_t's "flags" field from unsigned long to uint64_t.
I miss a connection here. This new flag is intended to be used in
"flags" field in struct diff_options. Is there any chance it can be
set on xdl_opts (of the same struct, I assume)?
quoted
Maybe this can be avoided?
I don't see a good way to avoid it. We normally enable or disable diff
features as bit flags and now we run out of bits. Adding something
like "flags2" works, but not pretty. Any suggestion is welcome.
Never mind. I think I found some way that does not look particularly bad.
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help