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

fix: convert input to lowercase during arg parse #843

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

damaredayo
Copy link

Fixes #842

@MaxOhn
Copy link
Owner

MaxOhn commented Sep 3, 2024

Not quite as simple with rust unfortunately 😩
str::to_lowercase allocates a new String which is dropped right after taking its reference and rust doesn't like that.

I'd recommend using the more efficient CowUtils::cow_to_ascii_lowercase to avoid the forced reallocation but it will have the same issue regarding returned references.

To fix that, I recommend not modifying the parse_key method and instead lowercase the key_opt value that results from the parse_key call.

@damaredayo
Copy link
Author

Sorry, been writing too much go recently I think - silly error on my part. Just getting back into rust after hyper-focusing go in my career, appreciate you being as friendly as always Max <3

@MaxOhn
Copy link
Owner

MaxOhn commented Sep 3, 2024

Don't forget to also adjust the parse_any function which handles arguments of the form AR9.3 instead of AR=9.3 😉

Would also appreciate if you add test cases at the bottom regarding lowercasing.

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.

lowercase <s arguments before reading them
2 participants