Generate an XCode like bridging header. #342
Conversation
Generates also the version number and name variable.
|
I think Xianwen should be the primary reviewer here since he has more experience with Swift. Two bits of housekeeping though:
|
|
I think I have already signed it two times. Should we sign it for every PR? |
|
Oops, sorry. I think it was my mistake searching for your userid. It's manual and error-prone since our bot broke. |
| //! Project version number for TextSort_Bridging_Header. | ||
| FOUNDATION_EXPORT double TextSort_Bridging_HeaderVersionNumber; | ||
|
|
||
| //! Project version string forTextSort_Bridging_Header. |
There was a problem hiding this comment.
Missing space between "for" and "TextSort_Bridging_Header" here.
| 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") |
src/source/Main.scala
Outdated
| @@ -346,6 +346,7 @@ object Main { | |||
| objcppNamespace, | |||
| objcBaseLibIncludePrefix, | |||
| objcSwiftBridgingHeaderWriter, | |||
| objcSwiftBridgingHeader, | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| @@ -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) | |||
There was a problem hiding this comment.
Inconsistent with the declaration objcSwiftBridgingHeader above.
| #import <Foundation/Foundation.h> | ||
|
|
||
| //! Project version number for TextSort_Bridging_Header. | ||
| FOUNDATION_EXPORT double TextSort_Bridging_HeaderVersionNumber; |
There was a problem hiding this comment.
I'd prefer to have a _ separating TextSort_Bridging_Header and VersionNumber so that it's TextSort_Bridging_Header_VersionNumber.
There was a problem hiding this comment.
I choosed the Xcode Convention. By calling a framework MyLib, the two variables are named MyLibVersionNumber and MyLibVersionString
There was a problem hiding this comment.
I see, thanks for the explanation.
In that case, would it make sense to name it TextSortBridgingHeaderVersionNumber?
| 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") |
There was a problem hiding this comment.
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.
| @@ -38,8 +38,23 @@ class SwiftBridgingHeaderGenerator(spec: Spec) extends Generator(spec) { | |||
| } | |||
|
|
|||
| object SwiftBridgingHeaderGenerator { | |||
| def writeAutogenerationWarning(writer: Writer) { | |||
|
|
|||
| val bridgingVarPrefix = (s: String) => s.split('-').mkString("_") | |||
There was a problem hiding this comment.
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.)
|
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.
Generates also the version number and name variable inside the bridging header.