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

Get Data representation for Writable #787

Merged
merged 6 commits into from
Aug 30, 2023
Merged

Get Data representation for Writable #787

merged 6 commits into from
Aug 30, 2023

Conversation

Ibrahimhass
Copy link
Contributor

@Ibrahimhass Ibrahimhass commented Jul 31, 2023

Resolves #786

Short description 📝

Added an option to get the pbxproj's String representation.
I want to modify a pbxproj in memory to add it to an in-memory archive later.
This is needed because we are making a server-side Swift application.
Since it is a server-side application, it would be great to get the modified pbxproj without any extra disk I/O operations.
The current APIs can only achieve this by first writing the pbxproj to disk.

Solution 📦

Added a function inside the PBXProj extension, which can be used to get the String representation of the pbxproj file.
Also refactored the code to use the above function for better code reuse.

Implementation 👩‍💻👨‍💻

extension PBXProj: Writable {
    public func write(path: Path, override: Bool) throws {
        try write(path: path, override: override, outputSettings: PBXOutputSettings())
    }

    public func write(path: Path, override: Bool, outputSettings: PBXOutputSettings) throws {
        let output = try encodeAsString(outputSettings: outputSettings)
        if override, path.exists {
            try path.delete()
        }
        try path.write(output)
    }
    
    public func encodeAsString(outputSettings: PBXOutputSettings) throws -> String {
        let encoder = PBXProjEncoder(outputSettings: outputSettings)
        return try encoder.encode(proj: self)
    }
}

@netlify
Copy link

netlify bot commented Jul 31, 2023

Deploy Preview for xcodeproj ready!

Name Link
🔨 Latest commit f91f424
🔍 Latest deploy log https://app.netlify.com/sites/xcodeproj/deploys/64db7abee8cb840007561e78
😎 Deploy Preview https://deploy-preview-787--xcodeproj.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this @Ibrahimhass and for outlining the motivation behind it.

My initial impression is, while this achieves the desired goal it seems a bit ad-hoc - PBXProj is one of many types that get serialised to disk.

Perhaps a more wholistic approach would be to update Writable to include stringRepresentation() method such that all writable types behave consistently?

@Ibrahimhass
Copy link
Contributor Author

@kwridan Thanks for the suggestion :)
Looking further, I found that the underlying type for Writable here is Data.
So shall we have stringRepresentation() as an Optional requirement for Writable?

Something similar to this:

/// Protocol that defines how an entity can be written to disk
public protocol Writable {
    /// Writes the object that conforms the protocol.
    ///
    /// - Parameter path: The path to write to
    /// - Parameter override: True if the content should be overridden if it already exists.
    /// - Throws: writing error if something goes wrong.
    func write(path: Path, override: Bool) throws

    /// Writes the object that conforms the protocol.
    ///
    /// - Parameter pathString: The path string to write to
    /// - Parameter override: True if the content should be overridden if it already exists.
    /// - Throws: writing error if something goes wrong.
    func write(pathString: String, override: Bool) throws
    
    /// Gets  the string representation of the object  that conforms the protocol.
    ///
    /// - Throws: error if encoding to String goes wrong.
    func stringRepresentation() throws -> String
}

extension Writable {
    public func write(pathString: String, override: Bool) throws {
        let path = Path(pathString)
        try write(path: path, override: override)
    }
    
    public func stringRepresentation() throws -> String { "" }
}

@Ibrahimhass Ibrahimhass closed this Aug 8, 2023
@Ibrahimhass Ibrahimhass reopened this Aug 8, 2023
@kwridan
Copy link
Collaborator

kwridan commented Aug 9, 2023

@kwridan Thanks for the suggestion :)
Looking further, I found that the underlying type for Writable here is Data.
So shall we have stringRepresentation() as an Optional requirement for Writable?

Thanks for looking further into this would making dataRepresentation() be a better fit then?

@Ibrahimhass
Copy link
Contributor Author

@kwridan

Thanks for looking further into this. Would making dataRepresentation() be a better fit then?

Thanks for the suggestion. Yes, this could work, we modify Writable to something like:

/// Protocol that defines how an entity can be written to disk
public protocol Writable {
    /// Writes the object that conforms the protocol.
    ///
    /// - Parameter path: The path to write to
    /// - Parameter override: True if the content should be overridden if it already exists.
    /// - Throws: writing error if something goes wrong.
    func write(path: Path, override: Bool) throws

    /// Writes the object that conforms the protocol.
    ///
    /// - Parameter pathString: The path string to write to
    /// - Parameter override: True if the content should be overridden if it already exists.
    /// - Throws: writing error if something goes wrong.
    func write(pathString: String, override: Bool) throws
    
    /// Gets the data representation of the object that conforms to the protocol.
    ///
    /// - Throws: error if encoding to Data goes wrong.
    func dataRepresentation() throws -> Data
}

The representation can be used to get the string representation of any of the file if required.

@Ibrahimhass
Copy link
Contributor Author

Ibrahimhass commented Aug 9, 2023

@kwridan Please review the changes here.

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @Ibrahimhass, one small correction needed.

Sources/XcodeProj/Objects/Project/PBXProj.swift Outdated Show resolved Hide resolved
@Ibrahimhass Ibrahimhass changed the title Get String representation of PBXProj Get Data representation of PBXProj and the Writable Aug 15, 2023
@Ibrahimhass Ibrahimhass changed the title Get Data representation of PBXProj and the Writable Get Data representation for Writable Aug 15, 2023
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @Ibrahimhass

@kwridan
Copy link
Collaborator

kwridan commented Aug 18, 2023

@all-contributors add @Ibrahimhass for code

@allcontributors
Copy link
Contributor

@kwridan

I've put up a pull request to add @Ibrahimhass! 🎉

@pepicrft pepicrft merged commit 8206939 into tuist:main Aug 30, 2023
9 checks passed
kwridan pushed a commit that referenced this pull request Oct 12, 2023
…#798)

Resolves #793

- Add the ability to instantiate `PBXProj` from `Data` representation
- This complements #787 where projects can now be serialised to `Data` representation rather than to disk

Notes:

- `PBXProj.name` isn't serialised as part of the serialised data representation and will need to be manually updated
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.

Add an option to get the String representation of PBXProj
4 participants