apply_change_to_project: Fix adding subproject that already exist. #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
byohay
commented
Aug 3, 2021
byohay
commented
- apply_change_to_project: Differentiate find file and reference proxy.
- apply_change_to_project: Fix adding existing subproject.
- Kintsugi: Fix adding build file to a file with multiple references.
byohay
changed the title
Since the caller knows whether they need to find a file or a reference proxy it doesn't make sense to have all functions call the same methods.
byohay
force-pushed
the
feature/add-existing-subproject
branch
from
6af0035 to
011552c
Compare
ashdnazg
reviewed
Oct 26, 2021
| def find_file(project, file_reference_change) | ||
| project.files.find do |file_reference| | ||
| next file_reference.path == file_reference_change["path"] | ||
| def find_file(project, file_reference_change, filter_file: ->(_) { true }) |
There was a problem hiding this comment.
maybe filter or filter_with or filter_closure instead of filter_file since it sounds like the parameter is a file.
There was a problem hiding this comment.
changed to file_filter.
ashdnazg
reviewed
Oct 26, 2021
| raise "Unsupported file reference change of type #{file_reference["isa"]}." | ||
| def find_file(project, file_reference_change, filter_file: ->(_) { true }) | ||
| file_references = project.files.select do |file_reference| | ||
| next file_reference.path == file_reference_change["path"] && |
ashdnazg
reviewed
Oct 26, 2021
| end | ||
|
|
||
| def find_reference_proxy(project, container_item_proxy_change) | ||
| def find_reference_proxy(project, container_item_proxy_change, filter_reference: ->(_) { true }) |
There was a problem hiding this comment.
filter_reference is also confusing
byohay
force-pushed
the
feature/add-existing-subproject
branch
2 times, most recently
from
04ca906 to
0c6c1df
Compare
|
@ashdnazg squashed. |
When a subproject is added, its file reference is used in several places: - It's added to a `PBXGroup`. - It's added to the dictionary of project references under `projectRef`. - A new `PBXContainerItemProxy` is added and it refers to it under `containerPortal`. Beforehand, when a subproject that already exists was added, `find_file` would not be able to distinguish between the existing and new reference, and it might refer to it in all places mentioned above, which would make an invalid project file. To distinguish between them, the file reference that is used is the one that doesn't exist in the project references.
byohay
force-pushed
the
feature/add-existing-subproject
branch
from
0c6c1df to
4e5f7a6
Compare
ashdnazg
reviewed
Oct 26, 2021
| # the other one without, this method will prefer to take the one without the build file. | ||
| # This assumes that it's preferred to have a file reference with build file than a file | ||
| # reference without/with two build files. | ||
| filter_files_without_build_files = lambda do |reference| |
There was a problem hiding this comment.
why not filter_reference_without_build_files?
Previously, if two file references that refer to the same file existed, a build file's reference won't prefer one over the other. Even though it is legal, a 1:1 relationship between build file and file reference usually exist. Therefore the logic of adding a build file changes to prefer a file reference that has no build files.
byohay
force-pushed
the
feature/add-existing-subproject
branch
from
4e5f7a6 to
ffb9654
Compare
|
@ashdnazg done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.