Re: [PATCH 2/3] config: fix case sensitive subsection names on writing

On Wed, Aug 1, 2018 at 3:51 PM Junio C Hamano [off-list ref] wrote:

Stefan Beller [off-list ref] writes:

quoted

A use reported a submodule issue regarding strange case indentation
issues, but it could be boiled down to the following test case:

Perhaps

s/use/user/
s/case indentation issues/section mix-up/

will be fixed in a reroll

quoted

... However we do not have a test for writing out config correctly with
case sensitive subsection names, which is why this went unnoticed in
6ae996f2acf (git_config_set: make use of the config parser's event
stream, 2018-04-09)

s/unnoticed in \(.*04-09)\)/unnoticed when \1 broke it./

This is why I asked if the patch is a "FIX" for an issue introduced
by the cited commit.

I did not check further down the history if it was a recent brakage.

quoted

 static int get_base_var(struct strbuf *name)
 {
+     cf->section_name_old_dot_style = 1;
      for (;;) {
              int c = get_next_char();
              if (cf->eof)

OK, let me rephrase.  The basic parse structure is that

 * upon seeing '[', we call get_base_var(), which stuffs the
   "section" (including subsection, if exists) in the strbuf.

 * get_base_var() upon seeing a space after "[section ", calls
   get_extended_base_var().  This space can never exist in an
   old-style three-level names, where it is spelled as
   "[section.subsection]".  This space cannot exist in two-level
   names, either.  The closing ']' is eaten by this function before
   it returns.

 * get_extended_base_var() grabs the "double quoted" subsection name
   and eats the closing ']' before it returns.

So you set the new bit (section_name_old_dot_style) at the beginning
of get_base_var(), i.e. declare that you assume we are reading old
style, but upon entering get_extended_base_var(), unset it, because
now you know we are parsing a modern style three-level name(s).

Feels quite sensible way to keep track of old/new styles.

When parsing two-level names, old-style bit is set, which we may
need to be careful, thoguh.

I considered setting it only when seeing the dot, but then we'd have
to make sure it is properly initialized.

And *technically* the two level is old style, so I figured it's ok.

quoted

-                     !strncasecmp(cf->var.buf, store->key, store->baselen);
+                     !cmpfn(cf->var.buf, store->key, store->baselen);

OK.  Section names should still be case insensitive (only the case
sensitivity of subsection names is special), but presumably that's
already normalized by the caller so we do not have to worry when we
use strncmp()?  Can we add a test to demonstrate that it works
correctly?

That was already demonstrated (but not tested) in
https://public-inbox.org/git/20180730230443.74416-4-sbeller@google.com/