Add crate name to grmtools section entries by ratmice · Pull Request #664 · softdevteam/grmtools

ratmice · 2026-08-25T18:15:12Z

So here is a first foray into adding a default "tool name" for %grmtools section entries,
This turned out a bit more complex than I had thought, for one it seemed that since lex_flags largely get passed
directly into the regex crate, we should name those regex.foo rather than grmtools.foo.

I named this "tool name" because there is already a thing we call namespace in this area, and that like YaccKind:: and the like.

ratmice · 2026-08-25T18:16:49Z

+    let mut map = HashMap::new();
+    let grmtools = [
+        "yacckind",
+        "lexerkind",


grmtools as the notion of a "tool name" is kind of awkward.

I was wondering if it might be better to have lrpar.yacckind and lrlex.lexerkind
instead of grmtools.* for all the crates?

Not sure if it is better to have uniformity across the crates, or indication of who uses it.

Rather than "tool name", maybe I'll try leaning into "crate name", in which case lrpar/lrlex make sense.
Anyhow, I'm going to try that since it's the most consistent naming scheme i've thought of.

Not too worried if we end up wanting to go back to grmtools.* it's not hard to change.

ratmice · 2026-08-25T18:19:12Z

I guess I should mention this just adds default "tool names" to the existing entries.
It doesn't actually allow any user specified values yet.

ratmice · 2026-08-25T18:21:10Z

 static RE_DIGITS: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^[0-9]+").unwrap());
 static RE_STRING: LazyLock<Regex> = LazyLock::new(|| Regex::new(r#"^\"(\\.|[^"\\])*\""#).unwrap());

+static DEFAULT_TOOLNAME: LazyLock<HashMap<&'static str, &'static str>> = LazyLock::new(|| {


would target name be better than tool name?

ratmice · 2026-08-25T21:46:23Z

            .parse()
            .map_err(|mut errs| errs.drain(..).map(|e| e.into()).collect::<Vec<_>>())?;
-        if let Some(HeaderValue(_, yk_val)) = header.get("grmtools.yacckind") {
+        if let Some(HeaderValue(_, yk_val)) = header.get("lrpar.yacckind") {


So, about the time I got done I remembered that YaccKind is actually defined in cfgrammar So i'm not sure if that isn't going to be more confusing than it is helpful?

If we want to use the crate names then we could just call this cfgrammar.yacckind?

Fair enough, I'm not super fond of having multiple prefixes for parser related things, but it does reflect the fact that the consumer of the field is cfgrammar.

I agree that it does expose a bit more internalness to the user than I would ideally like, but it has the virtue of consistency. Still, this is one of those things that we might change our minds on once we've sat with it for a while!

ltratt · 2026-08-26T07:51:48Z

+| `lrlex.lexerkind`                   | [LexerKind](lexcompatibility.md#lexerkinds) | &cross;  |
+| `lrlex.posix_escapes`[^†]           | bool      | &cross;  |
+| `lrlex.allow_wholeline_comments`[^‡] | bool      | &cross;  |
+| `regex.case_insensitive`            | bool      | &cross;  |


The regex prefix hadn't occurred to me. I think I like it!

ltratt · 2026-08-26T07:54:36Z

First thoughts: this looks like it works? You've raised a really good question in the sense of "what's the right prefix?" I hadn't thought about crate names, but so far this looks like it is probably better than grmtools.. Indeed, perhaps we can even elevate it to a "rule" along the lines of "prefixes are derived from published crates on crates.io; user-defined headers are prefixed with crate." so that there's always a way for the user to add guaranteed-non-conflicting headers of their own?

ratmice · 2026-08-26T08:17:40Z

Yeah, as far as working it at least passes cargo test. Now that I'm waking up I think it likely I missed something in nimbleparse I'll check if there the build bot script would have caught anything, and have a look through the nimbleparse code.

ratmice · 2026-08-26T09:22:12Z

Running through the .buildbot.sh, all except the lrpar.recoverer field were being caught by the script, however since the recoverer field has a default value it would fallback to that. I went looked through all uses of "recoverer" in the rest of the repo too just in case this was a problem outside nimbleparse.

ratmice · 2026-08-26T09:49:50Z

So I think this one is ready in the sense that I can probably proceed to the next step of relaxing the unused key checks for unrecognized crate names, to allow user defined keys.

I'm happy to let this one simmer while we think about it though.
I'll probably build a second branch with these commits as it's starting point.

ltratt · 2026-08-26T10:17:11Z

What are your initial thoughts on the design?

ratmice · 2026-08-26T10:28:25Z

Overall I'm pretty happy with how it has turned out, I like that the regex fields now all point to the regex crate explicitly.
I'm not too bothered by the cfgrammar vs lrpar prefix.

If we keep the current behavior where the library figures out unprefixed items perhaps people can just continue using yacckind and not have to worry about lrpar vs cfgrammar prefixes for that.

What I mean is: with the unused key check, we can keep checking that keys prefixed with [lrpar,regex,cfgrammar,lrlex] are all used. Anything with without a prefix is used. Any other prefix is used.

I guess what I'm thinking is we really don't seem forced to deprecate yacckind in this process, we can reserve the unprefixed keys only for keys where we can infer the crate name.

Edit: Besides that I also like that the crate names gives us a way to avoid conflicting keys like you've mentioned.
And nobody is hamstrung in their ability to add keys in the future.

Edit2: I guess to summarize overall I like it, and I wonder whether the downside of separating the key prefix for cfgrammar/lrpar is actually that much of a burden in practice if we/users can continue using yacckind and inferring the crate prefix.

ltratt · 2026-08-26T10:56:32Z

Yes, for backwards compatibility we should definitely allow yacckind (etc) to be kept as-is in user's grammars for now and we should IMHO automatically transform it to cfgrammar.yacckind as if that's what the user wrote in the first place. [At some point we could warn that their unprefixed use is deprecated and one day after that perhaps remove support -- but I agree that we don't have any compelling need to do that quickly and maybe we will never will. I suppose if/when someone introduces a crate called yacckind!]

Summary: I think we should move ahead with this approach.

ratmice · 2026-08-26T11:01:07Z

Actually it's not even a problem if someone makes a yacckind crate, because of the yacckind. allows us to disambiguate the crate prefix from the unprefixed field. As long as we refrain from using dot in keys in ways that don't correlate to a crate name.

Edit: I mean, it would be confusing, but there is not actually any conflict, we can differentiate the two syntactically.

ratmice · 2026-08-26T11:01:34Z

I went ahead and unmarked as draft then.

ltratt · 2026-08-26T11:19:19Z

Actually it's not even a problem if someone makes a yacckind crate, because of the yacckind. allows us to disambiguate the crate prefix from the unprefixed field. As long as we refrain from using dot in keys in ways that don't correlate to a crate name.

True!

In terms of the code, I'm already happy with this. Could/should we merge (subject to squashing)?

This allows entries in the grmtools section to specified including `crate_name.entry_name:` For backwards compatibility it infers the crate name from the known entries. This cannot currently be used by crates outside grmtools without error. In the future this may be relaxed.

ratmice · 2026-08-26T11:37:31Z

Sounds good to me, I went ahead and squashed it and rewrote the commit message.

ratmice commented Aug 25, 2026

View reviewed changes

ratmice assigned ltratt Aug 25, 2026

ratmice commented Aug 25, 2026

View reviewed changes

ltratt reviewed Aug 26, 2026

View reviewed changes

ratmice changed the title ~~Add tool name to grmtools section entries~~ Add crate name to grmtools section entries Aug 26, 2026

ratmice marked this pull request as ready for review August 26, 2026 11:01

ratmice force-pushed the grmtools_section_user_entries branch from 7807176 to 15cd813 Compare August 26, 2026 11:36

ltratt added this pull request to the merge queue Aug 26, 2026

Merged via the queue into softdevteam:master with commit 5e9d621 Aug 26, 2026
2 checks passed

ratmice deleted the grmtools_section_user_entries branch August 26, 2026 11:45

Uh oh!

Conversation

ratmice commented Aug 25, 2026

Uh oh!

ratmice Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ratmice Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

ratmice commented Aug 25, 2026

Uh oh!

ratmice Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

ratmice Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

ltratt Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

ratmice Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

ltratt Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

ltratt Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

ltratt commented Aug 26, 2026

Uh oh!

ratmice commented Aug 26, 2026

Uh oh!

ratmice commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ratmice commented Aug 26, 2026

Uh oh!

ltratt commented Aug 26, 2026

Uh oh!

ratmice commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ltratt commented Aug 26, 2026

Uh oh!

ratmice commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ratmice commented Aug 26, 2026

Uh oh!

ltratt commented Aug 26, 2026

Uh oh!

ratmice commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ratmice Aug 25, 2026 •

edited

Loading

ratmice commented Aug 26, 2026 •

edited

Loading

ratmice commented Aug 26, 2026 •

edited

Loading

ratmice commented Aug 26, 2026 •

edited

Loading

ratmice commented Aug 26, 2026 •

edited

Loading