Use Custom File Name for .xcodeproj at Model Level
#462
Conversation
SwiftLint found issuesWarnings
Generated by π« Danger |
Codecov Report
@@ Coverage Diff @@
## master #462 +/- ##
=========================================
+ Coverage 92.32% 92.4% +0.08%
=========================================
Files 334 336 +2
Lines 17373 17564 +191
=========================================
+ Hits 16039 16230 +191
Misses 1334 1334
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for adding support for this @adamkhazi.
There's one thing though missing, which I think is important when adding features: exposing an API to the users. In the past, we added features like supporting multiple configurations, that users could not use even the logic for it was in the TuistGenerator module.
Together with it we should document the attribute, how it can be used, and maybe an use case for its usage.
Don't forget to update the CHANGELOG π¬
|
Happy to expose an API to users. π I am worried about exposing it through the manifest description directly though as in the following example: I believe it may be confusing and may not fit the use case I described in #461. The distinction between the Additionally, to result in two Instead I suggest we add a Again, this is quite a niche use case and I am wondering whether this will be a widely applicable feature. |
Looking at the API you are right, it's confusing for the users. But I think it's also confusing in the internal framework, which ends up exposing an API that Tuist doesn't use. Someone looking at that
The other solution that you propose, let config = TuistConfig(generationOptions: [
.suffixProjectNames(with: "-yourSuffix")
])What do you think @tuist/core ? By the way, I'm not saying no to the feature π¬. I'm trying to fit it nicely in the public API to not end up with an internal API that just fits one user's need. |
|
Hey @adamkhazi! The idea is certainly interesting - I can understand why it might be useful to provide some transforms over the defaults Tuist provides.
I also worry about this. Typically we try to make the manifest obvious to the end-user. I like the idea @pepibumur has put forward - would that fit your use-case? or do you need to be able to do it on a per-project basis? In the future I would also like us to be able to provide prefixes to other kinds of files. One I would like to see would be the manifest files themselves (e.g. One change I would make would be to introduce the API like this so that we do not limit it to just prefixes. let config = TuistConfig(generationOptions: [
.xcodeProjectName("${TARGET_NAME}-yourSuffix")
])I'm happy to discuss this more if we think it needs it. |
|
Hey @ollieatkinson! Putting the suffix inside the Tuist config as @pepibumur suggested would fit my use case well. @ollieatkinson did you mean Extending @pepibumur's example I was thinking something like the following might be more obvious to users in the config instead of having a let config = TuistConfig(generationOptions: [
.suffixProjectName(with: String)
// .prefixProjectName(with: String)
// .suffixAndPrefixProjectName(with: String, and: String)
])
|
Yes, sorry. The previous comment was just an example of intent rather than actual implementation. I would probably like to use the new string interpolation from Swift 5. e.g. .xcodeProjectName("\(.projectName)-yourSuffix") |
|
Sure, that makes sense. π I will experiment with your interpolation example locally before posting some code. |
|
What about something like the following? struct TemplateString {
let rawString: String
}
extension TemplateString: ExpressibleByStringLiteral {
init(stringLiteral: String) {
self.rawString = stringLiteral
}
}
extension TemplateString: CustomStringConvertible {
var description: String {
return rawString
}
}
extension TemplateString: ExpressibleByStringInterpolation {
init(stringInterpolation: StringInterpolation) {
self.rawString = stringInterpolation.string
}
struct StringInterpolation: StringInterpolationProtocol {
var string: String
init(literalCapacity: Int, interpolationCount: Int) {
self.string = String()
}
mutating func appendLiteral(_ literal: String) {
self.string.append(literal)
}
}
}
extension TemplateString {
enum Token: String {
case projectName = "${project_name}"
}
}
extension TemplateString.StringInterpolation {
mutating func appendInterpolation(_ token: TemplateString.Token) {
self.string.append(token.rawValue)
}
}
let projectName: TemplateString = "prefix-\(.projectName)-suffix" |
|
API looks good, if you manage to push it up I can take a look at testing it out in a few of the fixtures. |
|
|
||
| private func enriched(model: TuistGenerator.Project, | ||
| with config: TuistGenerator.TuistConfig) throws -> TuistGenerator.Project { | ||
| var enrichedModel = model |
There was a problem hiding this comment.
Notice that the model being passed is a reference, so creating another internal variable does not make any copy. I'd just remove it.
There was a problem hiding this comment.
We do the following operations in the same method:
enrichedModel = enrichedModel.adding(target: manifestTarget)
...
enrichedModel = enrichedModel.replacing(fileName: xcodeFileName)The idea was to be consistent and continue returning a new model every time a Project property is changed. Project's properties are immutable.
The alternative would be to change the lets inside Project to vars.
There was a problem hiding this comment.
Let's keep it like that for now. If we see maintaining those methods that create copies of the instances become cumbersome, we can reconsider the approach.
| @@ -242,12 +276,24 @@ extension TuistGenerator.Project { | |||
| func adding(target: TuistGenerator.Target) -> TuistGenerator.Project { | |||
There was a problem hiding this comment.
Given that TuistGenerator.Project is a class, we don't need to be creating copies of the objects. Wherever adding and replacing is used, I'd just set the attribute to the object and not create new instances of it.
There was a problem hiding this comment.
I believe this is related to my previous comment about Project's properties being immutable.
fixtures/app_with_framework_and_tests_with_custom_filenames/TuistConfig.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I added some minor nitpick comments. Excellent job here @adamkhazi.
I didn't know the template interpolation thing was possible in Swift. The API that you achieved in this PR is very beautiful β€οΈ
|
Thanks @pepibumur! I believe the nitpicks should be addressed now apart from one. π |
|
Great job @adamkhazi
|
Resolves #461
Short description π
Add the ability to specify a custom file name at the model level to write an
.xcodeprojduring generation. Currentlytuistuses the name of the project when saving the.xcodeprojto disk.Why is this needed?
#461 goes into more detail but essentially this is needed to check two
.xcodeprojs for equality more easily.Solution π¦
Expose a
fileNameproperty in the project model (TuistGenerator/Models/Project.swift). By default it will use thenameof the project if afileNameis not specified. Otherwise, it will use thefileNamespecified to write to disk i.e.,filename.xcodeproj.Implementation π©βπ»π¨βπ»
fileNametoModels/Project.swiftwhich falls back tonameif nofileNameis specifiedGenerator/ProjectGeneratoruseproject.fileNamefor thexcodeprojPathGenerator/ProjectGeneratorTests.swifttest to check the filesystem for correctly written.xcodeprojnameModels/ProjectTests.swifttest to check the model