-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 ValidIBInspectable rule implementation #901
Conversation
Would love someone else to verify these. I don't have extensive experience with |
I'll defer to @marcelofabri to review this. I'm also not very familiar with |
Current coverage is 86.09% (diff: 100%)@@ master #901 diff @@
==========================================
Files 114 114
Lines 5058 5055 -3
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 4355 4352 -3
Misses 703 703
Partials 0 0
|
@dduan Thanks for sharing this. It seems that 1 and 2 are indeed correct. About 3, while Xcode shows the fields in IB, I get errors in runtime:
Can you reproduce that? I had to remove all the Also, this needs a rebase because I merged another PR (#886) 😬 |
@marcelofabri huh, you are right about the runtime error. So it seems that optionals and IUOs aren't supported at all. |
30c6278
to
3734a9c
Compare
@marcelofabri updated with changes only relate to |
3734a9c
to
e238516
Compare
@marcelofabri added entry to CHANGLOG. Thanks for reviewing! |
@@ -45,6 +45,9 @@ | |||
|
|||
##### Bug Fixes | |||
|
|||
* Rule out a few invalid `@IBInspectable` cases in `valid_ibinspectable`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need two trailing spaces after the .
😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. This is quite strange. Do you know the reason for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's to force markdown to add a <br/>
. From the original spec:
When you do want to insert a
<br />
break tag using Markdown, you end a line with two or more spaces, then type return.
1. `NSNumber` is not IB-inspectable at all. 2. Explicitly typed `Optional`s and `ImplicitlyUnwrappedOptional`s aren't supported by `@IBInspectable`, reference type or not.
e238516
to
7d4e0ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just wait Travis to merge ✅
Thanks again for bringing this up 🎉
Many thanks to both of you! |
There are several issue in the current implementation of this rule:
NSNumber
is not IB-inspectable at all.Optional
s andImplicitlyUnwrappedOptional
s aren'tsupported by
@IBInspectable
, reference type or not.Supported value types with!
s and?
s are indeed inspectable, contrary towhat the current implementation deems.
For verification, try these out with this little class:
https://gist.github.com/dduan/da4050ca3a0bce5b4d54d21effb06b24