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

isEmpty returns true for whitespace only strings #747

Closed
ZooeyMiller opened this issue Nov 16, 2017 · 6 comments
Closed

isEmpty returns true for whitespace only strings #747

ZooeyMiller opened this issue Nov 16, 2017 · 6 comments

Comments

@ZooeyMiller
Copy link

ZooeyMiller commented Nov 16, 2017

since isEmpty just checks if the string's length is zero, it still returns true given a string of only whitespace, which I feel, for most implementations is not what people need when checking if a string is empty.

Perhaps implementing .trim() on the string before checking if it's empty is worthwhile? Perhaps, if the original behaviour is to be maintained, and not break existing code the function could look something like this:

function isEmpty(string, allowOnlyWhitespace = true) {
  return (allowOnlyWhitespace ? string.length : string.trim().length) === 0;
}

Would love to make a PR if this is deemed a worthwhile addition.

@profnandaa
Copy link
Member

I'm 50-50 on this one, just because we can't "see" the space character, doesn't mean it's not there (empty)... On the other hand, having this as a choice with allowOnlyWhitespace sounds like a good idea to me, not just for b/c.

@ZooeyMiller
Copy link
Author

@profnandaa I think that having an option for not allowing whitespace, as mentioned, is a good choice, so will make a PR to reflect this, and allow any other opinions to come up in PR review.

@RobertFischer
Copy link

@profnandaa @ZooeyMiller Did this PR ever get written/merged in? If not, I'm happy to write it. I was literally just looking for this functionality, although I expected it to be under the name isBlank (coming from Rails).

@ZooeyMiller
Copy link
Author

@RobertFischer I never did write it! would be great to see a PR from you for it if you have the time as I still think it would be useful.

@ftrevo
Copy link
Contributor

ftrevo commented Aug 15, 2018

Credits to @ZooeyMiller for the pull request :)

@chriso
Copy link
Collaborator

chriso commented Aug 17, 2018

Fixed by #880.

@chriso chriso closed this as completed Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants