regex/token name separator for regexes that end with trailing spaces

[ratmice]

This is an issue that affects current grmtools which was brought to light when considering the feature request in #597 it seemed like it would be good to split this out into it's own issue than discuss it there.

If we take the following lex file (foo.l) which ends in a trailing space, current grmtools accepts it,
but it seems like it should probably reject it.

%%
\ \ "weirdness"
\n ;

The thing is the trailing space in \ \ is part of a regex, but it gets treated as the space separator that separates regex from the token name. Then it seems like perhaps it gets trimmed off of the regular expression before being passed to regex.

$ echo " " | cargo run --quiet -p lrlex ./foo.l -
weirdness
$ echo "  " | cargo run --quiet -p lrlex ./foo.l -
weirdness
weirdness

I would have expected the echo with two spaces to match a single weirdness token, and the one with a single token to not match.

The following regex is rejected, but by the regex crate, I assume it is just sending the '' token alone to regex.

%%
\ "rejectedByRegex"
\n ;

[ltratt]

I must admit that I deeply regret not making the syntax "regex" "id"! Still, we are where we are.

How about this as an algorithm: lrlex should be taught about \, specifically that \\ is a literal \ and anything else is \<char>. So your first example above would then be regex="\ \ " id="weirdness" on the basis that I think the implicit contract is "zero or more spaces between the regex and the open quote char".

I don't think we're going to get a perfect solution here, because we (I!) are starting from the wrong place, but we can probably lessen the problems far enough that we don't have to lose too much sleep over it!

[ratmice]

Seems reasonable, I'm not sure if extra trailing whitespace would affect the regex crate, like
if "\ \ " is treated like "\ \ " or three spaces. Depending on how that behaves we might also need to define a trimming behavior that trims the non-escaped spaces?

If I had druthers and was picking a format I'd pick some unicode separator, probably regex⇒"id", or regex▻ "id", or even using some unicode quote characters like «regex» "id".
You would just have to encode whatever character(s) chosen into a regex using unicode codepoint but that feels like it'd be a worthwhile trade.

[ltratt]

I think we should try and fix what we can in the current format, then maybe consider adding a "new" format, with a saner syntax, as an optional variant.

[ratmice]

So, I did notice one thing with your proposed interpretation,
This is somewhat impacted by #540 where double quotes are a valid part of regex.

In all likelyhood "\ \ "weirdness" is a bug where it is intended to be interpreted as id="weirdness", but technically it should be interpreted as regex="\ \ weirdness" id="" then error because there is no id.

If we treat \ \ "weirdness" as regex="\ \ " id="weirdness",
We should make sure that we're also treating: \ \ "partOfRegex"\ "foo" as regex="\ \ partOfRegex\ " id="foo".

So I think that more than zero or more spaces, the rule might be last escaped or non-escaped space before a valid id?

[ltratt]

I don't think there's any way we can make every interpretation make sense: the starting syntax is just wrong (and that's my fault). So I'd say: pick your poison :) There won't be a perfect answer.

[ratmice]

I probably take some share of the blame, I think I overhauled the algorithm when adding support for escaped spaces in the regex, but really I blame lex for it's choice of format and allowance of everything under the sun in regex (which has only grown in the ensuing years).

Anyhow before we try and fix this, I think it'd be good to decide whether we want to try and support #597 (which forces us to find the separating space by walking forwards through the regex, and do more regex parsing) or instead keep the current mechanism of finding the space using rfind.

grmtools/lrlex/src/lib/parser.rs

Line 439 in 12067d6

let rspace = match line.rfind(|ch: char| RE_SPACE_SEP.is_match(ch.encode_utf8(&mut cbuf))) {

My opinion is to keep the rfind because it's so simple, and when adding spaces those tokens still can't easily be embedded in lrpar.

[ratmice]

FWIW, I did check regex and it does treat whitespace significantly so " " is a valid regex, so we will need to trim non-escaped whitespace.

[ltratt]

My opinion is to keep the rfind because it's so simple, and when adding spaces those tokens still can't easily be embedded in lrpar.

I agree. I think this is a far better rule (given our starting point): it's much easier for users to understand.