Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: allowing ignoring case in tokens #196

Closed
gymore-io opened this issue Jan 24, 2021 · 4 comments
Closed

Feature request: allowing ignoring case in tokens #196

gymore-io opened this issue Jan 24, 2021 · 4 comments

Comments

@gymore-io
Copy link
Contributor

Being able to do something like that would be neat.

#[token("hello", ignore_case = true)
Hello,

Does an equivalent to this already exist? Would it be a good first issue? (I would like to try and implement it :) )

@maciejhirsz
Copy link
Owner

I'd go for just #[token("hello", ignore_case)].

It's all contained in the proc macro here, should be straight forward to mutate the definition given the flag, one thing to keep in mind is that this will have to work with both token and regex attributes.

@gymore-io
Copy link
Contributor Author

gymore-io commented Jan 28, 2021

I started to think about the implementation but I'm running in a little issue here:

How do we discriminate between ignore_case, a lone ident and ignore_case a callback function in first position? Do I filter certain keywords in parse_callback ?

A way to fix this issue without needing to think about callbacks may be to change the syntax into something like

#[token, ignore(case)]
Hello,

and just treat ignore(case) as a Nested::Named("ignore", NestedValue::Group("case")).

Also, unrelated to the issue: would a ignore_ascii_case variant be useful ? Only checking ascii case is much faster may be preferred in most cases.

The previous syntax could also be used with this variant.

#[token("hello", ignore(ascii_case))]
Hello

But I will be honest, I still prefer the "flag" version, with just #[token("hello", ignore_case)].

Any thought about that?

@maciejhirsz
Copy link
Owner

maciejhirsz commented Jan 28, 2021

Ah, you are right, a bare name assumes it's a function ref. ignore(case) and optionally ignore(ascii_case) sound good to me.

We can bikeshed over a better way to flag this later, I reckon changing that shouldn't much of a hassle once the logic is done.

@gymore-io
Copy link
Contributor Author

#198 merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants