{"thread":{"id":"24840","subject":"[PATCH v2] git-merge: ignore space support","startedAt":"2010-08-23T20:59:15Z","lastAt":"2011-01-16T01:08:42Z","messageCount":23,"participants":["Justin Frankel","Jonathan Nieder","Junio C Hamano","Bert Wesarg"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"148783","messageId":"20100823205915.GA4484@ns1.cockos.com","threadId":"24840","inReplyTo":null,"subject":"[PATCH v2] git-merge: ignore space support","fromName":"Justin Frankel","fromEmail":"justin@cockos.com","sentAt":"2010-08-23T20:59:15Z","receivedAt":"2010-08-23T20:59:15Z","isPatch":true,"sender":{"key":"justin@cockos.com","avatar":"https://avatars.githubusercontent.com/u/5803984?v=4"},"body":"git-merge and git-rebase can now be passed -Xignore-space-at-eol,\n-Xignore-space-change, -Xignore-all-space, -Xpatience options.\n\nThis is for the next/ branch (as the ll-merge flag interface has been\ncleaned up). This version of the patch is signed-off and passes git diff \n--check. \n\nSigned-off-by: Justin Frankel \n---\n builtin/merge-recursive.c | 9 +++++++++\n builtin/merge.c | 9 +++++++++\n ll-merge.c | 1 +\n ll-merge.h | 15 ++++++++++++++-\n merge-recursive.c | 2 +-\n merge-recursive.h | 1 +\n 6 files changed, 35 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex c2d4677..7cbc913 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -2,6 +2,7 @@\n #include \"commit.h\"\n #include \"tag.h\"\n #include \"merge-recursive.h\"\n+#include \"xdiff-interface.h\"\n \n static const char *better_branch_name(const char *branch)\n {\n@@ -49,6 +50,14 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\t\t\to.renormalize = 1;\n \t\t\telse if (!strcmp(arg+2, \"no-renormalize\"))\n \t\t\t\to.renormalize = 0;\n+\t\t\telse if (!strcmp(arg+2, \"ignore-all-space\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n+\t\t\telse if (!strcmp(arg+2, \"ignore-space-change\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n+\t\t\telse if (!strcmp(arg+2, \"ignore-space-at-eol\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n+\t\t\telse if (!strcmp(arg+2, \"patience\"))\n+\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4e4ec89..3fe5dc7 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -25,6 +25,7 @@\n #include \"help.h\"\n #include \"merge-recursive.h\"\n #include \"resolve-undo.h\"\n+#include \"xdiff-interface.h\"\n \n #define DEFAULT_TWOHEAD (1<<0)\n #define DEFAULT_OCTOPUS (1<<1)\n@@ -647,6 +648,14 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\t\to.renormalize = 1;\n \t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n \t\t\t\to.renormalize = 0;\n+\t\t\telse if (!strcmp(xopts[x], \"ignore-all-space\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n+\t\t\telse if (!strcmp(xopts[x], \"ignore-space-change\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n+\t\t\telse if (!strcmp(xopts[x], \"ignore-space-at-eol\"))\n+\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n+\t\t\telse if (!strcmp(xopts[x], \"patience\"))\n+\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option for merge-recursive: -X%s\", xopts[x]);\n \t\t}\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 6bb3095..8a0a076 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -80,6 +80,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \tmemset(&xmp, 0, sizeof(xmp));\n \txmp.level = XDL_MERGE_ZEALOUS;\n \txmp.favor = ll_opt_favor(flag);\n+\txmp.xpp.flags = ll_opt_xdl_opt(flag);\n \tif (git_xmerge_style >= 0)\n \t\txmp.style = git_xmerge_style;\n \tif (marker_size > 0)\ndiff --git a/ll-merge.h b/ll-merge.h\nindex ff7ca87..cf2d7b9 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -7,19 +7,32 @@\n \n #define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n #define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n-#define LL_OPT_FAVOR_SHIFT 1\n+#define LL_OPT_FAVOR_SHIFT \t1\n #define LL_OPT_RENORMALIZE\t(1 << 3)\n+#define LL_OPT_XDL_MASK \t(0x3F << 4)\n+#define LL_OPT_XDL_SHIFT \t4\n \n static inline int ll_opt_favor(int flag)\n {\n \treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n }\n \n+static inline int ll_opt_xdl_opt(int flag)\n+{\n+\treturn ((flag & LL_OPT_XDL_MASK) >> LL_OPT_XDL_SHIFT);\n+}\n+\n static inline int create_ll_flag(int favor)\n {\n \treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n }\n \n+static inline int create_ll_flag_xdl_opt(int favor, int xdl_opt)\n+{\n+\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK) |\n+\t\t((xdl_opt << LL_OPT_XDL_SHIFT) & LL_OPT_XDL_MASK);\n+}\n+\n int ll_merge(mmbuffer_t *result_buf,\n \t const char *path,\n \t mmfile_t *ancestor, const char *ancestor_label,\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex aadd48c..506c9db 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -646,7 +646,7 @@ static int merge_3way(struct merge_options *o,\n \t\t\t\t&src1, name1, &src2, name2,\n \t\t\t\t((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) |\n \t\t\t\t (o->renormalize ? LL_OPT_RENORMALIZE : 0) |\n-\t\t\t\t create_ll_flag(favor)));\n+\t\t\t\t create_ll_flag_xdl_opt(favor,o->xdl_opts)));\n \n \tfree(name1);\n \tfree(name2);\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 196f053..47f718f 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -13,6 +13,7 @@ struct merge_options {\n \t\tMERGE_RECURSIVE_THEIRS\n \t} recursive_variant;\n \tconst char *subtree_shift;\n+\tint xdl_opts;\n \tunsigned buffer_output : 1;\n \tunsigned renormalize : 1;\n \tint verbosity;\n-- \n1.7.0.2.msysgit.0\n"},{"id":"148804","messageId":"20100824022820.GE17406@burratino","threadId":"24840","inReplyTo":"20100823205915.GA4484@ns1.cockos.com","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-24T02:28:20Z","receivedAt":"2010-08-24T02:28:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Justin Frankel wrote:\n\n> This is for the next/ branch\n\nApplies on top of jn/merge-renormalize. Thanks for the reroll.\n\n> --- a/builtin/merge-recursive.c\n> +++ b/builtin/merge-recursive.c\n> @@ -49,6 +50,14 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n> \t\t\t\to.renormalize = 1;\n> \t\t\telse if (!strcmp(arg+2, \"no-renormalize\"))\n> \t\t\t\to.renormalize = 0;\n> +\t\t\telse if (!strcmp(arg+2, \"ignore-all-space\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n> +\t\t\telse if (!strcmp(arg+2, \"ignore-space-change\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n> +\t\t\telse if (!strcmp(arg+2, \"ignore-space-at-eol\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n> +\t\t\telse if (!strcmp(arg+2, \"patience\"))\n> +\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n\nSo this teaches merge-recursive and ll-merge to accept the xdiff\nwhitespace flags (plus \"patience\").\n\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -647,6 +648,14 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n> \t\t\t\to.renormalize = 1;\n> \t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n> \t\t\t\to.renormalize = 0;\n> +\t\t\telse if (!strcmp(xopts[x], \"ignore-all-space\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n> +\t\t\telse if (!strcmp(xopts[x], \"ignore-space-change\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n> +\t\t\telse if (!strcmp(xopts[x], \"ignore-space-at-eol\"))\n> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n> +\t\t\telse if (!strcmp(xopts[x], \"patience\"))\n> +\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n\nIt's tempting to fix this code duplication once and for all.\n\n> +++ b/ll-merge.h\n> @@ -7,19 +7,32 @@\n> \n> #define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n> #define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n> -#define LL_OPT_FAVOR_SHIFT 1\n> +#define LL_OPT_FAVOR_SHIFT \t1\n> #define LL_OPT_RENORMALIZE\t(1 << 3)\n> +#define LL_OPT_XDL_MASK \t(0x3F << 4)\n> +#define LL_OPT_XDL_SHIFT \t4\n\nThe space-instead-of-tab was intentional: the idea is that this is a\ntable:\n\n\tname\t\t\tbits\n\tVIRTUAL_ANCESTOR\t0\n\tFAVOR_MASK\t\t1, 2\n\tFAVOR_SHIFT = 1\n\tRENORMALIZE\t\t3\n\tWHITESPACE_MASK\t\t4, 5, 6\n\tWHITESPACE_SHIFT = 4\n\tPATIENCE\t\t7\n\nand since a shift is not a bitmask it does not deserve an entry in the\ntable.\n\nThe flag layout worries me a bit because presumably the next extension\nwould get bit 8, and so on. What happens if xdiff learns another\nwhitespace-ignoring mode?\n\n> \n> static inline int ll_opt_favor(int flag)\n> {\n> \treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n> }\n> \n> +static inline int ll_opt_xdl_opt(int flag)\n> +{\n> +\treturn ((flag & LL_OPT_XDL_MASK) >> LL_OPT_XDL_SHIFT);\n> +}\n\nStyle nit: parentheses around a return value are not needed.\n\n> static inline int create_ll_flag(int favor)\n> {\n> \treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n> }\n\nLooks like I am guilty of the same thing.\n \n> +static inline int create_ll_flag_xdl_opt(int favor, int xdl_opt)\n> +{\n> +\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK) |\n> +\t\t((xdl_opt << LL_OPT_XDL_SHIFT) & LL_OPT_XDL_MASK);\n> +}\n\nThis function (like its cousin create_ll_flag) is ugly, as Bert\nnoticed before. I am starting to wonder if it would not be simpler\nto use\n\n extern int ll_merge(..., const struct ll_merge_options *opts);\n\ntreating NULL as a request for the default options.\n\nIn other words, how about something like this?\n\n-- 8< --\nSubject: ll-merge: replace flag argument with options struct\n\nKeeping track of the flag bits is proving more trouble than it's\nworth. Instead, use a pointer to an options struct like most similar\nAPIs do.\n\nCallers with no special requests can pass NULL to request the default\noptions.\n\nCc: Bert Wesarg \nSigned-off-by: Jonathan Nieder \n---\n builtin/checkout.c | 2 +-\n ll-merge.c | 42 ++++++++++++++++++++++++++----------------\n ll-merge.h | 21 ++++++---------------\n merge-file.c | 2 +-\n merge-recursive.c | 22 +++++++++++-----------\n rerere.c | 2 +-\n 6 files changed, 46 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 24b67d5..4d36b28 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -155,7 +155,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \t * merge.renormalize set, too\n \t */\n \tstatus = ll_merge(&result_buf, path, &ancestor, \"base\",\n-\t\t\t &ours, \"ours\", &theirs, \"theirs\", 0);\n+\t\t\t &ours, \"ours\", &theirs, \"theirs\", NULL);\n \tfree(ancestor.ptr);\n \tfree(ours.ptr);\n \tfree(theirs.ptr);\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 6bb3095..98f353a 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -18,7 +18,7 @@ typedef int (*ll_merge_fn)(const struct ll_merge_driver *,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag,\n+\t\t\t const struct ll_merge_options *opts,\n \t\t\t int marker_size);\n \n struct ll_merge_driver {\n@@ -39,14 +39,15 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag, int marker_size)\n+\t\t\t const struct ll_merge_options *opts,\n+\t\t\t int marker_size)\n {\n \t/*\n \t * The tentative merge result is \"ours\" for the final round,\n \t * or common ancestor for an internal merge. Still return\n \t * \"conflicted merge\" status.\n \t */\n-\tmmfile_t *stolen = (flag & LL_OPT_VIRTUAL_ANCESTOR) ? orig : src1;\n+\tmmfile_t *stolen = opts->virtual_ancestor ? orig : src1;\n \n \tresult->ptr = stolen->ptr;\n \tresult->size = stolen->size;\n@@ -60,7 +61,8 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \t\t\tmmfile_t *orig, const char *orig_name,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n-\t\t\tint flag, int marker_size)\n+\t\t\tconst struct ll_merge_options *opts,\n+\t\t\tint marker_size)\n {\n \txmparam_t xmp;\n \n@@ -74,12 +76,12 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t\t orig, orig_name,\n \t\t\t\t src1, name1,\n \t\t\t\t src2, name2,\n-\t\t\t\t flag, marker_size);\n+\t\t\t\t opts, marker_size);\n \t}\n \n \tmemset(&xmp, 0, sizeof(xmp));\n \txmp.level = XDL_MERGE_ZEALOUS;\n-\txmp.favor = ll_opt_favor(flag);\n+\txmp.favor = opts->variant;\n \tif (git_xmerge_style >= 0)\n \t\txmp.style = git_xmerge_style;\n \tif (marker_size > 0)\n@@ -96,14 +98,15 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag, int marker_size)\n+\t\t\t const struct ll_merge_options *opts,\n+\t\t\t int marker_size)\n {\n \t/* Use union favor */\n-\tflag &= ~LL_OPT_FAVOR_MASK;\n-\tflag |= create_ll_flag(XDL_MERGE_FAVOR_UNION);\n+\tstruct ll_merge_options o = *opts;\n+\to.variant = XDL_MERGE_FAVOR_UNION;\n \treturn ll_xdl_merge(drv_unused, result, path_unused,\n \t\t\t orig, NULL, src1, NULL, src2, NULL,\n-\t\t\t flag, marker_size);\n+\t\t\t &o, marker_size);\n \treturn 0;\n }\n \n@@ -136,7 +139,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \t\t\tmmfile_t *orig, const char *orig_name,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n-\t\t\tint flag, int marker_size)\n+\t\t\tconst struct ll_merge_options *opts,\n+\t\t\tint marker_size)\n {\n \tchar temp[4][50];\n \tstruct strbuf cmd = STRBUF_INIT;\n@@ -337,15 +341,21 @@ int ll_merge(mmbuffer_t *result_buf,\n \t mmfile_t *ancestor, const char *ancestor_label,\n \t mmfile_t *ours, const char *our_label,\n \t mmfile_t *theirs, const char *their_label,\n-\t int flag)\n+\t const struct ll_merge_options *opts)\n {\n \tstatic struct git_attr_check check[2];\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n \tconst struct ll_merge_driver *driver;\n-\tint virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR;\n \n-\tif (flag & LL_OPT_RENORMALIZE) {\n+\tif (!opts) {\n+\t\tstruct ll_merge_options default_opts = {0};\n+\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n+\t\t\t\tours, our_label, theirs, their_label,\n+\t\t\t\t&default_opts);\n+\t}\n+\n+\tif (opts->renormalize) {\n \t\tnormalize_file(ancestor, path);\n \t\tnormalize_file(ours, path);\n \t\tnormalize_file(theirs, path);\n@@ -359,11 +369,11 @@ int ll_merge(mmbuffer_t *result_buf,\n \t\t}\n \t}\n \tdriver = find_ll_merge_driver(ll_driver_name);\n-\tif (virtual_ancestor && driver->recursive)\n+\tif (opts->virtual_ancestor && driver->recursive)\n \t\tdriver = find_ll_merge_driver(driver->recursive);\n \treturn driver->fn(driver, result_buf, path, ancestor, ancestor_label,\n \t\t\t ours, our_label, theirs, their_label,\n-\t\t\t flag, marker_size);\n+\t\t\t opts, marker_size);\n }\n \n int ll_merge_marker_size(const char *path)\ndiff --git a/ll-merge.h b/ll-merge.h\nindex ff7ca87..4b707f0 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -5,27 +5,18 @@\n #ifndef LL_MERGE_H\n #define LL_MERGE_H\n \n-#define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n-#define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n-#define LL_OPT_FAVOR_SHIFT 1\n-#define LL_OPT_RENORMALIZE\t(1 << 3)\n-\n-static inline int ll_opt_favor(int flag)\n-{\n-\treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n-}\n-\n-static inline int create_ll_flag(int favor)\n-{\n-\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n-}\n+struct ll_merge_options {\n+\tunsigned virtual_ancestor : 1;\n+\tunsigned variant : 2;\t/* favor ours, favor theirs, or union merge */\n+\tunsigned renormalize : 1;\n+};\n \n int ll_merge(mmbuffer_t *result_buf,\n \t const char *path,\n \t mmfile_t *ancestor, const char *ancestor_label,\n \t mmfile_t *ours, const char *our_label,\n \t mmfile_t *theirs, const char *their_label,\n-\t int flag);\n+\t const struct ll_merge_options *opts);\n \n int ll_merge_marker_size(const char *path);\n \ndiff --git a/merge-file.c b/merge-file.c\nindex db4d0d5..f7f4533 100644\n--- a/merge-file.c\n+++ b/merge-file.c\n@@ -37,7 +37,7 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our\n \t * common ancestor.\n \t */\n \tmerge_status = ll_merge(&res, path, base, NULL,\n-\t\t\t\tour, \".our\", their, \".their\", 0);\n+\t\t\t\tour, \".our\", their, \".their\", NULL);\n \tif (merge_status < 0)\n \t\treturn NULL;\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 762b549..ed09785 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -608,22 +608,25 @@ static int merge_3way(struct merge_options *o,\n \t\t const char *branch2)\n {\n \tmmfile_t orig, src1, src2;\n+\tstruct ll_merge_options ll_opts = {0};\n \tchar *base_name, *name1, *name2;\n \tint merge_status;\n-\tint favor;\n \n-\tif (o->call_depth)\n-\t\tfavor = 0;\n-\telse {\n+\tll_opts.renormalize = o->renormalize;\n+\n+\tif (o->call_depth) {\n+\t\tll_opts.virtual_ancestor = 1;\n+\t\tll_opts.variant = 0;\n+\t} else {\n \t\tswitch (o->recursive_variant) {\n \t\tcase MERGE_RECURSIVE_OURS:\n-\t\t\tfavor = XDL_MERGE_FAVOR_OURS;\n+\t\t\tll_opts.variant = XDL_MERGE_FAVOR_OURS;\n \t\t\tbreak;\n \t\tcase MERGE_RECURSIVE_THEIRS:\n-\t\t\tfavor = XDL_MERGE_FAVOR_THEIRS;\n+\t\t\tll_opts.variant = XDL_MERGE_FAVOR_THEIRS;\n \t\t\tbreak;\n \t\tdefault:\n-\t\t\tfavor = 0;\n+\t\t\tll_opts.variant = 0;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -646,10 +649,7 @@ static int merge_3way(struct merge_options *o,\n \tread_mmblob(&src2, b->sha1);\n \n \tmerge_status = ll_merge(result_buf, a->path, &orig, base_name,\n-\t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\t((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) |\n-\t\t\t\t (o->renormalize ? LL_OPT_RENORMALIZE : 0) |\n-\t\t\t\t create_ll_flag(favor)));\n+\t\t\t\t&src1, name1, &src2, name2, &ll_opts);\n \n \tfree(name1);\n \tfree(name2);\ndiff --git a/rerere.c b/rerere.c\nindex e40af0d..b180218 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -325,7 +325,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \t */\n \tll_merge(&result, path, &mmfile[0], NULL,\n \t\t &mmfile[1], \"ours\",\n-\t\t &mmfile[2], \"theirs\", 0);\n+\t\t &mmfile[2], \"theirs\", NULL);\n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfile[i].ptr);\n \n-- \n1.7.2.2\n"},{"id":"148807","messageId":"20100824033922.GA19628@burratino","threadId":"24840","inReplyTo":"20100824022820.GE17406@burratino","subject":"[RFC/PATCH jn/merge-renormalize] merge-recursive: expose merge options for builtin merge","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-24T03:39:22Z","receivedAt":"2010-08-24T03:39:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"There are two very similar blocks of code that recognize options for\nthe \"recursive\" merge strategy. Unify them.\n\nNo functional change intended.\n\nCc: Avery Pennarun \nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n> Justin Frankel wrote:\n\n>> --- a/builtin/merge.c\n>> +++ b/builtin/merge.c\n>> @@ -647,6 +648,14 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n>> \t\t\t\to.renormalize = 1;\n>> \t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n>> \t\t\t\to.renormalize = 0;\n>> +\t\t\telse if (!strcmp(xopts[x], \"ignore-all-space\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n[...]\n> It's tempting to fix this code duplication once and for all.\n\n builtin/merge-recursive.c | 14 +-------------\n builtin/merge.c | 20 ++------------------\n merge-recursive.c | 21 +++++++++++++++++++++\n merge-recursive.h | 2 ++\n 4 files changed, 26 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex c2d4677..a610b68 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -37,19 +37,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--\")) {\n \t\t\tif (!arg[2])\n \t\t\t\tbreak;\n-\t\t\tif (!strcmp(arg+2, \"ours\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(arg+2, \"theirs\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(arg+2, \"subtree\"))\n-\t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(arg+2, \"subtree=\"))\n-\t\t\t\to.subtree_shift = arg + 10;\n-\t\t\telse if (!strcmp(arg+2, \"renormalize\"))\n-\t\t\t\to.renormalize = 1;\n-\t\t\telse if (!strcmp(arg+2, \"no-renormalize\"))\n-\t\t\t\to.renormalize = 0;\n-\t\t\telse\n+\t\t\tif (parse_merge_opt(&o, arg + 2) <= 0)\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 037cd47..8dd81bf 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -629,25 +629,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \n \t\to.renormalize = option_renormalize;\n \n-\t\t/*\n-\t\t * NEEDSWORK: merge with table in builtin/merge-recursive\n-\t\t */\n-\t\tfor (x = 0; x < xopts_nr; x++) {\n-\t\t\tif (!strcmp(xopts[x], \"ours\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(xopts[x], \"theirs\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(xopts[x], \"subtree\"))\n-\t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(xopts[x], \"subtree=\"))\n-\t\t\t\to.subtree_shift = xopts[x]+8;\n-\t\t\telse if (!strcmp(xopts[x], \"renormalize\"))\n-\t\t\t\to.renormalize = 1;\n-\t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n-\t\t\t\to.renormalize = 0;\n-\t\t\telse\n+\t\tfor (x = 0; x < xopts_nr; x++)\n+\t\t\tif (parse_merge_opt(&o, xopts[x]) <= 0)\n \t\t\t\tdie(\"Unknown option for merge-recursive: -X%s\", xopts[x]);\n-\t\t}\n \n \t\to.branch1 = head_arg;\n \t\to.branch2 = remoteheads->item->util;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex ee52581..063d623 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1499,3 +1499,24 @@ void init_merge_options(struct merge_options *o)\n \tmemset(&o->current_directory_set, 0, sizeof(struct string_list));\n \to->current_directory_set.strdup_strings = 1;\n }\n+\n+int parse_merge_opt(struct merge_options *o, const char *s)\n+{\n+\tif (!s || !*s)\n+\t\treturn 0;\n+\tif (!strcmp(s, \"ours\"))\n+\t\to->recursive_variant = MERGE_RECURSIVE_OURS;\n+\telse if (!strcmp(s, \"theirs\"))\n+\t\to->recursive_variant = MERGE_RECURSIVE_THEIRS;\n+\telse if (!strcmp(s, \"subtree\"))\n+\t\to->subtree_shift = \"\";\n+\telse if (!prefixcmp(s, \"subtree=\"))\n+\t\to->subtree_shift = s + strlen(\"subtree=\");\n+\telse if (!strcmp(s, \"renormalize\"))\n+\t\to->renormalize = 1;\n+\telse if (!strcmp(s, \"no-renormalize\"))\n+\t\to->renormalize = 0;\n+\telse\n+\t\treturn 0;\n+\treturn 1;\n+}\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex c5fbe79..37ff99a 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -55,6 +55,8 @@ int merge_recursive_generic(struct merge_options *o,\n void init_merge_options(struct merge_options *o);\n struct tree *write_tree_from_memory(struct merge_options *o);\n \n+int parse_merge_opt(struct merge_options *out, const char *s);\n+\n /* builtin/merge.c */\n int try_merge_command(const char *strategy, struct commit_list *common, const char *head_arg, struct commit_list *remotes);\n \n-- \n1.7.2.2\n"},{"id":"148810","messageId":"5F681E95-9369-490A-84FE-55B99EA8FE63@cockos.com","threadId":"24840","inReplyTo":"20100824022820.GE17406@burratino","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Justin Frankel","fromEmail":"justin@cockos.com","sentAt":"2010-08-24T04:30:04Z","receivedAt":"2010-08-24T04:30:04Z","isPatch":true,"sender":{"key":"justin@cockos.com","avatar":"https://avatars.githubusercontent.com/u/5803984?v=4"},"body":"On Aug 23, 2010, at 7:28 PM, Jonathan Nieder wrote:\n\n> Justin Frankel wrote:\n>\n>> This is for the next/ branch\n>\n> Applies on top of jn/merge-renormalize. Thanks for the reroll.\n>\n>> --- a/builtin/merge-recursive.c\n>> +++ b/builtin/merge-recursive.c\n>> @@ -49,6 +50,14 @@ int cmd_merge_recursive(int argc, const char \n>> **argv, const char *prefix)\n>> \t\t\t\to.renormalize = 1;\n>> \t\t\telse if (!strcmp(arg+2, \"no-renormalize\"))\n>> \t\t\t\to.renormalize = 0;\n>> +\t\t\telse if (!strcmp(arg+2, \"ignore-all-space\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n>> +\t\t\telse if (!strcmp(arg+2, \"ignore-space-change\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n>> +\t\t\telse if (!strcmp(arg+2, \"ignore-space-at-eol\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n>> +\t\t\telse if (!strcmp(arg+2, \"patience\"))\n>> +\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n>\n> So this teaches merge-recursive and ll-merge to accept the xdiff\n> whitespace flags (plus \"patience\").\n>\n>> --- a/builtin/merge.c\n>> +++ b/builtin/merge.c\n>> @@ -647,6 +648,14 @@ static int try_merge_strategy(const char \n>> *strategy, struct commit_list *common,\n>> \t\t\t\to.renormalize = 1;\n>> \t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n>> \t\t\t\to.renormalize = 0;\n>> +\t\t\telse if (!strcmp(xopts[x], \"ignore-all-space\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE;\n>> +\t\t\telse if (!strcmp(xopts[x], \"ignore-space-change\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n>> +\t\t\telse if (!strcmp(xopts[x], \"ignore-space-at-eol\"))\n>> +\t\t\t\to.xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n>> +\t\t\telse if (!strcmp(xopts[x], \"patience\"))\n>> +\t\t\t\to.xdl_opts |= XDF_PATIENCE_DIFF;\n>\n> It's tempting to fix this code duplication once and for all.\n>\nI was tempted to do that, too, but felt I was out of my element.\n\n>> +++ b/ll-merge.h\n>> @@ -7,19 +7,32 @@\n>>\n>> #define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n>> #define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n>> -#define LL_OPT_FAVOR_SHIFT 1\n>> +#define LL_OPT_FAVOR_SHIFT \t1\n>> #define LL_OPT_RENORMALIZE\t(1 << 3)\n>> +#define LL_OPT_XDL_MASK \t(0x3F << 4)\n>> +#define LL_OPT_XDL_SHIFT \t4\n>\n> The space-instead-of-tab was intentional: the idea is that this is a\n> table:\n>\n> \tname\t\t\tbits\n> \tVIRTUAL_ANCESTOR\t0\n> \tFAVOR_MASK\t\t1, 2\n> \tFAVOR_SHIFT = 1\n> \tRENORMALIZE\t\t3\n> \tWHITESPACE_MASK\t\t4, 5, 6\n> \tWHITESPACE_SHIFT = 4\n> \tPATIENCE\t\t7\n>\n> and since a shift is not a bitmask it does not deserve an entry in the\n> table.\n>\nAh.. this gets a bit tedious if you have a 5 bit flag though, no? And \nsince the XDL options are all defined internally by xdiff, perhaps it \nmakes sense to abstract out whitespace/patience, and just pass that \nflag around as an opaque XDL-flag.\n\n> The flag layout worries me a bit because presumably the next extension\n> would get bit 8, and so on. What happens if xdiff learns another\n> whitespace-ignoring mode?\n\nHow about adding a define in xdiff.h for XDF_DIFF_FLAG_BITS , which \ndefines the number of bits relevant, build the mask/bits based on \nthis... the nice thing is that all of the bits are defined in one \nplace, so no other code would need to be modified. Having said that, \nthe solution you pasted below (struct instead of flag) is probably the \ncleanest so this paragraph is moot. Sorry for wasting the bits.\n\n>\n>>\n>> static inline int ll_opt_favor(int flag)\n>> {\n>> \treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n>> }\n>>\n>> +static inline int ll_opt_xdl_opt(int flag)\n>> +{\n>> +\treturn ((flag & LL_OPT_XDL_MASK) >> LL_OPT_XDL_SHIFT);\n>> +}\n>\n> Style nit: parentheses around a return value are not needed.\n\nYeah, I was imitating the other function ..\n\n>\n>> static inline int create_ll_flag(int favor)\n>> {\n>> \treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n>> }\n>\n> Looks like I am guilty of the same thing.\n>\n>> +static inline int create_ll_flag_xdl_opt(int favor, int xdl_opt)\n>> +{\n>> +\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK) |\n>> +\t\t((xdl_opt << LL_OPT_XDL_SHIFT) & LL_OPT_XDL_MASK);\n>> +}\n>\n> This function (like its cousin create_ll_flag) is ugly, as Bert\n> noticed before. I am starting to wonder if it would not be simpler\n> to use\n>\n> extern int ll_merge(..., const struct ll_merge_options *opts);\n>\n> treating NULL as a request for the default options.\n>\n> In other words, how about something like this?\n>\nFine by me... (ok if I were to nitpick I would probably make most of \nthe internal static functions check that opts was non-NULL before \ndereferencing, in case the calling code ever changed, but I'm the noob).\n\nThanks for looking at my patch ;)\n\n-Justin\n\n> -- 8< --\n> Subject: ll-merge: replace flag argument with options struct\n>\n> Keeping track of the flag bits is proving more trouble than it's\n> worth. Instead, use a pointer to an options struct like most similar\n> APIs do.\n>\n> Callers with no special requests can pass NULL to request the default\n> options.\n>\n> Cc: Bert Wesarg \n> Signed-off-by: Jonathan Nieder \n> ---\n> builtin/checkout.c | 2 +-\n> ll-merge.c | 42 ++++++++++++++++++++++++++----------------\n> ll-merge.h | 21 ++++++---------------\n> merge-file.c | 2 +-\n> merge-recursive.c | 22 +++++++++++-----------\n> rerere.c | 2 +-\n> 6 files changed, 46 insertions(+), 45 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 24b67d5..4d36b28 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -155,7 +155,7 @@ static int checkout_merged(int pos, struct \n> checkout *state)\n> \t * merge.renormalize set, too\n> \t */\n> \tstatus = ll_merge(&result_buf, path, &ancestor, \"base\",\n> -\t\t\t &ours, \"ours\", &theirs, \"theirs\", 0);\n> +\t\t\t &ours, \"ours\", &theirs, \"theirs\", NULL);\n> \tfree(ancestor.ptr);\n> \tfree(ours.ptr);\n> \tfree(theirs.ptr);\n> diff --git a/ll-merge.c b/ll-merge.c\n> index 6bb3095..98f353a 100644\n> --- a/ll-merge.c\n> +++ b/ll-merge.c\n> @@ -18,7 +18,7 @@ typedef int (*ll_merge_fn)(const struct \n> ll_merge_driver *,\n> \t\t\t mmfile_t *orig, const char *orig_name,\n> \t\t\t mmfile_t *src1, const char *name1,\n> \t\t\t mmfile_t *src2, const char *name2,\n> -\t\t\t int flag,\n> +\t\t\t const struct ll_merge_options *opts,\n> \t\t\t int marker_size);\n>\n> struct ll_merge_driver {\n> @@ -39,14 +39,15 @@ static int ll_binary_merge(const struct \n> ll_merge_driver *drv_unused,\n> \t\t\t mmfile_t *orig, const char *orig_name,\n> \t\t\t mmfile_t *src1, const char *name1,\n> \t\t\t mmfile_t *src2, const char *name2,\n> -\t\t\t int flag, int marker_size)\n> +\t\t\t const struct ll_merge_options *opts,\n> +\t\t\t int marker_size)\n> {\n> \t/*\n> \t * The tentative merge result is \"ours\" for the final round,\n> \t * or common ancestor for an internal merge. Still return\n> \t * \"conflicted merge\" status.\n> \t */\n> -\tmmfile_t *stolen = (flag & LL_OPT_VIRTUAL_ANCESTOR) ? orig : src1;\n> +\tmmfile_t *stolen = opts->virtual_ancestor ? orig : src1;\n>\n> \tresult->ptr = stolen->ptr;\n> \tresult->size = stolen->size;\n> @@ -60,7 +61,8 @@ static int ll_xdl_merge(const struct \n> ll_merge_driver *drv_unused,\n> \t\t\tmmfile_t *orig, const char *orig_name,\n> \t\t\tmmfile_t *src1, const char *name1,\n> \t\t\tmmfile_t *src2, const char *name2,\n> -\t\t\tint flag, int marker_size)\n> +\t\t\tconst struct ll_merge_options *opts,\n> +\t\t\tint marker_size)\n> {\n> \txmparam_t xmp;\n>\n> @@ -74,12 +76,12 @@ static int ll_xdl_merge(const struct \n> ll_merge_driver *drv_unused,\n> \t\t\t\t orig, orig_name,\n> \t\t\t\t src1, name1,\n> \t\t\t\t src2, name2,\n> -\t\t\t\t flag, marker_size);\n> +\t\t\t\t opts, marker_size);\n> \t}\n>\n> \tmemset(&xmp, 0, sizeof(xmp));\n> \txmp.level = XDL_MERGE_ZEALOUS;\n> -\txmp.favor = ll_opt_favor(flag);\n> +\txmp.favor = opts->variant;\n> \tif (git_xmerge_style >= 0)\n> \t\txmp.style = git_xmerge_style;\n> \tif (marker_size > 0)\n> @@ -96,14 +98,15 @@ static int ll_union_merge(const struct \n> ll_merge_driver *drv_unused,\n> \t\t\t mmfile_t *orig, const char *orig_name,\n> \t\t\t mmfile_t *src1, const char *name1,\n> \t\t\t mmfile_t *src2, const char *name2,\n> -\t\t\t int flag, int marker_size)\n> +\t\t\t const struct ll_merge_options *opts,\n> +\t\t\t int marker_size)\n> {\n> \t/* Use union favor */\n> -\tflag &= ~LL_OPT_FAVOR_MASK;\n> -\tflag |= create_ll_flag(XDL_MERGE_FAVOR_UNION);\n> +\tstruct ll_merge_options o = *opts;\n> +\to.variant = XDL_MERGE_FAVOR_UNION;\n> \treturn ll_xdl_merge(drv_unused, result, path_unused,\n> \t\t\t orig, NULL, src1, NULL, src2, NULL,\n> -\t\t\t flag, marker_size);\n> +\t\t\t &o, marker_size);\n> \treturn 0;\n> }\n>\n> @@ -136,7 +139,8 @@ static int ll_ext_merge(const struct \n> ll_merge_driver *fn,\n> \t\t\tmmfile_t *orig, const char *orig_name,\n> \t\t\tmmfile_t *src1, const char *name1,\n> \t\t\tmmfile_t *src2, const char *name2,\n> -\t\t\tint flag, int marker_size)\n> +\t\t\tconst struct ll_merge_options *opts,\n> +\t\t\tint marker_size)\n> {\n> \tchar temp[4][50];\n> \tstruct strbuf cmd = STRBUF_INIT;\n> @@ -337,15 +341,21 @@ int ll_merge(mmbuffer_t *result_buf,\n> \t mmfile_t *ancestor, const char *ancestor_label,\n> \t mmfile_t *ours, const char *our_label,\n> \t mmfile_t *theirs, const char *their_label,\n> -\t int flag)\n> +\t const struct ll_merge_options *opts)\n> {\n> \tstatic struct git_attr_check check[2];\n> \tconst char *ll_driver_name = NULL;\n> \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n> \tconst struct ll_merge_driver *driver;\n> -\tint virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR;\n>\n> -\tif (flag & LL_OPT_RENORMALIZE) {\n> +\tif (!opts) {\n> +\t\tstruct ll_merge_options default_opts = {0};\n> +\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n> +\t\t\t\tours, our_label, theirs, their_label,\n> +\t\t\t\t&default_opts);\n> +\t}\n> +\n> +\tif (opts->renormalize) {\n> \t\tnormalize_file(ancestor, path);\n> \t\tnormalize_file(ours, path);\n> \t\tnormalize_file(theirs, path);\n> @@ -359,11 +369,11 @@ int ll_merge(mmbuffer_t *result_buf,\n> \t\t}\n> \t}\n> \tdriver = find_ll_merge_driver(ll_driver_name);\n> -\tif (virtual_ancestor && driver->recursive)\n> +\tif (opts->virtual_ancestor && driver->recursive)\n> \t\tdriver = find_ll_merge_driver(driver->recursive);\n> \treturn driver->fn(driver, result_buf, path, ancestor, ancestor_label,\n> \t\t\t ours, our_label, theirs, their_label,\n> -\t\t\t flag, marker_size);\n> +\t\t\t opts, marker_size);\n> }\n>\n> int ll_merge_marker_size(const char *path)\n> diff --git a/ll-merge.h b/ll-merge.h\n> index ff7ca87..4b707f0 100644\n> --- a/ll-merge.h\n> +++ b/ll-merge.h\n> @@ -5,27 +5,18 @@\n> #ifndef LL_MERGE_H\n> #define LL_MERGE_H\n>\n> -#define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n> -#define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n> -#define LL_OPT_FAVOR_SHIFT 1\n> -#define LL_OPT_RENORMALIZE\t(1 << 3)\n> -\n> -static inline int ll_opt_favor(int flag)\n> -{\n> -\treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n> -}\n> -\n> -static inline int create_ll_flag(int favor)\n> -{\n> -\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n> -}\n> +struct ll_merge_options {\n> +\tunsigned virtual_ancestor : 1;\n> +\tunsigned variant : 2;\t/* favor ours, favor theirs, or union merge */\n> +\tunsigned renormalize : 1;\n> +};\n>\n> int ll_merge(mmbuffer_t *result_buf,\n> \t const char *path,\n> \t mmfile_t *ancestor, const char *ancestor_label,\n> \t mmfile_t *ours, const char *our_label,\n> \t mmfile_t *theirs, const char *their_label,\n> -\t int flag);\n> +\t const struct ll_merge_options *opts);\n>\n> int ll_merge_marker_size(const char *path);\n>\n> diff --git a/merge-file.c b/merge-file.c\n> index db4d0d5..f7f4533 100644\n> --- a/merge-file.c\n> +++ b/merge-file.c\n> @@ -37,7 +37,7 @@ static void *three_way_filemerge(const char *path, \n> mmfile_t *base, mmfile_t *our\n> \t * common ancestor.\n> \t */\n> \tmerge_status = ll_merge(&res, path, base, NULL,\n> -\t\t\t\tour, \".our\", their, \".their\", 0);\n> +\t\t\t\tour, \".our\", their, \".their\", NULL);\n> \tif (merge_status < 0)\n> \t\treturn NULL;\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 762b549..ed09785 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -608,22 +608,25 @@ static int merge_3way(struct merge_options *o,\n> \t\t const char *branch2)\n> {\n> \tmmfile_t orig, src1, src2;\n> +\tstruct ll_merge_options ll_opts = {0};\n> \tchar *base_name, *name1, *name2;\n> \tint merge_status;\n> -\tint favor;\n>\n> -\tif (o->call_depth)\n> -\t\tfavor = 0;\n> -\telse {\n> +\tll_opts.renormalize = o->renormalize;\n> +\n> +\tif (o->call_depth) {\n> +\t\tll_opts.virtual_ancestor = 1;\n> +\t\tll_opts.variant = 0;\n> +\t} else {\n> \t\tswitch (o->recursive_variant) {\n> \t\tcase MERGE_RECURSIVE_OURS:\n> -\t\t\tfavor = XDL_MERGE_FAVOR_OURS;\n> +\t\t\tll_opts.variant = XDL_MERGE_FAVOR_OURS;\n> \t\t\tbreak;\n> \t\tcase MERGE_RECURSIVE_THEIRS:\n> -\t\t\tfavor = XDL_MERGE_FAVOR_THEIRS;\n> +\t\t\tll_opts.variant = XDL_MERGE_FAVOR_THEIRS;\n> \t\t\tbreak;\n> \t\tdefault:\n> -\t\t\tfavor = 0;\n> +\t\t\tll_opts.variant = 0;\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> @@ -646,10 +649,7 @@ static int merge_3way(struct merge_options *o,\n> \tread_mmblob(&src2, b->sha1);\n>\n> \tmerge_status = ll_merge(result_buf, a->path, &orig, base_name,\n> -\t\t\t\t&src1, name1, &src2, name2,\n> -\t\t\t\t((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) |\n> -\t\t\t\t (o->renormalize ? LL_OPT_RENORMALIZE : 0) |\n> -\t\t\t\t create_ll_flag(favor)));\n> +\t\t\t\t&src1, name1, &src2, name2, &ll_opts);\n>\n> \tfree(name1);\n> \tfree(name2);\n> diff --git a/rerere.c b/rerere.c\n> index e40af0d..b180218 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -325,7 +325,7 @@ static int handle_cache(const char *path, \n> unsigned char *sha1, const char *outpu\n> \t */\n> \tll_merge(&result, path, &mmfile[0], NULL,\n> \t\t &mmfile[1], \"ours\",\n> -\t\t &mmfile[2], \"theirs\", 0);\n> +\t\t &mmfile[2], \"theirs\", NULL);\n> \tfor (i = 0; i < 3; i++)\n> \t\tfree(mmfile[i].ptr);\n>\n> -- \n> 1.7.2.2\n"},{"id":"148856","messageId":"7vk4nfvo56.fsf@alter.siamese.dyndns.org","threadId":"24840","inReplyTo":"20100824033922.GA19628@burratino","subject":"Re: [RFC/PATCH jn/merge-renormalize] merge-recursive: expose merge options for builtin merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T18:52:53Z","receivedAt":"2010-08-24T18:52:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> There are two very similar blocks of code that recognize options for\n> the \"recursive\" merge strategy. Unify them.\n>\n> No functional change intended.\n\nIt makes sense, but I wonder why you chose to do \"if (fun() <= 0) error\"\ninstead of usual \"if (fun() < 0) error\"...\n"},{"id":"148861","messageId":"7vsk23u96b.fsf@alter.siamese.dyndns.org","threadId":"24840","inReplyTo":"20100824022820.GE17406@burratino","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T19:01:32Z","receivedAt":"2010-08-24T19:01:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> The flag layout worries me a bit because presumably the next extension\n> would get bit 8, and so on. What happens if xdiff learns another\n> whitespace-ignoring mode?\n> ...\n> This function (like its cousin create_ll_flag) is ugly, as Bert\n> noticed before. I am starting to wonder if it would not be simpler\n> to use\n>\n> extern int ll_merge(..., const struct ll_merge_options *opts);\n>\n> treating NULL as a request for the default options.\n>\n> In other words, how about something like this?\n\nSounds like a good direction to go.\n"},{"id":"148865","messageId":"AANLkTint_aF2ZZue5PeSnaAVFa7v+b1diqm3xyvCQsVJ@mail.gmail.com","threadId":"24840","inReplyTo":"20100824022820.GE17406@burratino","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-08-24T20:01:26Z","receivedAt":"2010-08-24T20:01:26Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Hi,\n\nOn Tue, Aug 24, 2010 at 04:28, Jonathan Nieder wrote:\n> Subject: ll-merge: replace flag argument with options struct\n>\n> Keeping track of the flag bits is proving more trouble than it's\n> worth.  Instead, use a pointer to an options struct like most similar\n> APIs do.\n>\n> Callers with no special requests can pass NULL to request the default\n> options.\n\nthanks for doing this. But is there any prior art for using NULL as\n'use the default flags' in the project? For what I know, I can't think\nof an example. But my inside knowledge into git drifts slowly away.\n\nRegards,\nBert\n\n>\n> Cc: Bert Wesarg \n> Signed-off-by: Jonathan Nieder \n"},{"id":"148933","messageId":"20100825035718.GG11619@burratino","threadId":"24840","inReplyTo":"AANLkTint_aF2ZZue5PeSnaAVFa7v+b1diqm3xyvCQsVJ@mail.gmail.com","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-25T03:57:18Z","receivedAt":"2010-08-25T03:57:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nBert Wesarg wrote:\n\n> thanks for doing this. But is there any prior art for using NULL as\n> 'use the default flags' in the project? For what I know, I can't think\n> of an example. But my inside knowledge into git drifts slowly away.\n\nNot that I know of for option flags. Probably worth mentioning in the\nAPI docs, which I conveniently forgot to update.\n\n-- 8< --\nSubject: ll-merge: document options struct\n\nIn particular, try to make it clear that most callers will want to\npass `NULL` as the `opts` parameter.\n\nCc: Bert Wesarg \nCc: Avery Pennarun \nSigned-off-by: Jonathan Nieder \n---\ndiff --git a/Documentation/technical/api-merge.txt b/Documentation/technical/api-merge.txt\nindex a7e050b..9dc1bed 100644\n--- a/Documentation/technical/api-merge.txt\n+++ b/Documentation/technical/api-merge.txt\n@@ -17,6 +17,40 @@ responsible for a few things.\n path-specific merge drivers (specified in `.gitattributes`)\n into account.\n \n+Data structures\n+---------------\n+\n+* `mmbuffer_t`, `mmfile_t`\n+\n+These store data usable for use by the xdiff backend, for writing and\n+for reading, respectively. See `xdiff/xdiff.h` for the definitions\n+and `diff.c` for examples.\n+\n+* `struct ll_merge_options`\n+\n+This describes the set of options the calling program wants to affect\n+the operation of a low-level (single file) merge. Some options:\n+\n+`virtual_ancestor`::\n+\tBehave as though this were part of a merge between common\n+\tancestors in a recursive merge.\n+\tIf a helper program is specified by the\n+\t`[merge \"\"] recursive` configuration, it will\n+\tbe used (see linkgit:gitattributes[5]).\n+\n+`variant`::\n+\tResolve local conflicts automatically in favor\n+\tof one side or the other (as in 'git merge-file'\n+\t`--ours`/`--theirs`/`--union`). Can be `0`,\n+\t`XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`, or\n+\t`XDL_MERGE_FAVOR_UNION`.\n+\n+`renormalize`::\n+\tResmudge and clean the \"base\", \"theirs\" and \"ours\" files\n+\tbefore merging. Use this when the merge is likely to have\n+\toverlapped with a change in smudge/clean or end-of-line\n+\tnormalization rules.\n+\n Low-level (single file) merge\n -----------------------------\n \n@@ -28,15 +62,24 @@ Low-level (single file) merge\n \t`.git/info/attributes` into account. Returns 0 for a\n \tclean merge.\n \n-The caller:\n+Calling sequence:\n \n-1. allocates an mmbuffer_t variable for the result;\n-2. allocates and fills variables with the file's original content\n- and two modified versions (using `read_mmfile`, for example);\n-3. calls ll_merge();\n-4. reads the output from result_buf.ptr and result_buf.size;\n-5. releases buffers when finished (free(ancestor.ptr); free(ours.ptr);\n- free(theirs.ptr); free(result_buf.ptr);).\n+* Prepare a `struct ll_merge_options` to record options.\n+ If you have no special requests, skip this and pass `NULL`\n+ as the `opts` parameter to use the default options.\n+\n+* Allocate an mmbuffer_t variable for the result.\n+\n+* Allocate and fill variables with the file's original content\n+ and two modified versions (using `read_mmfile`, for example).\n+\n+* Call `ll_merge()`.\n+\n+* Read the merged content from `result_buf.ptr` and `result_buf.size`.\n+\n+* Release buffers when finished. A simple\n+ `free(ancestor.ptr); free(ours.ptr); free(theirs.ptr);\n+ free(result_buf.ptr);` will do.\n \n If the modifications do not merge cleanly, `ll_merge` will return a\n nonzero value and `result_buf` will generally include a description of\n@@ -47,18 +90,6 @@ The `ancestor_label`, `our_label`, and `their_label` parameters are\n used to label the different sides of a conflict if the merge driver\n supports this.\n \n-The `flag` parameter is a bitfield:\n-\n- - The `LL_OPT_VIRTUAL_ANCESTOR` bit indicates whether this is an\n- internal merge to consolidate ancestors for a recursive merge.\n-\n- - The `LL_OPT_FAVOR_MASK` bits allow local conflicts to be automatically\n- resolved in favor of one side or the other (as in 'git merge-file'\n- `--ours`/`--theirs`/`--union`).\n- They can be populated by `create_ll_flag`, whose argument can be\n- `XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`, or\n- `XDL_MERGE_FAVOR_UNION`.\n-\n Everything else\n ---------------\n \n-- \n"},{"id":"148937","messageId":"20100825042906.GI11619@burratino","threadId":"24840","inReplyTo":"7vk4nfvo56.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH jn/merge-renormalize] merge-recursive: expose merge options for builtin merge","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-25T04:29:06Z","receivedAt":"2010-08-25T04:29:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> There are two very similar blocks of code that recognize options for\n>> the \"recursive\" merge strategy. Unify them.\n>>\n>> No functional change intended.\n>\n> It makes sense, but I wonder why you chose to do \"if (fun() <= 0) error\"\n> instead of usual \"if (fun() < 0) error\"...\n\nNo good reason (an attempt to vaguely imitate the interface of\nhandle_revision_opt() from revision.c). Since both callers die for\nan unrecognized option, returning 0 for success and -1 for failure\nwould indeed be simpler.\n\nI'll try to find time to reroll the series (including --patience)\non top of jn/merge-renormalize tomorrow. Probably\n--ignore-space-change et al are safe, too, though some test cases\nwould be nice.\n"},{"id":"148939","messageId":"20100825044006.GJ11619@burratino","threadId":"24840","inReplyTo":"5F681E95-9369-490A-84FE-55B99EA8FE63@cockos.com","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-25T04:40:06Z","receivedAt":"2010-08-25T04:40:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Justin Frankel wrote:\n\n> Fine by me... (ok if I were to nitpick I would probably make most of\n> the internal static functions check that opts was non-NULL before\n> dereferencing, in case the calling code ever changed\n\nNot a bad idea. I'll squash in something like this.\n\nA real BUG_ON macro would make this less ugly.\n\n-- 8< --\nSubject: ll-merge: BUG_ON(!opts) in internal functions\n\nIf one of the functions used to implement ll_merge() gets exposed,\ncallers would be likely to pass a NULL options argument, resulting in\nsegfaults. Die with a more meaningful message in that case.\n\nSuggested-by: Justin Frankel \nSigned-off-by: Jonathan Nieder \n---\n ll-merge.c | 18 ++++++++++++++++--\n 1 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 98f353a..8ff0b27 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -42,12 +42,17 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t const struct ll_merge_options *opts,\n \t\t\t int marker_size)\n {\n+\tmmfile_t *stolen;\n+\n+\tif (!opts)\n+\t\tdie(\"BUG: !opts in binary merge driver\");\n+\n \t/*\n \t * The tentative merge result is \"ours\" for the final round,\n \t * or common ancestor for an internal merge. Still return\n \t * \"conflicted merge\" status.\n \t */\n-\tmmfile_t *stolen = opts->virtual_ancestor ? orig : src1;\n+\tstolen = opts->virtual_ancestor ? orig : src1;\n \n \tresult->ptr = stolen->ptr;\n \tresult->size = stolen->size;\n@@ -66,6 +71,9 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n {\n \txmparam_t xmp;\n \n+\tif (!opts)\n+\t\tdie(\"BUG: !opts in xdiff merge driver\");\n+\n \tif (buffer_is_binary(orig->ptr, orig->size) ||\n \t buffer_is_binary(src1->ptr, src1->size) ||\n \t buffer_is_binary(src2->ptr, src2->size)) {\n@@ -102,7 +110,10 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t int marker_size)\n {\n \t/* Use union favor */\n-\tstruct ll_merge_options o = *opts;\n+\tstruct ll_merge_options o;\n+\tif (!opts)\n+\t\tdie(\"BUG: !opts in union merge driver\");\n+\to = *opts;\n \to.variant = XDL_MERGE_FAVOR_UNION;\n \treturn ll_xdl_merge(drv_unused, result, path_unused,\n \t\t\t orig, NULL, src1, NULL, src2, NULL,\n@@ -149,6 +160,9 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \tint status, fd, i;\n \tstruct stat st;\n \n+\tif (!opts)\n+\t\tdie(\"BUG: !opts in custom merge driver\");\n+\n \tdict[0].placeholder = \"O\"; dict[0].value = temp[0];\n \tdict[1].placeholder = \"A\"; dict[1].value = temp[1];\n \tdict[2].placeholder = \"B\"; dict[2].value = temp[2];\n-- \n1.7.2.2\n"},{"id":"148954","messageId":"AANLkTimRyeMbLKLmbKJEqgGPDFo3OCsz=QwVzkSaP5n8@mail.gmail.com","threadId":"24840","inReplyTo":"20100825044006.GJ11619@burratino","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-08-25T07:22:34Z","receivedAt":"2010-08-25T07:22:34Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Hi,\n\nOn Wed, Aug 25, 2010 at 06:40, Jonathan Nieder wrote:\n> Justin Frankel wrote:\n>\n>> Fine by me... (ok if I were to nitpick I would probably make most of\n>> the internal static functions check that opts was non-NULL before\n>> dereferencing, in case the calling code ever changed\n>\n> Not a bad idea.  I'll squash in something like this.\n>\n> A real BUG_ON macro would make this less ugly.\n\nI think exactly because of this there is no prior art of using NULL as\n'use the default options'. Without this all these NULL pointer checks\nwont be necessary.\n\nBert\n"},{"id":"148971","messageId":"4C753BFD.5060807@cockos.com","threadId":"24840","inReplyTo":"AANLkTimRyeMbLKLmbKJEqgGPDFo3OCsz=QwVzkSaP5n8@mail.gmail.com","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Justin Frankel","fromEmail":"justin@cockos.com","sentAt":"2010-08-25T15:51:25Z","receivedAt":"2010-08-25T15:51:25Z","isPatch":true,"sender":{"key":"justin@cockos.com","avatar":"https://avatars.githubusercontent.com/u/5803984?v=4"},"body":"Bert Wesarg wrote:\n> Hi,\n>\n> On Wed, Aug 25, 2010 at 06:40, Jonathan Nieder wrote:\n> \n>> Justin Frankel wrote:\n>>\n>> \n>>> Fine by me... (ok if I were to nitpick I would probably make most of\n>>> the internal static functions check that opts was non-NULL before\n>>> dereferencing, in case the calling code ever changed\n>>> \n>> Not a bad idea. I'll squash in something like this.\n>>\n>> A real BUG_ON macro would make this less ugly.\n>> \n>\n> I think exactly because of this there is no prior art of using NULL as\n> 'use the default options'. Without this all these NULL pointer checks\n> wont be necessary.\n> \n\nThe only danger is that ll_merge()'s signature didn't change in such a \nway to break compilation, i.e:\n\nint ll_merge(mmbuffer_t *result_buf,\n const char *path,\n mmfile_t *ancestor, const char *ancestor_label,\n mmfile_t *ours, const char *our_label,\n mmfile_t *theirs, const char *their_label,\n int flag);\n\nbecomes:\n\nint ll_merge(mmbuffer_t *result_buf,\n const char *path,\n mmfile_t *ancestor, const char *ancestor_label,\n mmfile_t *ours, const char *our_label,\n mmfile_t *theirs, const char *their_label,\n struct whatever *conf);\n\nIn this case, passing 0 as the last parameter will compile either way.\n\nSure, we can grep all of the source, but who knows when something else \nwill get merged in...\n\n-Justin\n"},{"id":"148978","messageId":"7vy6bushjx.fsf@alter.siamese.dyndns.org","threadId":"24840","inReplyTo":"4C753BFD.5060807@cockos.com","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-25T17:55:46Z","receivedAt":"2010-08-25T17:55:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Frankel writes:\n\n> The only danger is that ll_merge()'s signature didn't change in such a\n> way to break compilation, i.e:\n>\n> int ll_merge(mmbuffer_t *result_buf,\n> const char *path,\n> mmfile_t *ancestor, const char *ancestor_label,\n> mmfile_t *ours, const char *our_label,\n> mmfile_t *theirs, const char *their_label,\n> int flag);\n>\n> becomes:\n>\n> int ll_merge(mmbuffer_t *result_buf,\n> const char *path,\n> mmfile_t *ancestor, const char *ancestor_label,\n> mmfile_t *ours, const char *our_label,\n> mmfile_t *theirs, const char *their_label,\n> struct whatever *conf);\n>\n> In this case, passing 0 as the last parameter will compile either way.\n>\n> Sure, we can grep all of the source, but who knows when something else\n> will get merged in...\n\nThat is technically a valid concern but I suspect it does not matter in\nthis particular case, where integer 0 used to mean \"use the default\" and\nthe new API uses NULL to mean the same.\n\nIf an existing call site used to pass 0 and the patch forgot to update it,\nit will look ugly (we encourage to spell a NULL pointer \"NULL\", not \"0\",\nin our codebase) but no harm is done. If an existing call site asked for\na non-default behaviour by passing a non-zero integer flag, and the patch\nforgot to update it, the compiler would have caught it. Merging a side\nbranch is the same deal; if it adds a call with a non-zero argument to ask\nfor a non-default behaviour, that will be done via an expression over some\ninteger variables or constants, and that won't be casted silently to a\npointer to \"struct whatever\", no?\n"},{"id":"148980","messageId":"4C755F0E.5080705@cockos.com","threadId":"24840","inReplyTo":"7vy6bushjx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] git-merge: ignore space support","fromName":"Justin Frankel","fromEmail":"justin@cockos.com","sentAt":"2010-08-25T18:21:02Z","receivedAt":"2010-08-25T18:21:02Z","isPatch":true,"sender":{"key":"justin@cockos.com","avatar":"https://avatars.githubusercontent.com/u/5803984?v=4"},"body":"Junio C Hamano wrote:\n> Justin Frankel writes:\n>\n> \n>> The only danger is that ll_merge()'s signature didn't change in such a\n>> way to break compilation, i.e:\n>>\n>> int ll_merge(mmbuffer_t *result_buf,\n>> const char *path,\n>> mmfile_t *ancestor, const char *ancestor_label,\n>> mmfile_t *ours, const char *our_label,\n>> mmfile_t *theirs, const char *their_label,\n>> int flag);\n>>\n>> becomes:\n>>\n>> int ll_merge(mmbuffer_t *result_buf,\n>> const char *path,\n>> mmfile_t *ancestor, const char *ancestor_label,\n>> mmfile_t *ours, const char *our_label,\n>> mmfile_t *theirs, const char *their_label,\n>> struct whatever *conf);\n>>\n>> In this case, passing 0 as the last parameter will compile either way.\n>>\n>> Sure, we can grep all of the source, but who knows when something else\n>> will get merged in...\n>> \n>\n> That is technically a valid concern but I suspect it does not matter in\n> this particular case, where integer 0 used to mean \"use the default\" and\n> the new API uses NULL to mean the same.\n>\n> If an existing call site used to pass 0 and the patch forgot to update it,\n> it will look ugly (we encourage to spell a NULL pointer \"NULL\", not \"0\",\n> in our codebase) but no harm is done. If an existing call site asked for\n> a non-default behaviour by passing a non-zero integer flag, and the patch\n> forgot to update it, the compiler would have caught it. Merging a side\n> branch is the same deal; if it adds a call with a non-zero argument to ask\n> for a non-default behaviour, that will be done via an expression over some\n> integer variables or constants, and that won't be casted silently to a\n> pointer to \"struct whatever\", no?\n>\n> \nAgreed, I was responding to Bert's email, in which he stated that he \nhadn't seen NULL-for-default anywhere else in git. Using NULL for \ndefault behavior is good in that it handles un-updated code and merges \ncorrectly (passing 0 uses defaults, passing nonzero fails compile).\n\n\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"149034","messageId":"20100826054158.GB9708@burratino","threadId":"24840","inReplyTo":"20100823205915.GA4484@ns1.cockos.com","subject":"[PATCH v3 0/4] git-merge: ignore space support","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:41:58Z","receivedAt":"2010-08-26T05:41:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJustin Frankel wrote:\n\n> git-merge and git-rebase can now be passed -Xignore-space-at-eol,\n> -Xignore-space-change, -Xignore-all-space, -Xpatience options.\n\nI've tried this out a little and it seems to do the right thing.\nHere's the series in its current state.\n\nPatch 1 is to avoid the code duplication between builtin/merge and\nmerge-recursive you noticed, by adding a function that recognizes -X\noptions to merge-recursive.h.\n\nUnlike the trial balloon patch I sent before, it uses a conventional\nreturn value with 0 meaning \"recognized\" and -1 meaning \"not recognized\".\n\nPatch 2 replaces the flag parameter to ll_merge with a struct.\nI think the maintenance trouble of keeping track of the flag bits is\nnot worth it for a marginal speedup.\n\nPatches 3 and 4 provide \"merge -Xpatience\" and\n\"merge -Xignore-space-change\" et al on top of this foundation.\nI split them up and put patience first because it is worth thinking\ncarefully about which version of a line with only whitespace changes\na merge will take.\n\nNo tests yet. (If someone would like to add them, that would be\ngreat.) Probably after responding to initial comments I will not have\nmuch time for this series, so please feel free to pick it up and take\nit in whatever direction you wish.\n\nJonathan Nieder (2):\n merge-recursive: expose merge options for builtin merge\n ll-merge: replace flag argument with options struct\n\nJustin Frankel (2):\n merge-recursive --patience\n merge-recursive: options to ignore whitespace changes\n\n Documentation/merge-strategies.txt | 22 ++++++++++\n Documentation/technical/api-merge.txt | 73 +++++++++++++++++++++++---------\n builtin/checkout.c | 2 +-\n builtin/merge-recursive.c | 15 +------\n builtin/merge.c | 20 +--------\n ll-merge.c | 50 +++++++++++++++-------\n ll-merge.h | 22 +++-------\n merge-file.c | 2 +-\n merge-recursive.c | 54 +++++++++++++++++++-----\n merge-recursive.h | 3 +\n rerere.c | 2 +-\n 11 files changed, 168 insertions(+), 97 deletions(-)\n\n-- \n1.7.2.2\n"},{"id":"149036","messageId":"20100826054758.GD9708@burratino","threadId":"24840","inReplyTo":"20100826054158.GB9708@burratino","subject":"[PATCH 1/4] merge-recursive: expose merge options for builtin merge","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:47:58Z","receivedAt":"2010-08-26T05:47:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"There are two very similar blocks of code that recognize options for\nthe \"recursive\" merge strategy. Unify them.\n\nNo functional change intended.\n\nCc: Avery Pennarun \nHelped-by: Junio C Hamano \nSigned-off-by: Jonathan Nieder \n---\n builtin/merge-recursive.c | 14 +-------------\n builtin/merge.c | 20 ++------------------\n merge-recursive.c | 21 +++++++++++++++++++++\n merge-recursive.h | 2 ++\n 4 files changed, 26 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex c2d4677..5a52f3d 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -37,19 +37,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--\")) {\n \t\t\tif (!arg[2])\n \t\t\t\tbreak;\n-\t\t\tif (!strcmp(arg+2, \"ours\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(arg+2, \"theirs\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(arg+2, \"subtree\"))\n-\t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(arg+2, \"subtree=\"))\n-\t\t\t\to.subtree_shift = arg + 10;\n-\t\t\telse if (!strcmp(arg+2, \"renormalize\"))\n-\t\t\t\to.renormalize = 1;\n-\t\t\telse if (!strcmp(arg+2, \"no-renormalize\"))\n-\t\t\t\to.renormalize = 0;\n-\t\t\telse\n+\t\t\tif (parse_merge_opt(&o, arg + 2))\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 037cd47..721c424 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -629,25 +629,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \n \t\to.renormalize = option_renormalize;\n \n-\t\t/*\n-\t\t * NEEDSWORK: merge with table in builtin/merge-recursive\n-\t\t */\n-\t\tfor (x = 0; x < xopts_nr; x++) {\n-\t\t\tif (!strcmp(xopts[x], \"ours\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(xopts[x], \"theirs\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(xopts[x], \"subtree\"))\n-\t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(xopts[x], \"subtree=\"))\n-\t\t\t\to.subtree_shift = xopts[x]+8;\n-\t\t\telse if (!strcmp(xopts[x], \"renormalize\"))\n-\t\t\t\to.renormalize = 1;\n-\t\t\telse if (!strcmp(xopts[x], \"no-renormalize\"))\n-\t\t\t\to.renormalize = 0;\n-\t\t\telse\n+\t\tfor (x = 0; x < xopts_nr; x++)\n+\t\t\tif (parse_merge_opt(&o, xopts[x]))\n \t\t\t\tdie(\"Unknown option for merge-recursive: -X%s\", xopts[x]);\n-\t\t}\n \n \t\to.branch1 = head_arg;\n \t\to.branch2 = remoteheads->item->util;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 762b549..44576b7 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1499,3 +1499,24 @@ void init_merge_options(struct merge_options *o)\n \tmemset(&o->current_directory_set, 0, sizeof(struct string_list));\n \to->current_directory_set.strdup_strings = 1;\n }\n+\n+int parse_merge_opt(struct merge_options *o, const char *s)\n+{\n+\tif (!s || !*s)\n+\t\treturn -1;\n+\tif (!strcmp(s, \"ours\"))\n+\t\to->recursive_variant = MERGE_RECURSIVE_OURS;\n+\telse if (!strcmp(s, \"theirs\"))\n+\t\to->recursive_variant = MERGE_RECURSIVE_THEIRS;\n+\telse if (!strcmp(s, \"subtree\"))\n+\t\to->subtree_shift = \"\";\n+\telse if (!prefixcmp(s, \"subtree=\"))\n+\t\to->subtree_shift = s + strlen(\"subtree=\");\n+\telse if (!strcmp(s, \"renormalize\"))\n+\t\to->renormalize = 1;\n+\telse if (!strcmp(s, \"no-renormalize\"))\n+\t\to->renormalize = 0;\n+\telse\n+\t\treturn -1;\n+\treturn 0;\n+}\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex c5fbe79..37ff99a 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -55,6 +55,8 @@ int merge_recursive_generic(struct merge_options *o,\n void init_merge_options(struct merge_options *o);\n struct tree *write_tree_from_memory(struct merge_options *o);\n \n+int parse_merge_opt(struct merge_options *out, const char *s);\n+\n /* builtin/merge.c */\n int try_merge_command(const char *strategy, struct commit_list *common, const char *head_arg, struct commit_list *remotes);\n \n-- \n1.7.2.2\n"},{"id":"149037","messageId":"20100826054953.GE9708@burratino","threadId":"24840","inReplyTo":"20100826054158.GB9708@burratino","subject":"[PATCH 2/4] ll-merge: replace flag argument with options struct","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:49:53Z","receivedAt":"2010-08-26T05:49:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Keeping track of the flag bits is proving more trouble than it's\nworth. Instead, use a pointer to an options struct like most similar\nAPIs do.\n\nCallers with no special requests can pass NULL to request the default\noptions.\n\nCc: Bert Wesarg \nCc: Avery Pennarun \nHelped-by: Justin Frankel \nHelped-by: Bert Wesarg \nSigned-off-by: Jonathan Nieder \n---\nThis time, with updated documentation.\n\nLet's avoid the die(\"BUG: ...\") ugliness by using assert().\n\n Documentation/technical/api-merge.txt | 73 +++++++++++++++++++++++---------\n builtin/checkout.c | 2 +-\n ll-merge.c | 49 +++++++++++++++-------\n ll-merge.h | 21 +++-------\n merge-file.c | 2 +-\n merge-recursive.c | 22 +++++-----\n rerere.c | 2 +-\n 7 files changed, 105 insertions(+), 66 deletions(-)\n\ndiff --git a/Documentation/technical/api-merge.txt b/Documentation/technical/api-merge.txt\nindex a7e050b..9dc1bed 100644\n--- a/Documentation/technical/api-merge.txt\n+++ b/Documentation/technical/api-merge.txt\n@@ -17,6 +17,40 @@ responsible for a few things.\n path-specific merge drivers (specified in `.gitattributes`)\n into account.\n \n+Data structures\n+---------------\n+\n+* `mmbuffer_t`, `mmfile_t`\n+\n+These store data usable for use by the xdiff backend, for writing and\n+for reading, respectively. See `xdiff/xdiff.h` for the definitions\n+and `diff.c` for examples.\n+\n+* `struct ll_merge_options`\n+\n+This describes the set of options the calling program wants to affect\n+the operation of a low-level (single file) merge. Some options:\n+\n+`virtual_ancestor`::\n+\tBehave as though this were part of a merge between common\n+\tancestors in a recursive merge.\n+\tIf a helper program is specified by the\n+\t`[merge \"\"] recursive` configuration, it will\n+\tbe used (see linkgit:gitattributes[5]).\n+\n+`variant`::\n+\tResolve local conflicts automatically in favor\n+\tof one side or the other (as in 'git merge-file'\n+\t`--ours`/`--theirs`/`--union`). Can be `0`,\n+\t`XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`, or\n+\t`XDL_MERGE_FAVOR_UNION`.\n+\n+`renormalize`::\n+\tResmudge and clean the \"base\", \"theirs\" and \"ours\" files\n+\tbefore merging. Use this when the merge is likely to have\n+\toverlapped with a change in smudge/clean or end-of-line\n+\tnormalization rules.\n+\n Low-level (single file) merge\n -----------------------------\n \n@@ -28,15 +62,24 @@ Low-level (single file) merge\n \t`.git/info/attributes` into account. Returns 0 for a\n \tclean merge.\n \n-The caller:\n-\n-1. allocates an mmbuffer_t variable for the result;\n-2. allocates and fills variables with the file's original content\n- and two modified versions (using `read_mmfile`, for example);\n-3. calls ll_merge();\n-4. reads the output from result_buf.ptr and result_buf.size;\n-5. releases buffers when finished (free(ancestor.ptr); free(ours.ptr);\n- free(theirs.ptr); free(result_buf.ptr);).\n+Calling sequence:\n+\n+* Prepare a `struct ll_merge_options` to record options.\n+ If you have no special requests, skip this and pass `NULL`\n+ as the `opts` parameter to use the default options.\n+\n+* Allocate an mmbuffer_t variable for the result.\n+\n+* Allocate and fill variables with the file's original content\n+ and two modified versions (using `read_mmfile`, for example).\n+\n+* Call `ll_merge()`.\n+\n+* Read the merged content from `result_buf.ptr` and `result_buf.size`.\n+\n+* Release buffers when finished. A simple\n+ `free(ancestor.ptr); free(ours.ptr); free(theirs.ptr);\n+ free(result_buf.ptr);` will do.\n \n If the modifications do not merge cleanly, `ll_merge` will return a\n nonzero value and `result_buf` will generally include a description of\n@@ -47,18 +90,6 @@ The `ancestor_label`, `our_label`, and `their_label` parameters are\n used to label the different sides of a conflict if the merge driver\n supports this.\n \n-The `flag` parameter is a bitfield:\n-\n- - The `LL_OPT_VIRTUAL_ANCESTOR` bit indicates whether this is an\n- internal merge to consolidate ancestors for a recursive merge.\n-\n- - The `LL_OPT_FAVOR_MASK` bits allow local conflicts to be automatically\n- resolved in favor of one side or the other (as in 'git merge-file'\n- `--ours`/`--theirs`/`--union`).\n- They can be populated by `create_ll_flag`, whose argument can be\n- `XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`, or\n- `XDL_MERGE_FAVOR_UNION`.\n-\n Everything else\n ---------------\n \ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 24b67d5..4d36b28 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -155,7 +155,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \t * merge.renormalize set, too\n \t */\n \tstatus = ll_merge(&result_buf, path, &ancestor, \"base\",\n-\t\t\t &ours, \"ours\", &theirs, \"theirs\", 0);\n+\t\t\t &ours, \"ours\", &theirs, \"theirs\", NULL);\n \tfree(ancestor.ptr);\n \tfree(ours.ptr);\n \tfree(theirs.ptr);\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 6bb3095..9bd3732 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -18,7 +18,7 @@ typedef int (*ll_merge_fn)(const struct ll_merge_driver *,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag,\n+\t\t\t const struct ll_merge_options *opts,\n \t\t\t int marker_size);\n \n struct ll_merge_driver {\n@@ -39,14 +39,18 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag, int marker_size)\n+\t\t\t const struct ll_merge_options *opts,\n+\t\t\t int marker_size)\n {\n+\tmmfile_t *stolen;\n+\tassert(opts);\n+\n \t/*\n \t * The tentative merge result is \"ours\" for the final round,\n \t * or common ancestor for an internal merge. Still return\n \t * \"conflicted merge\" status.\n \t */\n-\tmmfile_t *stolen = (flag & LL_OPT_VIRTUAL_ANCESTOR) ? orig : src1;\n+\tstolen = opts->virtual_ancestor ? orig : src1;\n \n \tresult->ptr = stolen->ptr;\n \tresult->size = stolen->size;\n@@ -60,9 +64,11 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \t\t\tmmfile_t *orig, const char *orig_name,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n-\t\t\tint flag, int marker_size)\n+\t\t\tconst struct ll_merge_options *opts,\n+\t\t\tint marker_size)\n {\n \txmparam_t xmp;\n+\tassert(opts);\n \n \tif (buffer_is_binary(orig->ptr, orig->size) ||\n \t buffer_is_binary(src1->ptr, src1->size) ||\n@@ -74,12 +80,12 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t\t orig, orig_name,\n \t\t\t\t src1, name1,\n \t\t\t\t src2, name2,\n-\t\t\t\t flag, marker_size);\n+\t\t\t\t opts, marker_size);\n \t}\n \n \tmemset(&xmp, 0, sizeof(xmp));\n \txmp.level = XDL_MERGE_ZEALOUS;\n-\txmp.favor = ll_opt_favor(flag);\n+\txmp.favor = opts->variant;\n \tif (git_xmerge_style >= 0)\n \t\txmp.style = git_xmerge_style;\n \tif (marker_size > 0)\n@@ -96,14 +102,17 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t mmfile_t *orig, const char *orig_name,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n-\t\t\t int flag, int marker_size)\n+\t\t\t const struct ll_merge_options *opts,\n+\t\t\t int marker_size)\n {\n \t/* Use union favor */\n-\tflag &= ~LL_OPT_FAVOR_MASK;\n-\tflag |= create_ll_flag(XDL_MERGE_FAVOR_UNION);\n+\tstruct ll_merge_options o;\n+\tassert(opts);\n+\to = *opts;\n+\to.variant = XDL_MERGE_FAVOR_UNION;\n \treturn ll_xdl_merge(drv_unused, result, path_unused,\n \t\t\t orig, NULL, src1, NULL, src2, NULL,\n-\t\t\t flag, marker_size);\n+\t\t\t &o, marker_size);\n \treturn 0;\n }\n \n@@ -136,7 +145,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \t\t\tmmfile_t *orig, const char *orig_name,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n-\t\t\tint flag, int marker_size)\n+\t\t\tconst struct ll_merge_options *opts,\n+\t\t\tint marker_size)\n {\n \tchar temp[4][50];\n \tstruct strbuf cmd = STRBUF_INIT;\n@@ -144,6 +154,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \tconst char *args[] = { NULL, NULL };\n \tint status, fd, i;\n \tstruct stat st;\n+\tassert(opts);\n \n \tdict[0].placeholder = \"O\"; dict[0].value = temp[0];\n \tdict[1].placeholder = \"A\"; dict[1].value = temp[1];\n@@ -337,15 +348,21 @@ int ll_merge(mmbuffer_t *result_buf,\n \t mmfile_t *ancestor, const char *ancestor_label,\n \t mmfile_t *ours, const char *our_label,\n \t mmfile_t *theirs, const char *their_label,\n-\t int flag)\n+\t const struct ll_merge_options *opts)\n {\n \tstatic struct git_attr_check check[2];\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n \tconst struct ll_merge_driver *driver;\n-\tint virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR;\n \n-\tif (flag & LL_OPT_RENORMALIZE) {\n+\tif (!opts) {\n+\t\tstruct ll_merge_options default_opts = {0};\n+\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n+\t\t\t\tours, our_label, theirs, their_label,\n+\t\t\t\t&default_opts);\n+\t}\n+\n+\tif (opts->renormalize) {\n \t\tnormalize_file(ancestor, path);\n \t\tnormalize_file(ours, path);\n \t\tnormalize_file(theirs, path);\n@@ -359,11 +376,11 @@ int ll_merge(mmbuffer_t *result_buf,\n \t\t}\n \t}\n \tdriver = find_ll_merge_driver(ll_driver_name);\n-\tif (virtual_ancestor && driver->recursive)\n+\tif (opts->virtual_ancestor && driver->recursive)\n \t\tdriver = find_ll_merge_driver(driver->recursive);\n \treturn driver->fn(driver, result_buf, path, ancestor, ancestor_label,\n \t\t\t ours, our_label, theirs, their_label,\n-\t\t\t flag, marker_size);\n+\t\t\t opts, marker_size);\n }\n \n int ll_merge_marker_size(const char *path)\ndiff --git a/ll-merge.h b/ll-merge.h\nindex ff7ca87..4b707f0 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -5,27 +5,18 @@\n #ifndef LL_MERGE_H\n #define LL_MERGE_H\n \n-#define LL_OPT_VIRTUAL_ANCESTOR\t(1 << 0)\n-#define LL_OPT_FAVOR_MASK\t((1 << 1) | (1 << 2))\n-#define LL_OPT_FAVOR_SHIFT 1\n-#define LL_OPT_RENORMALIZE\t(1 << 3)\n-\n-static inline int ll_opt_favor(int flag)\n-{\n-\treturn (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT;\n-}\n-\n-static inline int create_ll_flag(int favor)\n-{\n-\treturn ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK);\n-}\n+struct ll_merge_options {\n+\tunsigned virtual_ancestor : 1;\n+\tunsigned variant : 2;\t/* favor ours, favor theirs, or union merge */\n+\tunsigned renormalize : 1;\n+};\n \n int ll_merge(mmbuffer_t *result_buf,\n \t const char *path,\n \t mmfile_t *ancestor, const char *ancestor_label,\n \t mmfile_t *ours, const char *our_label,\n \t mmfile_t *theirs, const char *their_label,\n-\t int flag);\n+\t const struct ll_merge_options *opts);\n \n int ll_merge_marker_size(const char *path);\n \ndiff --git a/merge-file.c b/merge-file.c\nindex db4d0d5..f7f4533 100644\n--- a/merge-file.c\n+++ b/merge-file.c\n@@ -37,7 +37,7 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our\n \t * common ancestor.\n \t */\n \tmerge_status = ll_merge(&res, path, base, NULL,\n-\t\t\t\tour, \".our\", their, \".their\", 0);\n+\t\t\t\tour, \".our\", their, \".their\", NULL);\n \tif (merge_status < 0)\n \t\treturn NULL;\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 44576b7..0e9a29d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -608,22 +608,25 @@ static int merge_3way(struct merge_options *o,\n \t\t const char *branch2)\n {\n \tmmfile_t orig, src1, src2;\n+\tstruct ll_merge_options ll_opts = {0};\n \tchar *base_name, *name1, *name2;\n \tint merge_status;\n-\tint favor;\n \n-\tif (o->call_depth)\n-\t\tfavor = 0;\n-\telse {\n+\tll_opts.renormalize = o->renormalize;\n+\n+\tif (o->call_depth) {\n+\t\tll_opts.virtual_ancestor = 1;\n+\t\tll_opts.variant = 0;\n+\t} else {\n \t\tswitch (o->recursive_variant) {\n \t\tcase MERGE_RECURSIVE_OURS:\n-\t\t\tfavor = XDL_MERGE_FAVOR_OURS;\n+\t\t\tll_opts.variant = XDL_MERGE_FAVOR_OURS;\n \t\t\tbreak;\n \t\tcase MERGE_RECURSIVE_THEIRS:\n-\t\t\tfavor = XDL_MERGE_FAVOR_THEIRS;\n+\t\t\tll_opts.variant = XDL_MERGE_FAVOR_THEIRS;\n \t\t\tbreak;\n \t\tdefault:\n-\t\t\tfavor = 0;\n+\t\t\tll_opts.variant = 0;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -646,10 +649,7 @@ static int merge_3way(struct merge_options *o,\n \tread_mmblob(&src2, b->sha1);\n \n \tmerge_status = ll_merge(result_buf, a->path, &orig, base_name,\n-\t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\t((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) |\n-\t\t\t\t (o->renormalize ? LL_OPT_RENORMALIZE : 0) |\n-\t\t\t\t create_ll_flag(favor)));\n+\t\t\t\t&src1, name1, &src2, name2, &ll_opts);\n \n \tfree(name1);\n \tfree(name2);\ndiff --git a/rerere.c b/rerere.c\nindex e40af0d..b180218 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -325,7 +325,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \t */\n \tll_merge(&result, path, &mmfile[0], NULL,\n \t\t &mmfile[1], \"ours\",\n-\t\t &mmfile[2], \"theirs\", 0);\n+\t\t &mmfile[2], \"theirs\", NULL);\n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfile[i].ptr);\n \n-- \n1.7.2.2\n"},{"id":"149038","messageId":"20100826055045.GF9708@burratino","threadId":"24840","inReplyTo":"20100826054158.GB9708@burratino","subject":"[PATCH 3/4] merge-recursive --patience","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:50:45Z","receivedAt":"2010-08-26T05:50:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Justin Frankel \n\nTeach the merge-recursive strategy a --patience option to use the\n\"patience diff\" algorithm, which tends to improve results when\ncherry-picking a patch that reorders functions at the same time as\nrefactoring them.\n\nTo support this, struct merge_options and ll_merge_options gain an\nxdl_opts member, so programs can use arbitrary xdiff flags (think\n\"XDF_IGNORE_WHITESPACE\") in a git-aware merge.\n\ngit merge and git rebase can be passed the -Xpatience option to\nuse this.\n\n[jn: split from --ignore-space patch; with documentation]\n\nSigned-off-by: Justin Frankel \nSigned-off-by: Jonathan Nieder \n---\n Documentation/merge-strategies.txt | 7 +++++++\n builtin/merge-recursive.c | 1 +\n ll-merge.c | 1 +\n ll-merge.h | 1 +\n merge-recursive.c | 3 +++\n merge-recursive.h | 1 +\n 6 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 049313d..a5ae14f 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -40,6 +40,13 @@ the other tree did, declaring 'our' history contains all that happened in it.\n theirs;;\n \tThis is opposite of 'ours'.\n \n+patience;;\n+\tWith this option, 'merge-recursive' spends a little extra time\n+\tto avoid mismerges that sometimes occur due to unimportant\n+\tmatching lines (e.g., braces from distinct functions). Use\n+\tthis when the branches to be merged have diverged wildly.\n+\tSee also linkgit:git-diff[1] `--patience`.\n+\n renormalize;;\n \tThis runs a virtual check-out and check-in of all three stages\n \tof a file when resolving a three-way merge. This option is\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex 5a52f3d..70e1d25 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -2,6 +2,7 @@\n #include \"commit.h\"\n #include \"tag.h\"\n #include \"merge-recursive.h\"\n+#include \"xdiff-interface.h\"\n \n static const char *better_branch_name(const char *branch)\n {\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 9bd3732..dea6f2f 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -86,6 +86,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \tmemset(&xmp, 0, sizeof(xmp));\n \txmp.level = XDL_MERGE_ZEALOUS;\n \txmp.favor = opts->variant;\n+\txmp.xpp.flags = opts->xdl_opts;\n \tif (git_xmerge_style >= 0)\n \t\txmp.style = git_xmerge_style;\n \tif (marker_size > 0)\ndiff --git a/ll-merge.h b/ll-merge.h\nindex 4b707f0..244a31f 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -9,6 +9,7 @@ struct ll_merge_options {\n \tunsigned virtual_ancestor : 1;\n \tunsigned variant : 2;\t/* favor ours, favor theirs, or union merge */\n \tunsigned renormalize : 1;\n+\tlong xdl_opts;\n };\n \n int ll_merge(mmbuffer_t *result_buf,\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0e9a29d..3e67f81 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -613,6 +613,7 @@ static int merge_3way(struct merge_options *o,\n \tint merge_status;\n \n \tll_opts.renormalize = o->renormalize;\n+\tll_opts.xdl_opts = o->xdl_opts;\n \n \tif (o->call_depth) {\n \t\tll_opts.virtual_ancestor = 1;\n@@ -1512,6 +1513,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->subtree_shift = \"\";\n \telse if (!prefixcmp(s, \"subtree=\"))\n \t\to->subtree_shift = s + strlen(\"subtree=\");\n+\telse if (!strcmp(s, \"patience\"))\n+\t\to->xdl_opts |= XDF_PATIENCE_DIFF;\n \telse if (!strcmp(s, \"renormalize\"))\n \t\to->renormalize = 1;\n \telse if (!strcmp(s, \"no-renormalize\"))\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 37ff99a..d21b446 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -15,6 +15,7 @@ struct merge_options {\n \tconst char *subtree_shift;\n \tunsigned buffer_output : 1;\n \tunsigned renormalize : 1;\n+\tlong xdl_opts;\n \tint verbosity;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n-- \n1.7.2.2\n"},{"id":"149039","messageId":"20100826055147.GG9708@burratino","threadId":"24840","inReplyTo":"20100826054158.GB9708@burratino","subject":"[PATCH 4/4] merge-recursive: options to ignore whitespace changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:51:47Z","receivedAt":"2010-08-26T05:51:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Justin Frankel \n\nAdd support for merging with ignoring line endings (specifically\n--ignore-space-at-eol) when using recursive merging. This is\nas a strategy-option, so that you can do:\n\n\tgit merge --strategy-option=ignore-space-at-eol \n\nand\n\n\tgit rebase --strategy-option=ignore-space-at-eol \n\nThis can be useful for coping with line-ending damage (Xcode 3.1 has a\nnasty habit of converting all CRLFs to LFs, and VC6 tends to just use\nCRLFs for inserted lines).\n\nThe only option I need is ignore-space-at-eol, but while at it,\ninclude the other xdiff whitespace options (ignore-space-change,\nignore-all-space), too.\n\n[jn: with documentation]\n\nSigned-off-by: Justin Frankel \nSigned-off-by: Jonathan Nieder \n---\nThat's the end of the series. Thoughts (and tests) welcome, as\nalways.\n\n Documentation/merge-strategies.txt | 15 +++++++++++++++\n merge-recursive.c | 8 ++++++++\n 2 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex a5ae14f..91faba5 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -47,6 +47,21 @@ patience;;\n \tthis when the branches to be merged have diverged wildly.\n \tSee also linkgit:git-diff[1] `--patience`.\n \n+ignore-space-change;;\n+ignore-all-space;;\n+ignore-space-at-eol;;\n+\tTreats lines with the indicated type of whitespace change as\n+\tunchanged for the sake of a three-way merge. Whitespace\n+\tchanges mixed with other changes to a line are not ignored.\n+\tSee also linkgit:git-diff[1] `-b`, `-w`, and\n+\t`--ignore-space-at-eol`.\n++\n+* If 'their' version only introduces whitespace changes to a line,\n+ 'our' version is used;\n+* If 'our' version introduces whitespace changes but 'their'\n+ version includes a substantial change, 'their' version is used;\n+* Otherwise, the merge proceeds in the usual way.\n+\n renormalize;;\n \tThis runs a virtual check-out and check-in of all three stages\n \tof a file when resolving a three-way merge. This option is\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 3e67f81..944ca19 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1515,6 +1515,14 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->subtree_shift = s + strlen(\"subtree=\");\n \telse if (!strcmp(s, \"patience\"))\n \t\to->xdl_opts |= XDF_PATIENCE_DIFF;\n+\telse if (!strcmp(s, \"ignore-space-change\"))\n+\t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n+\telse if (!strcmp(s, \"ignore-all-space\"))\n+\t\to->xdl_opts |= XDF_IGNORE_WHITESPACE;\n+\telse if (!strcmp(s, \"ignore-space-at-eol\"))\n+\t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n+\telse if (!strcmp(s, \"ignore-space-at-eol\"))\n+\t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n \telse if (!strcmp(s, \"renormalize\"))\n \t\to->renormalize = 1;\n \telse if (!strcmp(s, \"no-renormalize\"))\n-- \n1.7.2.2\n"},{"id":"149067","messageId":"7vpqx5mipt.fsf@alter.siamese.dyndns.org","threadId":"24840","inReplyTo":"20100826054953.GE9708@burratino","subject":"Re: [PATCH 2/4] ll-merge: replace flag argument with options struct","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-26T16:39:26Z","receivedAt":"2010-08-26T16:39:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Keeping track of the flag bits is proving more trouble than it's\n> worth. Instead, use a pointer to an options struct like most similar\n> APIs do.\n>\n> Callers with no special requests can pass NULL to request the default\n> options.\n>\n> Cc: Bert Wesarg \n> Cc: Avery Pennarun \n> Helped-by: Justin Frankel \n> Helped-by: Bert Wesarg \n> Signed-off-by: Jonathan Nieder \n> ---\n> This time, with updated documentation.\n\nThanks.\n\n> diff --git a/ll-merge.c b/ll-merge.c\n> index 6bb3095..9bd3732 100644\n> --- a/ll-merge.c\n> +++ b/ll-merge.c\n> ...\n> @@ -96,14 +102,17 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n> \t\t\t mmfile_t *orig, const char *orig_name,\n> \t\t\t mmfile_t *src1, const char *name1,\n> \t\t\t mmfile_t *src2, const char *name2,\n> -\t\t\t int flag, int marker_size)\n> +\t\t\t const struct ll_merge_options *opts,\n> +\t\t\t int marker_size)\n> {\n> \t/* Use union favor */\n> -\tflag &= ~LL_OPT_FAVOR_MASK;\n> -\tflag |= create_ll_flag(XDL_MERGE_FAVOR_UNION);\n> +\tstruct ll_merge_options o;\n> +\tassert(opts);\n> +\to = *opts;\n> +\to.variant = XDL_MERGE_FAVOR_UNION;\n> \treturn ll_xdl_merge(drv_unused, result, path_unused,\n> \t\t\t orig, NULL, src1, NULL, src2, NULL,\n> -\t\t\t flag, marker_size);\n> +\t\t\t &o, marker_size);\n> \treturn 0;\n\nHmph, two returns...\n\n> @@ -337,15 +348,21 @@ int ll_merge(mmbuffer_t *result_buf,\n> \t mmfile_t *ancestor, const char *ancestor_label,\n> \t mmfile_t *ours, const char *our_label,\n> \t mmfile_t *theirs, const char *their_label,\n> -\t int flag)\n> +\t const struct ll_merge_options *opts)\n> {\n> \tstatic struct git_attr_check check[2];\n> \tconst char *ll_driver_name = NULL;\n> \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n> \tconst struct ll_merge_driver *driver;\n> -\tint virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR;\n> \n> +\tif (!opts) {\n> +\t\tstruct ll_merge_options default_opts = {0};\n> +\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n> +\t\t\t\tours, our_label, theirs, their_label,\n> +\t\t\t\t&default_opts);\n\nFun---expecting tail recursion elimination ;-)?\n\n> +\t}\n> +\n> -\tif (flag & LL_OPT_RENORMALIZE) {\n> +\tif (opts->renormalize) {\n> \t\tnormalize_file(ancestor, path);\n> \t\tnormalize_file(ours, path);\n> \t\tnormalize_file(theirs, path);\n> ...\n> \t\t}\n> \t}\n> \tdriver = find_ll_merge_driver(ll_driver_name);\n\nA tangent, as this comment is not about the \"richer ll-merge\" series.\n\nThe above strikes me that the low level merge driver *ought* to have a say\nin the use of renormalize. For example, ll_merge_binary() may probably\nnot want to have its input renormalized, no?\n"},{"id":"149068","messageId":"7viq2xmipe.fsf@alter.siamese.dyndns.org","threadId":"24840","inReplyTo":"20100826055147.GG9708@burratino","subject":"Re: [PATCH 4/4] merge-recursive: options to ignore whitespace changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-26T16:39:41Z","receivedAt":"2010-08-26T16:39:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> From: Justin Frankel \n>\n> Add support for merging with ignoring line endings (specifically\n> --ignore-space-at-eol) when using recursive merging. This is\n> as a strategy-option, so that you can do:\n\n\"This is as a strategy-option\"? s/is/& done/ perhaps?\n\nOtherwise looks good to me.\n"},{"id":"149113","messageId":"20100827082432.GA4800@burratino","threadId":"24840","inReplyTo":"7viq2xmipe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] merge-recursive: options to ignore whitespace changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-27T08:24:32Z","receivedAt":"2010-08-27T08:24:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> From: Justin Frankel \n>>\n>> Add support for merging with ignoring line endings (specifically\n>> --ignore-space-at-eol) when using recursive merging. This is\n>> as a strategy-option, so that you can do:\n>\n> \"This is as a strategy-option\"? s/is/& done/ perhaps?\n\nSure, that makes it clearer.\n\n> Otherwise looks good to me.\n\nHmph, one embarrasingly sloppy bit on my part:\n\n+ else if (!strcmp(s, \"ignore-space-at-eol\"))\n+ o->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n+ else if (!strcmp(s, \"ignore-space-at-eol\"))\n+ o->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n\nHere's a fixup, plus a quick test while at it.\n\nSigned-off-by: Jonathan Nieder \n---\n merge-recursive.c | 2 -\n t/t3032-merge-recursive-options.sh | 186 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 186 insertions(+), 2 deletions(-)\n create mode 100755 t/t3032-merge-recursive-options.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 944ca19..9b9f97e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1521,8 +1521,6 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->xdl_opts |= XDF_IGNORE_WHITESPACE;\n \telse if (!strcmp(s, \"ignore-space-at-eol\"))\n \t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n-\telse if (!strcmp(s, \"ignore-space-at-eol\"))\n-\t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;\n \telse if (!strcmp(s, \"renormalize\"))\n \t\to->renormalize = 1;\n \telse if (!strcmp(s, \"no-renormalize\"))\ndiff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-options.sh\nnew file mode 100755\nindex 0000000..2293797\n--- /dev/null\n+++ b/t/t3032-merge-recursive-options.sh\n@@ -0,0 +1,186 @@\n+#!/bin/sh\n+\n+test_description='merge-recursive options\n+\n+* [master] Clarify\n+ ! [remote] Remove cruft\n+--\n+ + [remote] Remove cruft\n+* [master] Clarify\n+*+ [remote^] Initial revision\n+* ok 1: setup\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tconflict_hunks () {\n+\t\tsed -n -e \"\n+\t\t\t/^<<<>>>/ b\n+\t\t\tn\n+\t\t\tb inconflict\n+\t\t\" \"$@\"\n+\t} &&\n+\n+\tcat <<-\\EOF >text.txt &&\n+\t Hope, he says, cherishes the soul of him who lives in\n+\t justice and holiness and is the nurse of his age and the\n+\t companion of his journey;--hope which is mightiest to sway\n+\t the restless soul of man.\n+\n+\tHow admirable are his words! And the great blessing of riches, I do\n+\tnot say to every man, but to a good man, is, that he has had no\n+\toccasion to deceive or to defraud others, either intentionally or\n+\tunintentionally; and when he departs to the world below he is not in\n+\tany apprehension about offerings due to the gods or debts which he owes\n+\tto men. Now to this peace of mind the possession of wealth greatly\n+\tcontributes; and therefore I say, that, setting one thing against\n+\tanother, of the many advantages which wealth has to give, to a man of\n+\tsense this is in my opinion the greatest.\n+\n+\tWell said, Cephalus, I replied; but as concerning justice, what is\n+\tit?--to speak the truth and to pay your debts--no more than this? And\n+\teven to this are there not exceptions? Suppose that a friend when in\n+\this right mind has deposited arms with me and he asks for them when he\n+\tis not in his right mind, ought I to give them back to him? No one\n+\twould say that I ought or that I should be right in doing so, any more\n+\tthan they would say that I ought always to speak the truth to one who\n+\tis in his condition.\n+\n+\tYou are quite right, he replied.\n+\n+\tBut then, I said, speaking the truth and paying your debts is not a\n+\tcorrect definition of justice.\n+\n+\tCEPHALUS - SOCRATES - POLEMARCHUS\n+\n+\tQuite correct, Socrates, if Simonides is to be believed, said\n+\tPolemarchus interposing.\n+\n+\tI fear, said Cephalus, that I must go now, for I have to look after the\n+\tsacrifices, and I hand over the argument to Polemarchus and the company.\n+\tEOF\n+\tgit add text.txt &&\n+\ttest_tick &&\n+\tgit commit -m \"Initial revision\" &&\n+\n+\tgit checkout -b remote &&\n+\tsed -e \"\n+\t\t\ts/\\. /\\. /g\n+\t\t\ts/[?] /? /g\n+\t\t\ts/ /\t/g\n+\t\t\ts/--/---/g\n+\t\t\ts/but as concerning/but as con cerning/\n+\t\t\t/CEPHALUS - SOCRATES - POLEMARCHUS/ d\n+\t\t\" text.txt >text.txt+ &&\n+\tmv text.txt+ text.txt &&\n+\tgit commit -a -m \"Remove cruft\" &&\n+\n+\tgit checkout master &&\n+\tsed -e \"\n+\t\t\ts/\\(not in his right mind\\),\\(.*\\)/\\1;\\2Q/\n+\t\t\ts/Quite correct\\(.*\\)/It is too correct\\1Q/\n+\t\t\ts/unintentionally/un intentionally/\n+\t\t\t/un intentionally/ s/$/Q/\n+\t\t\ts/Polemarchus interposing./Polemarchus, interposing.Q/\n+\t\t\t/justice and holiness/ s/$/Q/\n+\t\t\t/pay your debts/ s/$/Q/\n+\t\t\" text.txt | q_to_cr >text.txt+ &&\n+\tmv text.txt+ text.txt &&\n+\tgit commit -a -m \"Clarify\" &&\n+\tgit show-branch --all\n+'\n+\n+test_expect_success 'naive merge fails' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive HEAD^ -- HEAD remote &&\n+\ttest_must_fail git update-index --refresh &&\n+\tgrep \"<<<<<<\" text.txt\n+'\n+\n+test_expect_success '--ignore-space-change makes merge succeed' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-space-change HEAD^ -- HEAD remote\n+'\n+\n+test_expect_success '--ignore-space-change: our w/s-only change wins' '\n+\tq_to_cr <<-\\EOF >expected &&\n+\t justice and holiness and is the nurse of his age and theQ\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-space-change HEAD^ -- HEAD remote &&\n+\tgrep \"justice and holiness\" text.txt >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--ignore-space-change: their real change wins over w/s' '\n+\tcat <<-\\EOF >expected &&\n+\tit?---to speak the truth and to pay your debts---no more than this? And\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-space-change HEAD^ -- HEAD remote &&\n+\tgrep \"pay your debts\" text.txt >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--ignore-space-change: does not ignore new spaces' '\n+\tcat <<-\\EOF >expected1 &&\n+\tWell said, Cephalus, I replied; but as con cerning justice, what is\n+\tEOF\n+\tq_to_cr <<-\\EOF >expected2 &&\n+\tun intentionally; and when he departs to the world below he is not inQ\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-space-change HEAD^ -- HEAD remote &&\n+\tgrep \"Well said\" text.txt >actual1 &&\n+\tgrep \"when he departs\" text.txt >actual2 &&\n+\ttest_cmp expected1 actual1 &&\n+\ttest_cmp expected2 actual2\n+'\n+\n+test_expect_success '--ignore-all-space drops their new spaces' '\n+\tcat <<-\\EOF >expected &&\n+\tWell said, Cephalus, I replied; but as concerning justice, what is\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-all-space HEAD^ -- HEAD remote &&\n+\tgrep \"Well said\" text.txt >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--ignore-all-space keeps our new spaces' '\n+\tq_to_cr <<-\\EOF >expected &&\n+\tun intentionally; and when he departs to the world below he is not inQ\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --ignore-all-space HEAD^ -- HEAD remote &&\n+\tgrep \"when he departs\" text.txt >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--ignore-space-at-eol' '\n+\tq_to_cr <<-\\EOF >expected &&\n+\t<<<<<<< HEAD\n+\tis not in his right mind; ought I to give them back to him? No oneQ\n+\t=======\n+\tis not in his right mind, ought I to give them back to him? No one\n+\t>>>>>>> remote\n+\tEOF\n+\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --ignore-space-at-eol \\\n+\t\t\t\t\t\t HEAD^ -- HEAD remote &&\n+\tconflict_hunks text.txt >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n1.7.2.2\n"},{"id":"159532","messageId":"20110116010841.GA18049@burratino","threadId":"24840","inReplyTo":"7vpqx5mipt.fsf@alter.siamese.dyndns.org","subject":"[PATCH v1.7.4-rc2] ll-merge: simplify opts == NULL case","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-16T01:08:42Z","receivedAt":"2011-01-16T01:08:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> +\tif (!opts) {\n>> +\t\tstruct ll_merge_options default_opts = {0};\n>> +\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n>> +\t\t\t\tours, our_label, theirs, their_label,\n>> +\t\t\t\t&default_opts);\n>\n> Fun---expecting tail recursion elimination ;-)?\n\nFun but not warranted. Let's simplify.\n\n-- 8< --\nSubject: ll-merge: simplify opts == NULL case\n\nAs long as sizeof(struct ll_merge_options) is small, there is not\nmuch reason not to keep a copy of the default merge options in the BSS\nsection. In return, we get clearer code and one less stack frame in\nthe opts == NULL case.\n\nSigned-off-by: Jonathan Nieder \n---\n ll-merge.c | 9 +++------\n 1 files changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 007dd3e..6ce512e 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -351,16 +351,13 @@ int ll_merge(mmbuffer_t *result_buf,\n \t const struct ll_merge_options *opts)\n {\n \tstatic struct git_attr_check check[2];\n+\tstatic const struct ll_merge_options default_opts;\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n \tconst struct ll_merge_driver *driver;\n \n-\tif (!opts) {\n-\t\tstruct ll_merge_options default_opts = {0};\n-\t\treturn ll_merge(result_buf, path, ancestor, ancestor_label,\n-\t\t\t\tours, our_label, theirs, their_label,\n-\t\t\t\t&default_opts);\n-\t}\n+\tif (!opts)\n+\t\topts = &default_opts;\n \n \tif (opts->renormalize) {\n \t\tnormalize_file(ancestor, path);\n-- \n1.7.4.rc2\n"}]}