Skip to content
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

Improve xcconfig file update #4

Merged

Conversation

belemaire
Copy link
Member

Improve the logic for updating build settings in an existing xcconfig file.

Current logic is fully overwriting the file with up to date build settings, only keeping the build settings but trashing any other content that was in the file.

For example, if the file content was

//
// ElectrodeContainer-Debug.xcconfig
//
// Generated by BuildSettingExtractor on 6/7/18
// https://github.com/dempseyatgithub/BuildSettingExtractor
//

#include "Pods/Target Support Files/Pods-ElectrodeContainer/Pods-ElectrodeContainer.debug.xcconfig"


ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES
CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES

And we were to update ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES to NO, then current logic would just overwrite the existing file with just

ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = NO
CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES

Losing the comment header (which is not a huge deal), but also losing the #include statement (much more problematic).

This PR updates the logic to add/update build settings in an xcconfig file, so that only surgical patching is done to the file, to solely update what's needed, keeping the rest intact.

public async setBuildSettings(buildSettings: BuildSettings): Promise<BuildSettings> {
let configFileContent = (await exists(this.filePath)) ? (await readFile(this.filePath)).toString() : ''
if (configFileContent.length !== 0 && configFileContent[configFileContent.length-1] !== '\n') {
// Make sure that we have a new line at end of file for proper appending
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) 👍

Copy link
Member

@friederbluemle friederbluemle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍

src/XCConfig.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants