swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.67k stars 10.38k forks source link

[SR-2405] Print IB_DESIGNABLE and IBInspectable in the generated ObjC header #45012

Closed belkadan closed 5 years ago

belkadan commented 8 years ago
Previous ID SR-2405
Radar rdar://problem/22272850
Original Reporter @belkadan
Type Improvement
Status Resolved
Resolution Done
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Improvement, PrintAsObjC, StarterBug | |Assignee | @JGiola | |Priority | Medium | md5: a62b23d529adbcb3abf41a207a8f69e8

Issue Description:

We should print the IB_DESIGNABLE and IBInspectable Objective-C annotations in the generated header when a class is marked @IBDesignable or a property @IBInspectable.

JGiola commented 8 years ago

Hi, I want to try to tackle this, since is labeled as StarterBug 🙂

The things I have to change are in the PrintAsObjC.cpp file right?
There is some guidance on the order of the attributes? Like IBInspectable must be before or after IBOutlet? And IB_DESIGNABLE before or after the swift compatibility name?

There are other files where I have to make changes?

belkadan commented 8 years ago

Hi, welcome! Yes, all changes should be in PrintAsObjC.cpp. In this case I don't think we have to worry too much about attribute order. I personally think IB_DESIGNABLE looks better before the Swift compatibiity name on purely aesthetic grounds (it's shorter and more interesting to people). IBOutlet and IBInspectable shouldn't overlap, I think, but if they do it doesn't really matter which one goes first.

Once you've changed PrintAsObjC, you'll also need to add test cases by modifying the files in test/PrintAsObjC/. I'm also a little concerned that Xcode will get confused by the IB_DESIGNABLE and IBInspectable being in both the Swift files and the generated header file, so we'll have to try that out too.

JGiola commented 8 years ago

Ok got it!
I have to assign to me this issue? Or is it unnecessary?

And one last thing why Xcode can get confused? Is it looking inside both the swift file and the generated header for exposing things inside Interface Builder?

belkadan commented 8 years ago

Seems reasonable to assign the issue to yourself, to make it clear that it's being worked on when someone does a search.

I'm mostly just not sure about Xcode. Since this couldn't happen in the past, it's possible the current logic will indeed get confused (because the IB folks didn't bother to special-case it). It probably won't be an issue because IBOutlet and IBAction already work, but it's still something we have to test.

JGiola commented 8 years ago

Sorry, but can I ask you how can I test the changes on my local machine before pushing them?

On swift.org I've found instruction only for building swift from sources (and for now on macOS Sierra is failing but I have to look why), there is something I miss?

belkadan commented 8 years ago

Check the README.md file for a little more information (in particular, the -t flag to build-script). If you want to run individual tests it's a bit more complicated, but there are some instructions in docs/Testing.rst that explain the system in more detail and include some invocation lines you can use.

JGiola commented 8 years ago

I don't know if I'm supposed to tell here but I've sent a merge request on GitHub.

I'm not quite sure if I've get how to write the tests so if there is something wrong please tell me 🙂

JGiola commented 5 years ago

Can I close this issue? The PR has been merged.

belkadan commented 5 years ago

The typical workflow would be to move it to Resolved as the person who fixed it, and let the person who filed it Close it. (…says the person who filed it.)