[PATCH 3 of 4] hgmerge: remove obsoleted code, update docs

Steve Borho steve at borho.org
Mon Jan 7 22:19:13 CST 2008


On Mon, 2008-01-07 at 18:10 -0600, Oliver Xymoron wrote:
> On Mon, 2008-01-07 at 15:20 -0600, Steve Borho wrote:
> > +  Example ~/.hgrc:
> > +
> > +    [hgmerge]
> > +    default = kdiff3
> > +    ext.html = myHtmlPlugin
> > +    ext.lib = takemine
> 
> Extension-based is a little limited. The decode/encode filters can take
> patterns. It's a bit odd to have a [merge] and an hgmerge section, if
> we're making this core functionality.

There isn't currently a merge section as far as I know, only ui.merge.
But I have no problem naming the new section just [merge].  Using file
patterns that would look like:

[merge]
default = kdiff3
**.html = myHtmlPlugin
**.lib = takelocal

> > +
> > +  There are two special plug-ins intended for file extension use:
> > +  'takemine' and 'takeother' (with predictable behaviors).
> 
> Hmmm, 'mine' should be 'local' here to be consistent.

Agreed.

> > +hgmerge-plugins::
> > +  The user can override and extend the stock merge plug-ins by adding
> > +  entries to this section.
> > +
> > +  Example ~/.hgrc:
> > +
> > +    [hgmerge-plugins]
> > +    # Override stock plug-in location
> > +    kdiff3.executable = ~/bin/kdiff3
> 
> > +    # Define new plug-in
> > +    myHtmlPlugin.arguments = -m $local $other $base $output
> > +    myHtmlPlugin.priority = 1
> > +    myHtmlPlugin.win.regpath_installpath = Software\FooSoftware\HtmlMerge
> 
> The 'plug-in' terminology isn't great as it makes it sound much more
> substantial than it really is (typically two lines of config). It'd be
> even better if we could get it down to one line in the typical end-user
> case, ie:

The intent was to eventually allow python 'plug-ins', in which case they
would be more substantial.

> mymerge = supermerge -vkm $local $other $base -o $output

the priority and win path settings are going to be pretty rarely used
for user-defined tools. In this example they could have just used:

myHtmlPlugin.arguments = -m $local $other $base $output

> So this might want to be [merge-tools] or [merge-helpers] or maybe we
> even want to combine it with the existing [merge] section too. Then
> someone can just write:
> 
> [merge]
> default = mymerge
> mymerge = supermerge -vkm $local $other $base -o $output

We could get it down this far pretty easily:

[merge]
default = supermerge
supermerge.arguments = -vkm $local $other $base -o $output

> 
> If we can get that down to:
> 
> [merge]
> default = supermerge -vkm $local $other $base -o $output
> 
> ..then we know we can't do any better. I'm not saying this is precisely
> how it should be done, just that we should give some serious thought to
> absolutely minimizing the amount of work the average person will need to
> do to customize their setup. One line is ideal.

We may be able to get at least close to this.

> > +
> > +  executable;;
> > +    Either just the name of the executable or its pathname.
> > +    Default: the plug-in name.
> > +  arguments;;
> > +    The arguments to pass to the tool.
> > +    Default: $base $local $other $output
> > +  priority;;
> > +    The priority in which to evaluate this plug-in.  Default: 0
> 
> You haven't defined what priorities mean.

Good point, I'll add another line here.

> > +  stdout;;
> > +    Should the tool's standard output be used as the merge result.
> > +    Default: False
> > +  check_conflicts;;
> > +    Check whether there are conflicts even though the tool reported
> > +    none. Default: False
> > +  win.regpath_installdir;;
> > +    Specify a pathname in the Windows registry defining the tool's installation
> > +    directory. The format of this option is like this: <key name>\<value name>.
> > +    If the (default) key itself actually holds the value, end the pathname with
> > +    a backslash, so it's clear there is no value name component. 
> > +  win.regpath_installpath;;
> > +    Like the former, except that the registry value is taken to specify the
> > +    installation path of the tool's executable.
> 
> These last two are really confusing.

I'm tempted to only support these fields in the stock tool definitions
and not expose them to the user.  It's simpler for them to just provide
the full path to the app (provided it isn't already in $PATH).

> Also, please delete the old crap in a separate patch.

Will do

-- 
Steve Borho (steve at borho.org)
http://www.borho.org/~steve/steve.asc
Key fingerprint = 2D08 E7CF B624 624C DE1F  E2E4 B0C2 5292 F2C6 2C8C




More information about the Mercurial-devel mailing list