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

Update isDimension logic #4

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

Conversation

jbrower2
Copy link

@jbrower2 jbrower2 commented Oct 3, 2024

I believe the intention of the isDimension function is to test whether a feature ends with height or width, but what the regular expression is actually doing is checking whether it ends with any of the following characters: |deghitw, which doesn't seem correct. Additionally, the regular expression is case-sensitive, so I added the i modifier. See Regex101 for an explanation.

An alternative to this would be to remove the isDimension check entirely, because it seems unintended, and would technically be a breaking change to make it work as intended. This check also isn't documented, or thoroughly tested in the unit tests.

I believe the intention of the `isDimension` function is to test whether a feature ends with `height` or `width`, but what the regular expression is actually doing is checking whether it ends with any of the following characters: `|deghitw`, which doesn't seem correct. Additionally, the regular expression is case-sensitive, so I added the `i` modifier.

An alternative to this would be to remove the `isDimension` check entirely, because it seems unintended, and would technically be a breaking change to make it work as intended. This check also isn't documented, or thoroughly tested in the unit tests.
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.

1 participant