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

Trim whitespace of filter input #1573

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

nlfurniss
Copy link
Contributor

Often when pasting in a filter string, a user may copy extra whitespace, causing QUnit to throw an error about no tests matching.

After looking through the tests, I'm not sure the best place to put a unit test. If the maintainers are interested in adding this functionality, I'll figure out the best place to add the test.

@Krinkle
Copy link
Member

Krinkle commented Mar 31, 2021

@nlfurniss Thanks for the patch, this looks good to me. Just two things:

  1. We support projectst that in turn support older browsers, so we'll need to use a local trim function for now rather than ES5. See
    set.replace( /^\s+|\s+$/g, "" );
    for example. Feel free to move that into a local function for re-use.
  2. Since this is your first PR, also sign at cla.js.foundation/qunitjs/qunit to indicate that you are the author of this code and are okay with releaseing it under our open source license.

@nlfurniss
Copy link
Contributor Author

  1. We support projectst that in turn support older browsers, so we'll need to use a local trim function for now rather than ES5. See

Sounds good, done!

  1. Since this is your first PR, also sign at cla.js.foundation/qunitjs/qunit to indicate that you are the author of this code and are okay with releasing it under our open source license.

When i go to that page it says You have signed the CLA for qunitjs/qunit at the top 🤷

@Krinkle
Copy link
Member

Krinkle commented Mar 31, 2021

When i go to that page it says You have signed the CLA for qunitjs/qunit at the top 🤷

It seems your Git config author is not associated with your GitHub account. That might be the issue. The system is associated by email address I believe, so you may need to add the address used for your commits (git show) to your GH profile. (It can be a hidden/secondary address, doesn't need to be public I think.)

@nlfurniss nlfurniss force-pushed the trim-whitespace-filter-input branch from 4f88fad to a7dc438 Compare April 1, 2021 00:05
@nlfurniss
Copy link
Contributor Author

When i go to that page it says You have signed the CLA for qunitjs/qunit at the top 🤷

It seems your Git config author is not associated with your GitHub account. That might be the issue. The system is associated by email address I believe, so you may need to add the address used for your commits (git show) to your GH profile. (It can be a hidden/secondary address, doesn't need to be public I think.)

Very embarrassing, I spelled my email wrong 😳

@Krinkle Krinkle merged commit befc942 into qunitjs:main Apr 1, 2021
@Krinkle
Copy link
Member

Krinkle commented Apr 1, 2021

No worries, glad that was all. Thanks again!

I'll cut a patch release within the next few days.

@nlfurniss nlfurniss deleted the trim-whitespace-filter-input branch April 1, 2021 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants