Add XCBuildConfiguration.append public method #450
Conversation
|
Note that some of the changes of the diff were generated after running the code formatters. |
Codecov Report
@@ Coverage Diff @@
## master #450 +/- ##
==========================================
+ Coverage 80.43% 80.55% +0.11%
==========================================
Files 149 149
Lines 7652 7697 +45
==========================================
+ Hits 6155 6200 +45
Misses 1497 1497
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #450 +/- ##
==========================================
+ Coverage 80.94% 81.12% +0.18%
==========================================
Files 147 148 +1
Lines 7592 7667 +75
==========================================
+ Hits 6145 6220 +75
Misses 1447 1447
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
this will be a nice addition to XcodeProj - will allow re-using some of the helpers from Tuist! π
A few suggestions on some more cases that can be present with settings - especially as we have no control on what will be read from an existing project.
Array Settings
Xcode prefers array settings sometimes - it even goes as far as re-formatting the string with spaces to an array after opening the project.
e.g.
HEADER_SEARCH_PATHS = "$(inherited) \"My Libraries/MyLibraryA/include\" \"My Libraries/MyLibraryB/include\""
gets modified to:
HEADER_SEARCH_PATHS = (
"$(inherited)",
"\"My Libraries/MyLibraryA/include\"",
"\"My Libraries/MyLibraryB/include\"",
);
Other Settings
If I'm not mistaken, settings can be of type boolean too? Testing it out, settings with a boolean value e.g. "YES" are treated as strings within the library so not a 100% sure on this one.
Tests/xcodeprojTests/Objects/Configuration/XCBuildConfigurationTests.swift
Show resolved
Hide resolved
| let newValue = [existingValue, value].joined(separator: " ") | ||
|
|
||
| // Remove duplicates | ||
| let newValueComponents = Set(newValue.split(separator: " ")) |
There was a problem hiding this comment.
Some settings may have a space e.g. "My Libraries/MyLibraryA/include" - using a Set will shuffle the order of the reconstructed setting.
| subject.append(setting: "OTHER_LDFLAGS", value: "flag2") | ||
|
|
||
| // Then | ||
| XCTAssertEqual(subject.buildSettings["OTHER_LDFLAGS"] as? String, "flag1 flag2") |
There was a problem hiding this comment.
I noticed this failed locally for me
XCTAssertEqual failed: ("Optional("flag2 flag1")") is not equal to ("Optional("flag1 flag2")")It's due to using a Set to reconstruct the setting. I think this test itself is correct as it is, appending settings should append the new setting to the end.
|
@kwridan I've added logic to handle the settings that are arrays of strings. I added the test that you proposed to test that it's properly handled. Regarding booleans, I think it should be fine because they are stored as strings. Notice in the changes that in the case of arrays, I'm removing the duplicates. That's not straightforward in the case of strings. I was thinking though about removing duplicated |
|
Thanks @pepibumur
That's fair - perhaps we can just remove duplicates as a whole e.g.
This will protect against the case where you append the same item twice. func test_append_when_duplicateSettingExists() {
// Given
let subject = XCBuildConfiguration(name: "Debug",
baseConfiguration: nil,
buildSettings: ["OTHER_LDFLAGS": "flag1"])
// When
subject.append(setting: "OTHER_LDFLAGS", value: "flag1")
// Then
XCTAssertEqual(subject.buildSettings["OTHER_LDFLAGS"] as? String, "flag1")
} |
Resolves #395
Short description π
Add public method to conveniently append settings as suggested here
Implementation π©βπ»π¨βπ»