Run scripts #17
Run scripts #17
Conversation
| name: runScript.name ?? "Run Script", | ||
| inputPaths: Set(runScript.inputFiles), | ||
| outputPaths: Set(runScript.outputFiles), | ||
| shellPath: runScript.shell ?? "/bin/sh", |
There was a problem hiding this comment.
What about making shell non-optional and using a default value in the RunScript constructor with /bin/sh. Since the shell is required for creating a PBXShellScriptBuildPhase I'd make it non-optional here as well. What do you think?
There was a problem hiding this comment.
Yeah that would be an option. I've been trying to keep any specific knowledge out of ProjectSpec though, and just keep it dumb.
Also I think that /bin/sh should be the default value in the xcodeproj constructor, and I didn't want to double up work there. It's something we still need to add to xcodeproj though
| case let .script(script): | ||
| shellScript = script | ||
| } | ||
| let escapedScript = shellScript.replacingOccurrences(of: "\"", with: "\\\"").replacingOccurrences(of: "\n", with: "\\n") |
There was a problem hiding this comment.
Wouldn't you extract this into an extension or a method that is more explicit about why this is necessary?
There was a problem hiding this comment.
You're right, this should go into PXBWriter, but I just put it here for now to get it working
| @@ -33,7 +33,16 @@ public class ProjectGenerator { | |||
| return spec.configs.first { $0.type == .release }! | |||
| } | |||
|
|
|||
| func validate() throws { | |||
| func getPath(_ path: String) -> Path { | |||
There was a problem hiding this comment.
There's a similar method in PBXProjGenerator. Have you consider extracting it somewhere to use it from both places?
There was a problem hiding this comment.
Yeah, it felt weird to add it twice. Maybe an extension on Path
There was a problem hiding this comment.
I just started to add an extension of Path, but found out that appending paths already checks for absolute paths, so this was all unnecessary :)
| @@ -146,8 +146,42 @@ targets: | |||
| Release: config_files/release.xcconfig | |||
| ``` | |||
|
|
|||
| #### Run Scripts | |||
There was a problem hiding this comment.
It's great that you document all these things, very useful 👏
There was a problem hiding this comment.
Looks ok to me @yonaskolb. I just left some minor comments.
Resolves #7
This adds prebuildScripts and postbuildScripts to targets