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: allow command casing to be ignored #30

Merged
merged 3 commits into from
Jul 19, 2024
Merged

Conversation

roziscoding
Copy link
Contributor

Fixes #27

@roziscoding roziscoding self-assigned this Jul 16, 2024
Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should the plugin handle the case of hasCommand(/REGEX/i, { ignoreCase: false }) where the regex itself declares case-independent matching but the options explicitly require the casing to be matched exactly?

src/command.ts Show resolved Hide resolved
src/command.ts Show resolved Hide resolved
test/command.test.ts Outdated Show resolved Hide resolved
src/command.ts Outdated Show resolved Hide resolved
@carafelix
Copy link
Member

How should the plugin handle the case of hasCommand(/REGEX/i, { ignoreCase: false }) ?

I think the RegExp should always take priority, not only because ignoreCase is set to false as a default, but more importantly, because is more likely for someone to set the i flag when working with regex's and not care / notice about the ignoreCase option, than to purposely add the i flag and manually set ignoreCase to true

@KnorpelSenf
Copy link
Member

In other words, if either i or ignoreCase: true is provided, casing will be ignored, and case-sensitive matching will only happen when none of the two options are set. Correct?

@carafelix
Copy link
Member

carafelix commented Jul 16, 2024

In other words, if either i or ignoreCase: true is provided, casing will be ignored, and case-sensitive matching will only happen when none of the two options are set. Correct?

Exactly

@carafelix carafelix requested a review from KnorpelSenf July 19, 2024 03:46
Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@carafelix carafelix merged commit 71d5da2 into main Jul 19, 2024
6 checks passed
@KnorpelSenf KnorpelSenf deleted the feat/ignore-case branch July 19, 2024 19:36
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.

Case-insensitive command names
3 participants