Skip to content

Add common configuration for swift-format #73

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

Closed
wants to merge 1 commit into from
Closed

Add common configuration for swift-format #73

wants to merge 1 commit into from

Conversation

denil-ct
Copy link
Contributor

Motivation

Fixes this issue

Modifications

  • Removed the configuration defined in SwiftFormat.swift.
  • Used the json configuration file in the project root to generate the configuration object.

Result

A single configuration file for swift-format, which will be easier to handle.

Comment on lines +20 to +24
let configurationFilePath = #file
.components(separatedBy: "/")
.prefix(while: { $0 != "Sources" })
.joined(separator: "/")
.appending("/.swift-format")
Copy link
Contributor Author

@denil-ct denil-ct Jun 18, 2023

Choose a reason for hiding this comment

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

I feel like this is quite susceptible to be triggered by other folders that have a Sources in their name. I started with it because it seemed the simplest. What is your opinion, am I being overly cautious here?

Maybe something like this might work better as it narrows down the search criteria?

Suggested change
let configurationFilePath = #file
.components(separatedBy: "/")
.prefix(while: { $0 != "Sources" })
.joined(separator: "/")
.appending("/.swift-format")
let configurationFilePath = #file
.components(separatedBy: "swift-openapi-generator/Sources")[0]
.appending("swift-openapi-generator/.swift-format")

Or is there some other way of getting the file location, that I am missing?

@czechboy0
Copy link
Contributor

czechboy0 commented Jun 18, 2023

Hi @denil-ct, thanks for picking this one up!

While the approach of finding the format file on disk would work when the source package is cloned in some local directory, but we also have to consider the use case where the CLI is built as a binary, transferred to another machine, and executed there. In that scenario, we still need to support formatting as part of the code generation pipeline.

What might be worth investigating is how we could capture the format file at compile time and embed it in the binary, so that it's then always available, even when not distributed as a source package.

There's some precedent in embedding an Info.plist for a CLI into the binary, so maybe that's a place to start looking. Possibly having an internal build tool plugin that reads the file and generates a variable that contains the string representation is another. I'll let you investigate these and propose which one you think works best.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Marking as needs work as this doesn't cover the use case where the CLI is invoked outside of the generator source package.

@simonjbeaumont
Copy link
Collaborator

Thanks @denil-ct for jumping in again!

What might be worth investigating is how we could capture the format file at compile time and embed it in the binary, so that it's then always available, even when not distributed as a source package.

There's some precedent in embedding an Info.plist for a CLI into the binary, so maybe that's a place to start looking. Possibly having an internal build tool plugin that reads the file and generates a variable that contains the string representation is another. I'll let you investigate these and propose which one you think works best.

Can't we use Swift PM for this. IIRC you can provide a resources: parameter in the target definition? Something like this:

...
    targets: [
...
        .executableTarget(
            name: "swift-openapi-generator",
            dependencies: [ ... ],
            resources: [
                .process(".swift-format"),
            ],
        ),
...
    ]

@czechboy0
Copy link
Contributor

@simonjbeaumont IIRC, that'll create a resource bundle next to the binary, and if you transfer the binary to another machine without explicitly also transferring the resource bundle, it won't find it at runtime. But might require a confirmation in case I misremembered how it works.

So it'd work for a framework or an app target (it'd get embedded in that bundle), but not for an executable binary CLI that can be distributed on its own, as it doesn't get distributed as a bundle.

@czechboy0
Copy link
Contributor

While trying @simonjbeaumont's suggestion, I stumbled upon a new SwiftPM 5.9 feature that allows embedding files in code: #40 (comment)

@denil-ct
Copy link
Contributor Author

Shall I close this PR, then? @czechboy0

@denil-ct
Copy link
Contributor Author

Closing this PR for now, as we are blocked on this.

@denil-ct denil-ct closed this Jun 19, 2023
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.

3 participants