Re: [GSoC PATCH v2] userdiff: add builtin driver for gitconfig syntax
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Mar 24, 2025, 07:18 UTC
- Message-ID
- <d4c0c9a4-0402-4456-9fa0-3102b5bcc3dc@kdbg.org>
- In-Reply-To
- <20250324021101.7483-1-lucasseikioshiro@gmail.com>
Am 24.03.25 um 03:11 schrieb Lucas Seiki Oshiro:
Show 15 quoted lines
> From Documentation/config.adoc: > > Add a new builtin driver for gitconfig files, where: > > - the funcname regular expression matches sections and subsections, > i. e. the pattern [SECTION] or [SECTION "SUBSECTION"], where the > section is composed by alphanumeric numbers, `-` and `.`, and > subsection names may be composed by any characters; > > - word_regex is more permissive than the syntax specification, matching > any word with one or more non-whitespace characters without checking > if it is a valid variable name or value. > > A more detailed description on the format of gitconfig syntax can be > seen by running `git show cfd409:Documentation/config.txt`.
Can we please have a more recent reference? The difference of config.txt here and config.adoc above is very surprising.
Show 6 quoted lines
> Also add tests for the new userdiff driver. These files define sections > and subsections, with and without indentation. > > Helped-by: Patrick Steinhardt <ps@pks.im> > Helped-by: D. Ben Knoble <ben.knoble@gmail.com> > Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Thank you for your contribution.
The file format of .git/config files isn't specific to .git/config; it's called "ini-file" and is already very old. Wouldn't it make sense to generalize the format? It would be just a matter of choosing a different name; the regular expressions would not have to change.
Show 12 quoted lines
> diff --git a/t/t4018/gitconfig-section b/t/t4018/gitconfig-section > new file mode 100644 > index 0000000000..18c85eb613 > --- /dev/null > +++ b/t/t4018/gitconfig-section > @@ -0,0 +1,6 @@ > +[RIGHT] > + # comment > + ; comment > + name = value > + ChangeMe > +
This could test two sections in a row and ensure that the later one is chosen.
You have now managed to avoid the "No newline at end of file", but have added a blank line instead. Not a big deal, but unconventional.
Show 26 quoted lines
> diff --git a/t/t4018/gitconfig-section-noindent b/t/t4018/gitconfig-section-noindent > new file mode 100644 > index 0000000000..5c58a7ac92 > --- /dev/null > +++ b/t/t4018/gitconfig-section-noindent > @@ -0,0 +1,6 @@ > +[RIGHT] > +# comment > +; comment > +name = value > +ChangeMe > + > diff --git a/t/t4018/gitconfig-subsection b/t/t4018/gitconfig-subsection > new file mode 100644 > index 0000000000..569be04a32 > --- /dev/null > +++ b/t/t4018/gitconfig-subsection > @@ -0,0 +1,8 @@ > +[LEFT] > + > +[LEFT "RIGHT"] > + # comment > + ; comment > + name = value > + ChangeMe > +
This could test two sub-sections in a row and ensure that the later one is chosen.
What happens if there is an *indented* header after the "RIGHT" one? Should it be chosen or not? Can this happen in a valid file?
Show 24 quoted lines
> diff --git a/t/t4018/gitconfig-subsection-noindent b/t/t4018/gitconfig-subsection-noindent
> new file mode 100644
> index 0000000000..85c5074f47
> --- /dev/null
> +++ b/t/t4018/gitconfig-subsection-noindent
> @@ -0,0 +1,8 @@
> +[LEFT]
> +
> +[LEFT "RIGHT"]
> +# comment
> +; comment
> +name = value
> +ChangeMe
> +
> diff --git a/userdiff.c b/userdiff.c
> index 340c4eb4f7..5bbcc2b690 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -198,6 +198,10 @@ IPATTERN("fountain",
> "^((\\.[^.]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$",
> /* -- */
> "[^ \t-]+"),
> +PATTERNS("gitconfig",
> + "^\\[[a-zA-Z0-9]+\\]|\\[[a-zA-Z0-9]+[ \t]+\".+\"\\]$",The regular expression can assume that the syntax of the processed file is correct. For example,
[!not a section!]
cannot be a section header and will not occur in a valid file. Or can it?
Therefore, it would be sufficient to just take everything after the '[' at the beginning of the line without further inspection.
Furthermore, a valid file can look like this:
[section] key = value another_key = more values
but your pattern would not pick up this header, because it insists in that the closing bracket is at the end of the line. Having a test for this case would be great.
Show 5 quoted lines
> + /* -- */
> + "[^ \t]+"),
> PATTERNS("golang",
> /* Functions */
> "^[ \t]*(func[ \t]*.*(\\{[ \t]*)?)\n"-- Hannes