Generate an XCode like bridging header. #342

yageek · 2017-12-07T13:21:59Z

Generates also the version number and name variable inside the bridging header.

Generates also the version number and name variable.

artwyman · 2017-12-11T21:42:14Z

I think Xianwen should be the primary reviewer here since he has more experience with Swift. Two bits of housekeeping though:

Can you please re-generate the code for the example and test-suite so they're in-sync, and reviewers can see the effects of your change?
Before this can merge you need to sign the CLA here: https://opensource.dropbox.com/cla/

yageek · 2017-12-12T07:15:48Z

I think I have already signed it two times. Should we sign it for every PR?

artwyman · 2017-12-12T07:26:01Z

Oops, sorry. I think it was my mistake searching for your userid. It's manual and error-prone since our bot broke.
I can confirm now that you've signed the CLA, so you're all set.

xianwen · 2018-01-03T20:45:52Z

example/generated-src/objc/TextSort-Bridging-Header.h

+//! Project version number for TextSort_Bridging_Header.
+FOUNDATION_EXPORT double TextSort_Bridging_HeaderVersionNumber;
+
+//! Project version string forTextSort_Bridging_Header.


Missing space between "for" and "TextSort_Bridging_Header" here.

xianwen · 2018-01-03T20:46:19Z

src/source/SwiftBridgingHeaderGenerator.scala

+    writer.write("#import <Foundation/Foundation.h>\n\n")
+    writer.write("//! Project version number for " + varPrefix +".\n")
+    writer.write("FOUNDATION_EXPORT double " + varPrefix + "VersionNumber;\n\n")
+    writer.write("//! Project version string for" + varPrefix +".\n")


Need to add a space after for.

xianwen · 2018-01-03T20:54:35Z

src/source/Main.scala

@@ -346,6 +346,7 @@ object Main {
      objcppNamespace,
      objcBaseLibIncludePrefix,
      objcSwiftBridgingHeaderWriter,
+      objcSwiftBridgingHeader,


This should probably be objcSwiftBridgingHeaderName as you're using spec.objcSwiftBridgingHeaderName.get below.

Actually, I'd love to see this config used in Unit Test for verification.

The variable is declared as objcSwiftBridgingHeader inside main.scala.
Do you want me to change the name of the property in generator.scala from objcSwiftBridgingHeaderName to objcSwiftBridgingHeader instead?

Oh, I see, it's just the variable name here while objcSwiftBridgingHeaderName is the parameter, sorry I got confused.
Yes, I'd prefer to have the same name since this is the case for all the other parameters.

xianwen · 2018-01-03T20:59:38Z

src/source/generator.scala

@@ -216,7 +217,8 @@ package object generatorTools {
        new ObjcppGenerator(spec).generate(idl)
      }
      if (spec.objcSwiftBridgingHeaderWriter.isDefined) {
-        SwiftBridgingHeaderGenerator.writeAutogenerationWarning(spec.objcSwiftBridgingHeaderWriter.get)
+        SwiftBridgingHeaderGenerator.writeAutogenerationWarning(spec.objcSwiftBridgingHeaderName.get, spec.objcSwiftBridgingHeaderWriter.get)


Inconsistent with the declaration objcSwiftBridgingHeader above.

xianwen · 2018-01-03T21:02:32Z

example/generated-src/objc/TextSort-Bridging-Header.h

+#import <Foundation/Foundation.h>
+
+//! Project version number for TextSort_Bridging_Header.
+FOUNDATION_EXPORT double TextSort_Bridging_HeaderVersionNumber;


I'd prefer to have a _ separating TextSort_Bridging_Header and VersionNumber so that it's TextSort_Bridging_Header_VersionNumber.

I choosed the Xcode Convention. By calling a framework MyLib, the two variables are named MyLibVersionNumber and MyLibVersionString

I see, thanks for the explanation.
In that case, would it make sense to name it TextSortBridgingHeaderVersionNumber?

I guess yes 🤔

xianwen · 2018-01-03T21:05:26Z

src/source/SwiftBridgingHeaderGenerator.scala

+    writer.write("#import <Foundation/Foundation.h>\n\n")
+    writer.write("//! Project version number for " + varPrefix +".\n")
+    writer.write("FOUNDATION_EXPORT double " + varPrefix + "VersionNumber;\n\n")
+    writer.write("//! Project version string for" + varPrefix +".\n")


I don't like the way this variable is named. In this case: "..." + varPrefix +"...", it's not used as a prefix.

Although it happens to be a prefix in the case of VersionNumber, I'd still prefer to remove the prefix in the var name, to make it something like bridgingHeaderVarName.

xianwen · 2018-01-03T21:07:19Z

src/source/SwiftBridgingHeaderGenerator.scala

@@ -38,8 +38,23 @@ class SwiftBridgingHeaderGenerator(spec: Spec) extends Generator(spec) {
 }

 object SwiftBridgingHeaderGenerator {
-  def writeAutogenerationWarning(writer: Writer) {
+
+  val bridgingVarPrefix = (s: String) => s.split('-').mkString("_")


Similar to my comment below, I'd prefer to remove the Prefix here, to make it something like bridgingHeaderName, since it doesn't have to be used as a prefix. (Although it's not a great name either, as - is replaced by _ and this method name I suggest wouldn't make it obvious.)

xianwen · 2018-03-02T21:23:55Z

Thanks for your updates, I'll merge it after testing.

* Generate an XCode like bridging header. Generates also the version number and name variable. * Add new generated headers. * Changes naming accorging to comments. * Update generated variable names.

Generate an XCode like bridging header.

4ae2de0

Generates also the version number and name variable.

artwyman assigned xianwen Dec 11, 2017

Add new generated headers.

ee423f6

artwyman added the enhancement label Dec 14, 2017

xianwen reviewed Jan 3, 2018

View reviewed changes

xianwen suggested changes Jan 3, 2018

View reviewed changes

xianwen assigned yageek and unassigned xianwen Jan 4, 2018

yageek added 2 commits February 1, 2018 11:30

Changes naming accorging to comments.

b38b819

Update generated variable names.

494238a

xianwen approved these changes Mar 2, 2018

View reviewed changes

xianwen merged commit 9463fa9 into dropbox:master Mar 2, 2018

jonmcclung mentioned this pull request Jun 20, 2018

Any plans for swift support? #181

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate an XCode like bridging header. #342

Generate an XCode like bridging header. #342

yageek commented Dec 7, 2017

artwyman commented Dec 11, 2017

yageek commented Dec 12, 2017

artwyman commented Dec 12, 2017

xianwen Jan 3, 2018

xianwen Jan 3, 2018

xianwen Jan 3, 2018

yageek Jan 8, 2018

xianwen Jan 24, 2018

xianwen Jan 3, 2018

xianwen Jan 3, 2018

yageek Jan 8, 2018

xianwen Jan 8, 2018

yageek Jan 9, 2018

xianwen Jan 3, 2018

xianwen Jan 3, 2018

xianwen commented Mar 2, 2018

Generate an XCode like bridging header. #342

Generate an XCode like bridging header. #342

Conversation

yageek commented Dec 7, 2017

artwyman commented Dec 11, 2017

yageek commented Dec 12, 2017

artwyman commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xianwen commented Mar 2, 2018