git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/4] ll-merge: replace flag argument with options struct

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 26, 2010, 16:39 UTC
Message-ID
<7vpqx5mipt.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20100826054953.GE9708@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 14 quoted lines
> Keeping track of the flag bits is proving more trouble than it's
> worth.  Instead, use a pointer to an options struct like most similar
> APIs do.
>
> Callers with no special requests can pass NULL to request the default
> options.
>
> Cc: Bert Wesarg <bert.wesarg@googlemail.com>
> Cc: Avery Pennarun <apenwarr@gmail.com>
> Helped-by: Justin Frankel <justin@cockos.com>
> Helped-by: Bert Wesarg <bert.wesarg@googlemail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This time, with updated documentation.
Thanks.
Show 25 quoted lines
> diff --git a/ll-merge.c b/ll-merge.c
> index 6bb3095..9bd3732 100644
> --- a/ll-merge.c
> +++ b/ll-merge.c
> ...
> @@ -96,14 +102,17 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
>  			  mmfile_t *orig, const char *orig_name,
>  			  mmfile_t *src1, const char *name1,
>  			  mmfile_t *src2, const char *name2,
> -			  int flag, int marker_size)
> +			  const struct ll_merge_options *opts,
> +			  int marker_size)
>  {
>  	/* Use union favor */
> -	flag &= ~LL_OPT_FAVOR_MASK;
> -	flag |= create_ll_flag(XDL_MERGE_FAVOR_UNION);
> +	struct ll_merge_options o;
> +	assert(opts);
> +	o = *opts;
> +	o.variant = XDL_MERGE_FAVOR_UNION;
>  	return ll_xdl_merge(drv_unused, result, path_unused,
>  			    orig, NULL, src1, NULL, src2, NULL,
> -			    flag, marker_size);
> +			    &o, marker_size);
>  	return 0;
Hmph, two returns...
Show 18 quoted lines
> @@ -337,15 +348,21 @@ int ll_merge(mmbuffer_t *result_buf,
>  	     mmfile_t *ancestor, const char *ancestor_label,
>  	     mmfile_t *ours, const char *our_label,
>  	     mmfile_t *theirs, const char *their_label,
> -	     int flag)
> +	     const struct ll_merge_options *opts)
>  {
>  	static struct git_attr_check check[2];
>  	const char *ll_driver_name = NULL;
>  	int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
>  	const struct ll_merge_driver *driver;
> -	int virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR;
>  
> +	if (!opts) {
> +		struct ll_merge_options default_opts = {0};
> +		return ll_merge(result_buf, path, ancestor, ancestor_label,
> +				ours, our_label, theirs, their_label,
> +				&default_opts);
Fun---expecting tail recursion elimination ;-)?
Show 11 quoted lines
> +	}
> +
> -	if (flag & LL_OPT_RENORMALIZE) {
> +	if (opts->renormalize) {
>  		normalize_file(ancestor, path);
>  		normalize_file(ours, path);
>  		normalize_file(theirs, path);
> ...
>  		}
>  	}
>  	driver = find_ll_merge_driver(ll_driver_name);
A tangent, as this comment is not about the "richer ll-merge" series.

The above strikes me that the low level merge driver *ought* to have a say in the use of renormalize. For example, ll_merge_binary() may probably not want to have its input renormalized, no?

Previous: Jonathan NiederNext: Jonathan Nieder
Message 18 of 23 in “git-merge: ignore space support”
  1. git-merge: ignore space supportJustin Frankel, Aug 23, 2010
  2. Jonathan NiederAug 24, 2010
  3. merge-recursive: expose merge options for builtin mergeJonathan Nieder, Aug 24, 2010
  4. Junio C HamanoAug 24, 2010
  5. Jonathan NiederAug 25, 2010
  6. Justin FrankelAug 24, 2010
  7. Jonathan NiederAug 25, 2010
  8. Bert WesargAug 25, 2010
  9. Justin FrankelAug 25, 2010
  10. Junio C HamanoAug 25, 2010
  11. Justin FrankelAug 25, 2010
  12. Junio C HamanoAug 24, 2010
  13. Bert WesargAug 24, 2010
  14. Jonathan NiederAug 25, 2010
  15. 0/4 git-merge: ignore space supportJonathan Nieder, Aug 26, 2010
  16. 1/4 merge-recursive: expose merge options for builtin mergeJonathan Nieder, Aug 26, 2010
  17. 2/4 ll-merge: replace flag argument with options structJonathan Nieder, Aug 26, 2010
  18. Junio C HamanoAug 26, 2010
  19. ll-merge: simplify opts == NULL caseJonathan Nieder, Jan 16, 2011
  20. 3/4 merge-recursive --patienceJonathan Nieder, Aug 26, 2010
  21. 4/4 merge-recursive: options to ignore whitespace changesJonathan Nieder, Aug 26, 2010
  22. Junio C HamanoAug 26, 2010
  23. Jonathan NiederAug 27, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.