Reuse identifier

[carlosypunto]

Starting from comments of @djbe on PR #15 I saw necessary a new form for compile info from storyboard. This PR try to solve several problems in actual compilation.
Now we can:

• to know the correct class (both Mac and iOS) of Scenes when is not an custom class
  • ios before now
  • mac before now
• to know the correct type "module.class" (both Mac and iOS) of Scenes when is a custom class
  • ios
  • mac
• to know the correct type "module.class" (both Mac and iOS) of Scenes when is not a custom class
  • ios
  • mac
• to know the type of destination of every Segue
• to know which Scene belongs every Segue. Each Scene has a segues array
• to know which Scene belongs every Cell. Each Scene has a reuseIdentifiers array

[AliSoftware]

Hi, and thanks for your contribution!

Before reviewing the whole PR in details, here are some preliminary feedback:

to know the correct type "module.class" (both Mac and iOS) of Scenes when is a custom class
This is not necessary, as it's already present in the context and template, and handled by the template itself already (even with the current version of SwiftGen and current templates, the class is already prefixed by it's module every time)

to know the correct type "module.class" (both Mac and iOS) of Scenes when is not a custom class

When it's not a custom class, the module is UIKit, no need for hard-coding that in the context, that should also be handled by the template itself to let people have more flexibility

• to know which Scene belongs every Segue. Each Scene has a segues array
• to know which Scene belongs every Cell. Each Scene has a reuseIdentifiers array

Those two points should probably be handled in a separate PR, that current PR addresses a little too many things at once IMHO.

[AliSoftware]

without…?

[AliSoftware]

There's way too many readyFor… boolean properties now if we go that route. Better use a state machine e.g. an enum to tell in which subnode we are, rather than these booleans, if we have so many states now.

[carlosypunto]

readyState is first step always once but next really aren't states as inside TableViews or CollectionViews may have nested segues in its cells. Maybe readyForTableViews and readyForCollectionView must best check as within TableViews or CollectionViews we could have nested them inside cells then can be set readyForTableView to false can cause skip any of them.

[AliSoftware]

At this point I think it would be worth migrating the XML parsing throughout the SwiftGenKit framework to a DOM parser (like SWXMLHash for example) to make the logic more readable and maintainable.

In the first versions of SwiftGen I decided to use the built-in SAX Parser in iOS (NSXMLParser) because I didn't want to introduce an additional dependency while the parsing logic was simple enough. But with the use cases growing and the different use cases we want to handle becoming wider, it might be time to migrate now.

[djbe]

Note that it might be better then to split this up in a few separate PRs:

• one PR for switch to SWXMLHash
• one PR to implement reusable identifier
• one PR for the advanced segue features you want to add

[djbe]

Also, please avoid creating a whole new format/context structure and try to work within the existing structure. We have to keep existing users in mind.

Let's try to keep this PR focused on reuse identifiers (cells) of any kind (table, collection, ...), and defer all other functionality (segue and scene stuff) to other PRs.

[AliSoftware]

At this point I think it would be worth migrating the XML parsing throughout the SwiftGenKit framework to a DOM parser (like SWXMLHash for example) to make the logic more readable and maintainable.

So to address all that properly, the next plan of action is to make separate PRs:

• One to migrate the existing XML parsing code to SWXMLParser (without adding any new feature, just to prepare the groundwork)
• One to add the type of destination of a Segue as an additional entry in the segue context
• One to know in which Scene belongs every Segue
• One to know in which Scene belongs every Cell

The first one to migrate to a DOM parser would hopefully help making the next ones way simpler, by not relying on those readyForXXX booleans anymore but instead make the parsing logic more understandable.

Those different features must be separate, both for our sanity when reviewing to keep the review and discussion isolated, and because some discussion has already been started for some of those features in existing issues.
And for those cases, please discuss your idea of the solution and implementation before opening a new PR, that will avoid you to do code for nothing if it doesn't match everybody's use case or our view of the solution and will avoid to spend time reviewing a code if it doesn't align with the goal in the first place

[djbe]

@carlosypunto If you want, I can do the switch to the new xml parser so you can focus on these features. Should be able to do this during the weekend.

[AliSoftware]

Here are some information about this feature for anyone interested in making it happen

References

• I've just closed Added support for UITableViewCell & UICollectionViewCell identifiers … SwiftGen#66 and Add generation of Table and Collection cell reuseIdentifiers SwiftGen#134 in favor of this PR, but we might find some interesting comments in those so don't hesitate to take a look
• This feature has also already been requested multiple times in the past, so might be worth looking at the comments on those issues too:
  • Add support of XIBs SwiftGen#235
  • Support UITableViewCell & UICollectionViewCell identifiers SwiftGen#19

What's next

To make this PR happen, it might be worth as @djbe mentioned to migrate the existing code to a DOM XML parser first (for everywhere in SwiftGen when we parse XML files). This work/migration has been started in #18

Once we have that, we could resume working on this feature, which will probably become way easier to implement once we have a DOM parser rather than a SAX parser.

Schedule

Tbh our current focus as of now is to first release SwiftGen 5.0 which will cleanup the code and remove all the remaining old code and old template variables that we deprecated after our recent great refactoring in 4.2.

SwiftGen 5.0 shouldn't take long to be released (it's mainly a matter of removing stuff that we marked as deprecated in 4.2.1) but will allow us to start add new features from a cleaner codebase afterwards instead of building on top of some legacy code. So even if not strictly necessary I think it might be worth waiting a few days or weeks until SwiftGen 5.0 is out before starting to work on that PR again?