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

Add API to search for files in a group with path. Handles non-unique filenames in groups #671

Closed
wants to merge 7 commits into from
52 changes: 41 additions & 11 deletions Sources/XcodeProj/Objects/Files/PBXGroup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public extension PBXGroup {
}

/// Returns the file in the group with the given name.
/// - Note: Performs a linear search based on the file's name _only_. To search for a specific file
/// based on file-system location, use the `file(with path:)` method.
Comment on lines +120 to +121
Copy link
Author

Choose a reason for hiding this comment

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

Note on the limitations, instead of deprecation

///
/// - Parameter name: file name.
/// - Returns: file with the given name contained in the given parent group.
Expand All @@ -126,6 +128,19 @@ public extension PBXGroup {
.first(where: { $0.name == name })
}

/// Returns the file in the group with the given path, based on locations on-disk.
/// - Note: Paths function as-in Xcode; with respect to the group's `PBXSourceTree`.
/// Internally, identity is calculated based on normalized absolute paths for all files involved; i.e. actual
/// location in the file system.
///
/// - Parameters:
/// - path: a file path to search for.
/// - sourceRoot: path to project's source root.
/// - Returns: an existing reference to that file, or `nil` if not found
func file(with path: Path, sourceRoot: Path) -> PBXFileReference? {
try? existingFileReference(with: path, sourceRoot: sourceRoot)?.value
}

/// Creates a group with the given name and returns it.
///
/// - Parameters:
Expand Down Expand Up @@ -187,17 +202,11 @@ public extension PBXGroup {
}
let groupPath = try fullPath(sourceRoot: sourceRoot)

if override, let existingFileReference = try projectObjects.fileReferences.first(where: {
// Optimization: compare lastComponent before fullPath compare
guard let fileRefPath = $0.value.path else {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
let fileRefLastPathComponent = fileRefPath.split(separator: "/").last!
if filePath.lastComponent == fileRefLastPathComponent {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
return false
}) {
if override, let existingFileReference = try self.existingFileReference(
from: projectObjects,
with: filePath,
sourceRoot: sourceRoot
Comment on lines -190 to +208
Copy link
Author

Choose a reason for hiding this comment

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

Having spent some time going round and round on this, trying to create comprehensive tests, whilst still being slightly confused by how PBXSourceTrees actually work, I finally realised that the functionality needed is actually already here. It's only necessary to extract is and create a public driver method @kwridan @damirdavletov.

Therefore I decided not to assume too much about my knowledge of this lib, or PBXProj in general, and simply assume that this functionality works and expose it as we require. Seems much simpler. I can't guarantee it previously worked, but it should not break anything either...?

) {
if !childrenReferences.contains(existingFileReference.key) {
existingFileReference.value.path = groupPath.flatMap { filePath.relative(to: $0) }?.string
childrenReferences.append(existingFileReference.key)
Expand Down Expand Up @@ -233,3 +242,24 @@ public extension PBXGroup {
return fileReference
}
}

private extension PBXGroup {

func existingFileReference(
from objects: PBXObjects? = nil,
Copy link
Author

Choose a reason for hiding this comment

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

Since the original already accessed a PBXObjects, I thought to at least keep that as an optional arg. It seems to be just a constant reference, but playing it safe here:

func objects() throws -> PBXObjects {
guard let objects = reference.objects else {
let objectType = String(describing: type(of: self))
throw PBXObjectError.orphaned(type: objectType, reference: reference.value)
}
return objects
}

with filePath: Path,
sourceRoot: Path
) throws -> (key: PBXObjectReference, value: PBXFileReference)? {
try (objects ?? self.objects()).fileReferences.first(where: {
// Optimization: compare lastComponent before fullPath compare
guard let fileRefPath = $0.value.path else {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
let fileRefLastPathComponent = fileRefPath.split(separator: "/").last!
if filePath.lastComponent == fileRefLastPathComponent {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
return false
})
Comment on lines +253 to +263
Copy link
Author

Choose a reason for hiding this comment

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

Moved from

if override, let existingFileReference = try projectObjects.fileReferences.first(where: {
// Optimization: compare lastComponent before fullPath compare
guard let fileRefPath = $0.value.path else {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
let fileRefLastPathComponent = fileRefPath.split(separator: "/").last!
if filePath.lastComponent == fileRefLastPathComponent {
return try filePath == $0.value.fullPath(sourceRoot: sourceRoot)
}
return false
}) {

}
}