-
-
Notifications
You must be signed in to change notification settings - Fork 621
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
Swift templates proof of concept. #95
Conversation
Generated by 🚫 danger |
26daa54
to
fa2f055
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.
Great work 👍 I only have minor suggestions
extension NSCoder { | ||
|
||
@nonobjc func decode(forKey: String) -> Int { | ||
// swiftlint:disable force_cast |
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.
the force_cast seem unnecesary?
aCoder.encode(self.arguments, forKey: "arguments") | ||
} | ||
|
||
override func isEqual(_ object: Any?) -> Bool { |
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.
can't we just generate this one?
let error: String | ||
} | ||
|
||
private extension Path { |
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 could create a simple extension that both tests and sourcery can reuse, the code is the same outside of the path component which we can parametrize
static let close = "%>" | ||
} | ||
|
||
func generateSwiftCode(templateContent _templateContent: String, path: Path) throws -> String { |
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.
should we mark them private? also any reason this isn't part of SwiftTemplate class?
self.isClass = aDecoder.decodeBool(forKey: "isClass") | ||
self.isFailableInitializer = aDecoder.decodeBool(forKey: "isFailableInitializer") | ||
self.annotations = aDecoder.decode(forKey: "annotations") | ||
self.__parserData = aDecoder.decode(forKey: "__parserData") |
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.
you should mark __parserData not codable, no point in serializing that, it's only used in parsing stage
} | ||
} | ||
|
||
extension Type { |
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.
this can be removed, we now have dedicated Class
object
@ilyapuchka take a look as well as this one is important, worth 2 people reviewing |
To be honest I don't think I get it how it works except on a very general level =) Will need to look at it closer and run it manually. Will do this weekend. |
First we parse data as usual, then we persist the data to disk. |
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.
looks good to me, we can iterate on swift templates in the future via different prs, good to merge after @ilyapuchka approval
|
||
``` | ||
<% for type in types.classes { %> | ||
extension <%= type.name %>: Equatable {} |
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.
What about using string interpolation style?
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.
Hi @ilyapuchka ,
We need to use something obscure to have the least possible impact on Swift grammar and for it visually stand out from regular swift for ease of human template parsing.
There are two most common families of template grammars. One that use {{}}
, and one that use <%%>
.
{{}}
is more suitable for languages that don't have {}
as a part of their grammar (HTML).
I've found that <%
is typically used for template engines that generate C family of languages so I've just used what was proven and common to me.
/// lists all known types, excluding protocols | ||
/// sourcery: skipEquality | ||
lazy var all: [Type] = { | ||
return self.types.all |
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.
In current version we exclude protocols, I guess should do the same here?
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.
This is fixed, I've made types.all
perform the filtering.
fileprivate static func generateSwiftCode(templateContent _templateContent: String, path: Path) throws -> String { | ||
let templateContent = "<%%>" + _templateContent | ||
|
||
let components = templateContent.components(separatedBy: Delimiters.open) |
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.
If delimiter is used for example in a string literal, do we escape it somehow?
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 don't
let context = GenerationContext(types: types, arguments: arguments) | ||
let swiftCode = try SwiftTemplate.generateSwiftCode(templateContent: try sourcePath.read(), path: sourcePath) | ||
|
||
let compilationDir = Path.cleanTemporaryDir(name: "build") |
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.
Will this clean build folder of the target that uses Sourcery? Wouldn't that lead to Xcode recompiling it every time completely?
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.
no, we are using NSTemporaryDirectory for each run, completely unrelated to sourcery or users folders
throw SwiftTemplateParsingError.compilationError(path: binaryFile, error: compilationResult.error) | ||
} | ||
|
||
let result = try Process.runCommand(path: binaryFile.description, |
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.
So if I get it correctly we encode context with all types information, then we pass the path to this archive to another swift file in which we insert code that has <%= %>
replaced with prints and that decodes the archive and runs the printing code we inserted in it. And then we get the output of all these prints as a result. Right?
Do we somehow handle the issue with empty lines when removing control code (stencilproject/Stencil#32) or we don't have it at all?
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'm unsure what that referenced issue exactly is. I'm assuming it doesn't have that issue.
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.
Yeah, I guess as instead of removing control code we use print we don't have that exact issue, but then how is indentation handled?
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.
Hi @ilyapuchka ,
there is no magic behavior. It should be simple to deduce how your result template will look like according to white spaces in template since all non code pars will be escaped and printed.
|
||
} | ||
|
||
/* |
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.
Why all that commented out? It does not work? Do we miss something to be able to generate that code properly?
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.
@kzaher uses a script to copy this into each class until we have inline code generation as part of sourcery
@kzaher can you rebase on top of master (2 conflicts)? I'll merge it once that's done |
merged 👍 |
Hi,
this PR adds a proof of concept for Swift generator.
It was the simplest implementation I could come up with for now. It works by serializing metadata from sourcery in binary form and deserializing it in compiled template binary.
This approach can be drastically improved in future:
The ugliest part was that I've had to manually copy generated serialization code to original source files. This code can be removed in future and it's just temporary. If Sourcery will be packed as framework in future, then we can remove that serialization code entirely.
I didn't want to update documentation because it would probably be more suitable for project owners to update it where they feel appropriate.