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

Improve error message for invalid parser_engine value #286

Merged

Conversation

Earlopain
Copy link
Contributor

parser doesn't exist, it must be parser_whitequark. In addition, this only accepts symbols.

When passing a string, this would previously print The keyword argument `parser_engine` accepts `parser` or `parser_prism`, but `parser_prism` was passed.. Use inspect to clearly show what you did wrong.

The new error message looks like this: The keyword argument `parser_engine` accepts `:parser_whitequark` or `:parser_prism`, but `"parser_prism"` was passed..

@Earlopain Earlopain force-pushed the fix-parser-engine-error-message branch 2 times, most recently from 6e46cdc to 91e1616 Compare March 8, 2024 10:20
@koic
Copy link
Member

koic commented Mar 8, 2024

It seems to be issues with Layout/LineLength.
https://github.com/rubocop/rubocop-ast/actions/runs/8201827148/job/22431355671?pr=286

@Earlopain
Copy link
Contributor Author

Yes, sorry. I already force-pushed.

@bbatsov
Copy link
Contributor

bbatsov commented Mar 8, 2024

I wonder if we shouldn't convert strings to keywords automatically here (if passed), as it's not a big deal.

@Earlopain Earlopain force-pushed the fix-parser-engine-error-message branch from 91e1616 to 9e3a92a Compare March 8, 2024 14:51
@Earlopain
Copy link
Contributor Author

That makes sense to me. Reverted most of the error message changes since they are now unnecessary and made it to instead accept strings as well.

@Earlopain Earlopain force-pushed the fix-parser-engine-error-message branch from 9e3a92a to 2f0ea83 Compare March 8, 2024 15:23
@marcandre marcandre merged commit e424be0 into rubocop:master Mar 8, 2024
19 of 20 checks passed
@marcandre
Copy link
Contributor

Released in v1.31.2, thanks!

@koic
Copy link
Member

koic commented Mar 8, 2024

I had initially decided to accept only symbol argument, designing it would lead to better performance through consistency. But, currently, I think the convenience of accepting both string and symbol argument might be more beneficial. Let's accept both 👍

@Earlopain Earlopain deleted the fix-parser-engine-error-message branch March 8, 2024 15:31
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.

None yet

4 participants