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

feat: ParsePositional::non_strict #374

Merged
merged 6 commits into from
Jul 23, 2024
Merged

Conversation

ozwaldorf
Copy link
Contributor

@ozwaldorf ozwaldorf commented Jul 20, 2024

Attempt to resolve #373 by adding ParsePositional::non_strict, the inverse of ParsePositional::strict. Derive support also added. Internally adds an enum for the position states; Unrestricted, Strict, and NonStrict.

Tested manually against lutgen, as well as some unit tests, and is a working as expected.


Note: There are some failing unrelated unit tests when using --all, but main also has the same failures, so I'll ignore. CI doesn't seem to run them either

Adds new catchable error NotStrictPos, and a utility to set the
condition. Also adds derive support.
@pacak
Copy link
Owner

pacak commented Jul 20, 2024

There is a small edge case where both strict and not_strict are able to be used together

I think it can be an enum with 3 states: Strict, NonStrict and Unrestricted.

Going to sleep for now, dealing with a newborn is like two full time jobs. Will check tomorrow.

@ozwaldorf
Copy link
Contributor Author

ozwaldorf commented Jul 21, 2024

Refactored the conditionals into a marker trait, in order to only implement the strict/non_strict modifier methods for the default unrestricted parser type. I know it's not the enum, but this way prevents misusage at the type level.

Trying to use both of the modifiers results in a compilation error for manual usage:

image

As well as derive usage:

image

@pacak
Copy link
Owner

pacak commented Jul 21, 2024

Derive macro part looks good, but I'm not sure if it's worth extra complexity to go type level stuff. Can we go back to having a simple enum field and changing runtime behavior based on that?

This technically allows for subsequent calls to `strict` and `non_strict`, but only
the last call will actually be used.
@ozwaldorf ozwaldorf changed the title feat: ParsePositional::not_strict feat: ParsePositional::non_strict Jul 22, 2024
@ozwaldorf
Copy link
Contributor Author

Derive macro part looks good, but I'm not sure if it's worth extra complexity to go type level stuff. Can we go back to having a simple enum field and changing runtime behavior based on that?

Sure, it's your call, maintainability is important 👍

@pacak pacak merged commit fc14443 into pacak:master Jul 23, 2024
3 checks passed
@ozwaldorf ozwaldorf deleted the feat/not_strict branch July 23, 2024 17:23
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

Successfully merging this pull request may close these issues.

Strict and regular many positionals conflict when parsing
2 participants