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?