Re: [PATCH 2/3] config: fix case sensitive subsection names on writing
- From
Stefan Beller <sbeller@google.com>
- Date
- Aug 3, 2018, 00:30 UTC
- Message-ID
- <CAGZ79kbeUHVQmfTFkoGEM617iVa_3mCsLjpYGoEebqxNKAM_xA@mail.gmail.com>
- In-Reply-To
- <xmqqva8t4s63.fsf@gitster-ct.c.googlers.com>
On Wed, Aug 1, 2018 at 3:51 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> > Stefan Beller <sbeller@google.com> writes: > > > 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
Show 10 quoted lines
> > > ... 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.
Show 31 quoted lines
> > 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.
Show 8 quoted lines
> > - !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/