Initial parsing of %user section as a Header. - #662
Conversation
The thought is that we will reuse the `%grmtools` section parser, but can 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 in the `%grmtools` section.
| String(String, Span), | ||
| Num(u64, Span), | ||
| Bool(bool, Span), | ||
| Array(Vec<UserSectionValue>, Span), |
There was a problem hiding this comment.
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?
|
I think that pretty much covers the approach that I was thinking would largely reuse the code we already have, |
| start_states, | ||
| lex_flags: DEFAULT_LEX_FLAGS, | ||
| expected_missing_tokens: vec![], | ||
| user_section: HashMap::new(), |
There was a problem hiding this comment.
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?
| } | ||
|
|
||
| pub fn parse(&'_ self) -> Result<(FileHeaders, usize), Vec<HeaderError<Span>>> { | ||
| let mut sections_lookup = HashSet::from_iter(["%grmtools", "%user"]); |
There was a problem hiding this comment.
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.
2 participants
This still needs some work, but is an initial attempt at a
%usersection we can parse by reusing the%grmtoolssection parser, then restrict the types within it as a post processing stage.In a subsequent patch we can iterate over the
%usersection and convert it into a nicer to useHashMap. Throwing invalid value errors on the more rust-like constructs available viaHeaderValueThis is also currently missing any way to access the information from the section.