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

Fix removal of whitespace in ignored/required tags #201

Merged
merged 1 commit into from
Apr 2, 2017

Conversation

Krenair
Copy link
Contributor

@Krenair Krenair commented Apr 1, 2017

This replacement was presumably to handle spaces following commas, but it also got important spaces in the middle of tags.

Bug: T155333

This replacement was presumably to handle spaces following commas, but it also got important spaces in the middle of tags.

Bug: T155333
@se4598
Copy link
Member

se4598 commented Apr 1, 2017

if I'm reading this correctly, this only gets one whitespace at the first position.
And as I like using framework functions instead of writing own, I would use the following x = x.replace("\n", "").trimmed(); (I left the replace b/c if somehow a strange '\n' is inside a string based on original code)

@Krenair
Copy link
Contributor Author

Krenair commented Apr 1, 2017

Why do you want to get rid of all whitespace on both sides of each tag?

@se4598
Copy link
Member

se4598 commented Apr 1, 2017

Are tags possible that start or end with spaces? If not and this is directly from user input (whether user config or input field, why shouldn't it removed for usability? Previously the spaces would get removed.
Yeah, it may be a little bit speed inefficient most of the time, but this shouldn't be a location and magnitude where this matters.

Disclaimer: Despite marked as org 'member', I'm just doing code reading here, I don't know/can't look up where the function is called from, as haven't looked at huggle and it's code for a long time.

@benapetr
Copy link
Member

benapetr commented Apr 2, 2017

I think that se4598 is right, one thing is that at some point, there could be whitespace on both sides, for example if someone edited their huggle3.css by hand and added there string like that "tag1, tag2 , tag3"

This is of course their mistake, but I like software that is more error-tolerant and thus user friendly :)

@benapetr
Copy link
Member

benapetr commented Apr 2, 2017

but it really is easy to fix I can do that myself, thanks for finding where the problem was

@benapetr benapetr merged commit 752788d into huggle:master Apr 2, 2017
@benapetr
Copy link
Member

benapetr commented Apr 2, 2017

See 3815aef followup ;)

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.

None yet

3 participants