Initial parsing of %user section as a Header.

[ratmice]

This still needs some work, but is an initial attempt at a %user section we can parse by reusing the %grmtools section parser, then restrict the types within it as a post processing stage.

In a subsequent patch we can iterate over the %user section and convert it into a nicer to use HashMap. Throwing invalid value errors on the more rust-like constructs available via HeaderValue

This is also currently missing any way to access the information from the section.

[ratmice]

I wasn't sure if we wanted to go super simple e.g. HashMap<String, String>, or a serde_json::Value inspired enum like this, I went with the latter for now because it was easy enough to do, and allows us to add types later if we need?

[ltratt]

I think this is the right way to go.

[ratmice]

I think that pretty much covers the approach that I was thinking would largely reuse the code we already have,
I did a cargo semver-checks --exclude lrpar-tests to double check that there is no semver bump needed.

[ratmice]

This actually seems like it could be objectionable, unlike GrammarAST which is never constructed from parser codegen. CTLexerBuilder is going to call this from_rules function from codegen. So this is pretty much a useless allocation from that perspective.

Maybe it should be an Option<...> or somewhere else entirely?

[ltratt]

I think it's fine: in practise HashMap won't malloc until the first entry goes in it.

[ratmice]

I suppose I was thinking about the dotted keys vs named sections
Technically it seems like we could have both a named section and a declaration of the same name,
giving users the ability to specify the name of their section (via sections_lookup here rather than hard coding %user

We could have both:

%foo {
  // The user specified a foo section
}

%foo later-grmtools-added-a-foo-declaration

The commitment would need to make is that we agree not to add sections other than %grmtools.
There I suppose is an obscure conflict in that it is assuming that the %foo declaration is not starting with a {, at the very beginning of the file at least.

This would reduce some duplication of the tool name

%user {
    nimbleparse_lsp.input_file_extension: ".foo"
    nimbleparse_lsp.grammar_path: "./foo.y"
}

could turn into:

%nimbleparse_lsp {
   input_file_extension: "foo",
   grammar_path: "./foo.y",
}

Actually it's probably a bit more complex than just adding it here, we'd need to ignore it for cases like nimbleparse which aren't expecting/passing any additional section_lookup entries. The approach taken in this patch, definitely eliminates that complexity.

[ltratt]

This raises another possibility (off the top of my head: not properly thought through etc etc!). Perhaps we only need a %grmtools section and we namespace all directives in there (accepting, for backwards compatibility reasons, that not everything will have a grmtools. prefix at first).

[ratmice]

I think there is actually still a bit of a reason to have separate directives in that we can check that each declaration in the %grmtools section is used easily, while it's more difficult for the %user section (because we aren't documenting/exposing the markmap that allows marking them as used)

Well I guess check that the grmtools. prefixed items are used only, and filter out other prefixes in the unused check, we'd have to split the . during the filter (only filtering out items with a non-grmtools non-empty prefix).

[ratmice]

The other thing I would say is that as it is currently written, with the hashmap like API, and the string grmtools.yacc_kind being a different key than yacckind, it's definitely going to double a lot of checks in the codebase while we still have legacy entries without the namespace. It seems like it might be quite a bit.

I don't know if trying to change the way that lookup is handled to use a default prefix/search defaulting to using a grmtools namespace would be viable. But that might avoid adding a second lookup including the namespace.