[PATCH] eol: support alternate location for .hgeol file (issue3975)

[Jorge Acereda]

On Jul 11, 2013, at 4:01 PM, Martin Geisler wrote:

> Jorge Acereda <jacereda at brainstorm.es> writes:
> 
>> # HG changeset patch
>> # User Jorge Acereda <jacereda at brainstorm.es>
>> # Date 1373469140 -7200
>> # Branch stable
>> # Node ID 3487e904f78164e70cf91552419e1ecf235d8daf
>> # Parent  fbdac607bff3dfc0056268ca77e524a5ddd4f665
>> eol: support alternate location for .hgeol file (issue3975)
> 
> I'm still not completely convinced that this is a good idea, but here
> are some review comments on your patch.

Nor am I, if you have a good solution for this scenario I'd be glad to hear it.

Basically I have 80 subrepos and I don't want to maintain 80 .hgeol files.


> 
>> Adds a 'config' setting to the [eol] section that can be used to
>> specify an alternate location for the .hgeol file. This can help when
>> dealing with lots of subrepos.
>> 
>> When both the .hgeol and the setting are present, .hgeol takes
>> precedence.
> 
> You would need to add a test for this. You should be able to find a
> test-eol.t file where you can add the necessary test.

I'll do if there's a chance this will be accepted.

> 
>> diff -r fbdac607bff3 -r 3487e904f781 hgext/eol.py
>> --- a/hgext/eol.py	Mon Jul 01 18:07:33 2013 -0500
>> +++ b/hgext/eol.py	Wed Jul 10 17:12:20 2013 +0200
>> @@ -6,8 +6,20 @@
>> Unix/Mac, thereby letting everybody use their OS native line endings.
>> 
>> The extension reads its configuration from a versioned ``.hgeol``
>> -configuration file found in the root of the working copy. The
>> -``.hgeol`` file use the same syntax as all other Mercurial
>> +configuration file found in the root of the working copy.
>> +
>> +An alternate location for the ``.hgeol`` configuration file can be
>> +specified in a ``config`` setting in the ``[eol]`` section of your
>> +``hgrc``. This can be useful when dealing with lots of subrepos. When
>> +the ``.hgeol`` file is present in a repository, it will take
>> +precedence over the ``config`` setting.
> 
> I think 'config' is too generic -- what about using 'eolpath' instead?

eol.eolpath seems a bit redundant. 'configpath'?

> 
> Falling back to the configured path feels backwards: when configure
> something I expect it to override the normal behavior.

You might want to keep 78 subrepos sharing the same hgeol and 2 subrepos might have special needs that can be defined via their own .hgeol.


> Maybe one could instead read both files (if they exist)? So by default
> Mercurial reads no file, when the eol extension is enabled it reads
> <repo>/.hgeol by default and with your setting you can extend the list
> of files read. That somehow sounds better to my ears.

Would be cool to specify something like:

[eol]
configpaths = ~/sharedcfg/hgeol .hgeol

to specify that both files should be read, but I'm not familiar with the .ini syntax. How do you specify a list in a .ini?

> 
>> +Example ``.hgrc`` file specifying the location of the ``.hgeol`` file::
>> +
>> +  [eol]
>> +  config = ~/shared-cfg/hgeol
>> +
>> +The ``.hgeol`` file use the same syntax as all other Mercurial
>> configuration files. It uses two sections, ``[patterns]`` and
>> ``[repository]``.
>> 
>> @@ -207,7 +219,12 @@
>>                 if node is None:
>>                     # Cannot use workingctx.data() since it would load
>>                     # and cache the filters before we configure them.
>> -                    data = repo.wfile('.hgeol').read()
>> +                    try:
>> +                        f = repo.wfile('.hgeol')
>> +                    except IOError:
>> +                        f = open(util.expandpath(ui.config('eol', 'config')))
> 
> I guess this will read a relative path relative to the current working
> directory? It should probably be changed to be relative to the
> repository root.


I was thinking only absolute paths would be useful. Is there any utility to open a path relative to the root?


> 
> -- 
> Martin Geisler