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

Remove trimming from StringValue expert #20

Merged
merged 2 commits into from
Feb 18, 2014
Merged

Conversation

adrianheine
Copy link
Contributor

No description provided.

return null;
}
return value;
return this._newValue !== false ? this._newValue : this.$input.val();
Copy link
Member

Choose a reason for hiding this comment

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

can be changed to equality by flipping - makes it easier to read

@JeroenDeDauw
Copy link
Member

The commit message does not seem to match the code change. It removes the returning of null when empty string.

@adrianheine
Copy link
Contributor Author

You are absolutely right, that's not trimming, it's checking for white-space only. @addshore: We still wanna do this, right?

@adrianheine
Copy link
Contributor Author

@brightbyte What do you think about this? It's not trimming, it's checking for whitespace-only.

@addshore
Copy link
Member

addshore commented Feb 4, 2014

probably up to @lydiapintscher

@lydiapintscher
Copy link
Member

I think we should remove white spaces at the beginning and the end. I can't think of a good case where this is useful compared to the pain of removing them when they shouldn't be there but are saved by Wikibase. Where and how it is done I don't have an opinion on.

Turned !== around since it's a little bit easier to read.
thiemowmde added a commit that referenced this pull request Feb 18, 2014
This really, really doesn't belong here. Raw should be raw and not care about the meaning of specific characters.
@thiemowmde thiemowmde merged commit 8d1cb52 into master Feb 18, 2014
@JeroenDeDauw JeroenDeDauw deleted the stringValueTrim branch February 18, 2014 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants