[augeas-devel] [PATCH] svn: lens and tests for /etc/subversion/{config, servers}

[David Lutterkort]

On Mon, 2009-08-17 at 18:34 +0200, Marc Fournier wrote:
> Signed-off-by: Marc Fournier <marc.fournier at camptocamp.com>
> ---
>  lenses/subversion.aug            |   48 +++++++
>  lenses/tests/test_subversion.aug |  286 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 334 insertions(+), 0 deletions(-)
>  create mode 100644 lenses/subversion.aug
>  create mode 100644 lenses/tests/test_subversion.aug

Thanks for doing this. I have a few wishes ;)

> diff --git a/lenses/tests/test_subversion.aug b/lenses/tests/test_subversion.aug
> new file mode 100644
> index 0000000..1855ac3
> --- /dev/null
> +++ b/lenses/tests/test_subversion.aug
> @@ -0,0 +1,286 @@
> +module Test_subversion =
> +

The test almost exclusively tests that comments are parsed properly; can
you delete most of the comments, and uncomment as many settings as
possible so that the test shows that we handle all those directives
properly ?

It would also be nice to split some of the entries into smaller ones,
e.g.:

> +# password-stores = gnome-keyring,kwallet

How about parsing that into

  { "password-stores" { "gnome-keyring" } { "kwallet" } }

> +# global-ignores = *.o *.lo *.la *.al .libs *.so *.so.[0-9]* *.a *.pyc *.pyo
> +#   *.rej *~ #*# .#* .*.swp .DS_Store

This would also be more useful if it were split into its components,
i.e. something like

  { "global-ignores"
    { "glob" = "*.o" }
    { "glob" = "*.lo" }
    ...
    { "glob" = ".DS_Store" } }

> +### Section for configuring automatic properties.
> +[auto-props]
> +### The format of the entries is:
> +###   file-name-pattern = propname[=value][;propname[=value]...]
> +### The file-name-pattern can contain wildcards (such as '*' and
> +### '?').  All entries which match (case-insensitively) will be
> +### applied to the file.  Note that auto-props functionality
> +### must be enabled, which is typically done by setting the
> +### 'enable-auto-props' option.
> +# *.c = svn:eol-style=native
> +# *.cpp = svn:eol-style=native
> +# *.h = svn:eol-style=native
> +# *.dsp = svn:eol-style=CRLF
> +# *.dsw = svn:eol-style=CRLF
> +# *.sh = svn:eol-style=native;svn:executable
> +# *.txt = svn:eol-style=native
> +# *.png = svn:mime-type=image/png
> +# *.jpg = svn:mime-type=image/jpeg
> +# Makefile = svn:eol-style=native

This section should be handled extra-special, putting a glob into the
label of a node makes queries later on very awkward, since you need to
escape some special characters, e.g. '*'. Also, the format for each line
explains how the RHS can be deconstructed further. IOW, I'd prefer the
following tree structure for this section:

        { "auto-props"
          (* Line '*.c = svn:eol-style=native' *)
          { "prop"
            { "glob" = "*.c" }
            { "eol-style" = "native" } }
          ...
          (* Line '*.sh = svn:eol-style=native;svn:executable' *)
          { "prop"
            { "glob" = ".sh" }
            { "eol-style" = "native" }
            { "executable" } } }

I am not sure if with all this the Inifile module will be all that
useful, or if it would be easier to write the lens w/o using Inifile.

David