Add Properties PBXFileElement #244

briantkelley · 2018-03-07T19:37:08Z

Short description 📝

PBXFileReference and PBXGroup share many properties. Consolidate them in PBXFileElement and add additional unique properties to PBXFileReference.

Solution 📦

Refactor the PBXFileReference.includeInIndex, PBXGroup.usesTabs, PBXGroup.indentWidth, PBXGroup.tabWidth properties so they're implemented in PBXFileReference so they can be shared by both subclasses. Add PBXFileElement.wrapsLines, PBXFileReference.languageSpecificationIdentifier, and PBXFileReference.plistStructureDefinitionIdentifier.

Implementation 👩‍💻👨‍💻

Add PBXFileElement.wrapsLines property and parameters to the PBXFileElement, PBXFileReference, and PBXGroup initializers.
Move includeInIndex from PBXFileReference to PBXFileElement and add parameters to the PBXFileElement and PBXGroup initializers.
Move usesTabs from PBXGroup to PBXFileElement and add parameters to the PBXFileElement and PBXFileReference initializers.
Move indentWidth from PBXGroup to PBXFileElement and add parameters to the PBXFileElement and PBXFileReference initializers.
Move tabWidth from PBXGroup to PBXFileElement and add parameters to the PBXFileElement and PBXFileReference initializers.
Add languageSpecificationIdentifier and plistStructureDefinitionIdentifier to PBXFileReference and parameters to its initializer.
Simplify PBXFileReference.isEqual(to:) and PBXGroup.isEqual(to:) only compare properties the class implements.

This change does not remove any properties or initializer parameters on PBXGroup or PBXFileElement, so source compatibility should be maintained.

This change is

pepicrft · 2018-03-08T08:48:49Z

Sources/xcproj/PBXFileElement.swift

    // MARK: - Init

    /// Initializes the file element with its properties.
    ///
    /// - Parameters:
    ///   - sourceTree: file source tree.
    ///   - name: file name.
+    ///   - usesTabs: group uses tabs.


The documentation comments doesn't seem to match the parameters the constructor takes.

pepicrft · 2018-03-08T08:50:02Z

Sources/xcproj/PBXGroup.swift

@@ -25,19 +16,26 @@ final public class PBXGroup: PBXFileElement {
    ///   - sourceTree: group source tree.
    ///   - name: group name.
    ///   - path: group path.
+    ///   - wrapsLines: should the IDE wrap lines for files in the group?
    ///   - usesTabs: group uses tabs.
    public init(children: [String],


Same here. Would you mind adding them? 😬
I'll go through all the files on the project and make sure they are properly documented.

pepicrft

Some minor comments. Good idea to abstract those properties into the PBXFileElement class. There's something that I keep noticing and is the need of automating the equatable conformance and the constructors using something like Sourcery. As the models grow, it's very easy to introduce a bug in there (and you already found some in the code base).

yonaskolb · 2018-03-08T09:39:48Z

Are these new properties in PBXFileElement also applicable to the other subclasses? I can’t look them up right now, but I believe variant group is one

briantkelley · 2018-03-08T21:05:57Z

Good point @yonaskolb. I forgot to consider other subclasses. It seems Xcode implements the following properties in PBXReference:

path
name
includeInIndex
usesTabs
indentWidth
tabWidth
wrapsLines

PBXReference is the super class of both PBXFileReference and PBXGroup.

PBXGroup is the super class of both PBXVariantGroup and XCVersionGroup.

Those are the four classes in xcproj that directly or indirectly inherit from PBXFileElement so the change seems to be the correct behavior for all affected classes. But, given the new information, I'll also refactor PBXVariantGroup and XCVersionGroup to inherit from PBXGroup, which will simplify both.

yonaskolb · 2018-03-08T23:11:06Z

It seems PBXReference also inherrits from PBXContainerItem

yonaskolb · 2018-03-08T23:11:48Z

Though I’m not sure what it gets for that. A comment property? Where is that set?

briantkelley · 2018-03-09T01:07:46Z

We didn't have any instances of comments for these family of elements in our codebase, and I'm sure the UI for it has been removed if it ever existed. I can rebase this on top of #243 to set PBXContainerItem as the super class of PBXFileElement if you'd like.

Both PBXGroup and PBXFileReference support a wrapsLines property. Add support for unarchiving and archiving it from Xcode projects to prevent data loss.

Both PBXFileReference and PBXGroup may have the includeInIndex property, so move it from PBXFileReference to PBXFileElement so it can be shared by both classes.

Both PBXFileReference and PBXGroup implemented the usesTabs property. Extract it to the base class so both can share the same implementation.

PBXGroup implemented the indentWidth property. Extract it to the base class so it can be shared with PBXFileReference, which also supports the property.

PBXGroup implemented the tabWidth property. Extract it to the base class so it can be shared with PBXFileReference, which also supports the property.

Remove equality checks from the isEqual(to:) implementation that’s performed by the superclass. Now PBXFileReference and PBXGroup only check their unique properties.

Add some additional properties discovered while round tripping some Xcode projects through xcproj. There’s languageSpecificationIdentifier, which is different than xcLanguageSpecificationIdentifier, and plistStructureDefinitionIdentifier.

Refactor PBXVariantGroup so it’s a subclass of PBXGroup instead of PBXFileElement, better modeling the way Xcode handles this type.

Refactor XCVersionGroup so it’s a subclass of PBXGroup instead of PBXFileElement, better modeling the way Xcode handles this type.

Update PBXFileElement to be a subclass of PBXContainerItem to better reflect Xcode’s model, which also adds support for the comments property.

# Conflicts: # CHANGELOG.md

briantkelley · 2018-03-15T15:50:58Z

Thanks everyone!

briantkelley requested review from pepicrft and yonaskolb March 7, 2018 19:37

pepicrft reviewed Mar 8, 2018

View reviewed changes

briantkelley force-pushed the pbxfileelement-props branch from b9adf41 to 58c5a8d Compare March 8, 2018 22:33

briantkelley force-pushed the pbxfileelement-props branch from 58c5a8d to cffad76 Compare March 9, 2018 01:06

briantkelley added 11 commits March 9, 2018 08:45

Add wrapsLines property to PBXFileElement

bb47a6a

Both PBXGroup and PBXFileReference support a wrapsLines property. Add support for unarchiving and archiving it from Xcode projects to prevent data loss.

Promote includeInIndex to PBXFileElement

85300fe

Both PBXFileReference and PBXGroup may have the includeInIndex property, so move it from PBXFileReference to PBXFileElement so it can be shared by both classes.

Refactor usesTabs to PBXFileElement

3ec15a3

Both PBXFileReference and PBXGroup implemented the usesTabs property. Extract it to the base class so both can share the same implementation.

Refactor indentWidth to PBXFileElement

a3b38db

PBXGroup implemented the indentWidth property. Extract it to the base class so it can be shared with PBXFileReference, which also supports the property.

Refactor tabWidth to PBXFileElement

c79cf2e

PBXGroup implemented the tabWidth property. Extract it to the base class so it can be shared with PBXFileReference, which also supports the property.

Simplify PBXFileElement Subclasses isEqual(to:)

42654c6

Remove equality checks from the isEqual(to:) implementation that’s performed by the superclass. Now PBXFileReference and PBXGroup only check their unique properties.

Add Properties to PBXFileReference

b0e9f07

Add some additional properties discovered while round tripping some Xcode projects through xcproj. There’s languageSpecificationIdentifier, which is different than xcLanguageSpecificationIdentifier, and plistStructureDefinitionIdentifier.

Make PBXVariantGroup inherit PBXGroup

0d32a27

Refactor PBXVariantGroup so it’s a subclass of PBXGroup instead of PBXFileElement, better modeling the way Xcode handles this type.

Make XCVersionGroup inherit PBXGroup

7205e90

Refactor XCVersionGroup so it’s a subclass of PBXGroup instead of PBXFileElement, better modeling the way Xcode handles this type.

PBXFileElement is a PBXContainer Item

94a21eb

Update PBXFileElement to be a subclass of PBXContainerItem to better reflect Xcode’s model, which also adds support for the comments property.

Update CHANGELOG to describe the fixes in this Pull Request.

c9f2a12

# Conflicts: # CHANGELOG.md

briantkelley force-pushed the pbxfileelement-props branch from cffad76 to c9f2a12 Compare March 9, 2018 16:55

briantkelley mentioned this pull request Mar 15, 2018

Special Quote Cases for Commented Strings #245

Merged

3 tasks

pepicrft approved these changes Mar 15, 2018

View reviewed changes

briantkelley merged commit 2d80b77 into master Mar 15, 2018

pepicrft deleted the pbxfileelement-props branch March 16, 2018 05:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Properties PBXFileElement #244

Add Properties PBXFileElement #244

briantkelley commented Mar 7, 2018 •

edited by pepicrft

Loading

pepicrft Mar 8, 2018

pepicrft Mar 8, 2018

pepicrft left a comment

yonaskolb commented Mar 8, 2018

briantkelley commented Mar 8, 2018

yonaskolb commented Mar 8, 2018

yonaskolb commented Mar 8, 2018

briantkelley commented Mar 9, 2018

briantkelley commented Mar 15, 2018

Add Properties PBXFileElement #244

Add Properties PBXFileElement #244

Conversation

briantkelley commented Mar 7, 2018 • edited by pepicrft Loading

Short description 📝

Solution 📦

Implementation 👩‍💻👨‍💻

pepicrft Mar 8, 2018

Choose a reason for hiding this comment

pepicrft Mar 8, 2018

Choose a reason for hiding this comment

pepicrft left a comment

Choose a reason for hiding this comment

yonaskolb commented Mar 8, 2018

briantkelley commented Mar 8, 2018

yonaskolb commented Mar 8, 2018

yonaskolb commented Mar 8, 2018

briantkelley commented Mar 9, 2018

briantkelley commented Mar 15, 2018

briantkelley commented Mar 7, 2018 •

edited by pepicrft

Loading