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

[Martin Geisler]

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.

> 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.

> 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?

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

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.

> +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.

-- 
Martin Geisler