git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [GSoC PATCH 1/2] userdiff: add builtin driver for gitconfig syntax
Date: Thu, 20 Mar 2025 09:38:17 +0100	[thread overview]
Message-ID: <Z9vT-dqBzXxzXnlU@pks.im> (raw)
In-Reply-To: <20250319172016.2115-2-lucasseikioshiro@gmail.com>

On Wed, Mar 19, 2025 at 02:20:15PM -0300, Lucas Seiki Oshiro wrote:
> From Documentation/config.adoc:
> 
> """
> The file consists of sections and variables. A section begins with
> the name of the section in square brackets and continues until the next
> section begins. Section names are case-insensitive. Only alphanumeric
> characters, `-` and `.` are allowed in section names. Each variable
> must belong to some section, which means that there must be a section
> header before the first setting of a variable.
> 
> [...]
> 
> Subsection names are case sensitive and can contain any characters except
> newline and the null byte.
> 
> The variable names are case-insensitive, allow only alphanumeric characters
> and `-`, and must start with an alphabetic character.
> """

I don't think it's necessary to quote this whole paragraph here, as most
of us should be quite familiar with its format. I'd rather summarize the
info a bit and explain how we can use the userdiff patterns for the
general structure of the config. And in case there are any subtleties in
the format it may make sense to specifically point out those instead of
quoting the whole manual.

> Then, 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;

Okay, makes sense.

> - word_regex is more permissive, matching any word with one or more
>   non-whitespace characters.

It would be nice to provide context _why_ it is more permissive and what
the effect is.

The order of the commit message in our project is typically a bit
different than what you have here: we first explain the actual problem
that we aim to solve before discussing how you solve it.

The code change itself looks sensible to me.

Patrick

  reply	other threads:[~2025-03-20  8:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-19 17:20 [GSoC PATCH 0/2] add userdiff driver for gitconfig Lucas Seiki Oshiro
2025-03-19 17:20 ` [GSoC PATCH 1/2] userdiff: add builtin driver for gitconfig syntax Lucas Seiki Oshiro
2025-03-20  8:38   ` Patrick Steinhardt [this message]
2025-03-21  2:11     ` D. Ben Knoble
2025-03-19 17:20 ` [GSoC PATCH 2/2] t4018: add tests for gitconfig in userdiff Lucas Seiki Oshiro
2025-03-20  8:38   ` Patrick Steinhardt
2025-03-23 16:08     ` Lucas Seiki Oshiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z9vT-dqBzXxzXnlU@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=lucasseikioshiro@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).