Skip to content

Exclude-views/subviews Comma Separated List Fix#312

Merged
timbodeit merged 1 commit intotombenner:masterfrom
whynotmatt:master
Nov 18, 2015
Merged

Exclude-views/subviews Comma Separated List Fix#312
timbodeit merged 1 commit intotombenner:masterfrom
whynotmatt:master

Conversation

@whynotmatt
Copy link
Copy Markdown
Contributor

A comma separated list of exclude-views or exclude-subviews wasn't working correctly. This is a quick fix for that.

@timbodeit
Copy link
Copy Markdown
Collaborator

Please have a look at the Readme.
exclude-views is documented to be used with comma separation without any spaces before or after the comma.
You're placing spaces before and after the comma, right? That is why exclude-views didn't work correctly for you.

The way I see it, your changes would break the existing and documented behavior and force everybody to add a single space before and after the comma.

Nonetheless a pull-request, that adds support for spaces (without breaking existing notations) in this comma separated list would be very welcome.
Ideally any combination zero, one or multiple spaces on both sides of the comma should be supported.

@whynotmatt
Copy link
Copy Markdown
Contributor Author

In my exclude-subviews, I am using a comma separated list without spaces.

exclude-subviews:PUAlbumListCellContentView,UIImagePickerController,UITableViewCell;

The problem, as pointed out by frapaa in #269 is that the NUIParserDelegate.m is adding in extra spaces around the commas. So:

exclude-subviews:PUAlbumListCellContentView,UIImagePickerController,UITableViewCell;

is converted to
exclude-subviews:PUAlbumListCellContentView , UIImagePickerController , UITableViewCell;

This happens on line 48 of NUIParserDelegate.m

Because the exclude-subviews string now has extra spaces in it, the logic on UIView+NUI.m lines 52 and 66 doesn't work. So my pull request was to address this issue by splitting the exclude-subviews string with a " , " instead of ",".

I think this is ultimately the error that is being reported in issues #224 , #269 and #299

@timbodeit
Copy link
Copy Markdown
Collaborator

I'm sorry.
Thank you for explaining.

Looks good to me.

timbodeit added a commit that referenced this pull request Nov 18, 2015
Exclude-views/subviews Comma Separated List Fix
@timbodeit timbodeit merged commit df043d7 into tombenner:master Nov 18, 2015
@timbodeit
Copy link
Copy Markdown
Collaborator

For future reference:
The changes in this pull request are a workaround for a regression caused in 79f74d9

In my eyes the parser needs some work as

  • taking apart a comma separated list (in the parser)
  • putting it back together with spaces (in the parser)
  • taking it apart again in the view

is not exactly ideal.

Also this is probably something we should add a unit test for.

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.

2 participants