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

Ability to use custom header prefix #1389

Merged

Conversation

ilia3546
Copy link
Contributor

@ilia3546 ilia3546 commented Dec 10, 2024

This PR adds ability to add additional prefix before header in generated files

Problem

If you use SwiftLint rule "file_name", then you will catch "File Name Violation" errors/warnings for each generated file. Even if you have "swiftlint:disable all" after header.

Also we have swiftlint rule for file header pattern "file_header" which also throws a warning :(

Screenshot 2024-11-22 at 17 46 51

Solution

I found a similar problem in the SwiftLint repo - realm/SwiftLint#2277. Users suggest to add this line at the beginning of the file (at 0 line) // swiftlint:disable:this file_name.

But at the moment this line contains header comment // Generated using Sourcery .... And I thought that if we be able to add additional custom prefix before the header with the following line // swiftlint:disable:this file_name, then we could solve this problem.

@krzysztofzablocki
Copy link
Owner

can you update the param names etc? it using old that don't match what it's doing since we are no longer hiding

@ilia3546
Copy link
Contributor Author

ilia3546 commented Dec 16, 2024

can you update the param names etc? it using old that don't match what it's doing since we are no longer hiding

In this PR I created param --headerPrefix <custom-prefix>. In the old PR it was called --hideHeader. I checked this PR carefully for old params and it seems clear.

Do you mean that I should rename param --headerPrefix to something else?

@@ -118,8 +118,9 @@ func runCLI() {
Option<Path>("ejsPath", default: "", description: "Path to EJS file for JavaScript templates."),
Option<Path>("cacheBasePath", default: "", description: "Base path to Sourcery's cache directory"),
Option<Path>("buildPath", default: "", description: "Sets a custom build path"),
Flag("hideVersionHeader", description: "Do not include Sourcery version in the generated files headers.")
) { watcherEnabled, disableCache, verboseLogging, logAST, logBenchmark, parseDocumentation, quiet, prune, serialParse, sources, excludeSources, templates, excludeTemplates, output, isDryRun, configPaths, forceParse, baseIndentation, args, ejsPath, cacheBasePath, buildPath, hideVersionHeader in
Flag("hideVersionHeader", description: "Do not include Sourcery version in the generated files headers."),

Choose a reason for hiding this comment

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

I mean this

@@ -301,8 +303,9 @@ func runCLI() {
"""),
Option<Path>("cacheBasePath", default: "", description: "Base path to Sourcery's cache directory"),
Option<Path>("buildPath", default: "", description: "Sets a custom build path"),
Flag("hideVersionHeader", description: "Do not include Sourcery version in the generated files headers.")
) { disableCache, verboseLogging, logAST, logBenchmark, parseDocumentation, quiet, prune, serialParse, sources, excludeSources, templates, excludeTemplates, output, isDryRun, configPaths, forceParse, baseIndentation, args, cacheBasePath, buildPath, hideVersionHeader in
Flag("hideVersionHeader", description: "Do not include Sourcery version in the generated files headers."),

Choose a reason for hiding this comment

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

this

@krzysztofzablocki
Copy link
Owner

I marked the old code that doesn't seem should be part of this pr

@ilia3546
Copy link
Contributor Author

Oh, the old parameter hideVersionHeader is still unchanged, I just added a comma at the end to be able to add the new parameter :)

@krzysztofzablocki krzysztofzablocki merged commit 3d3a2be into krzysztofzablocki:master Dec 18, 2024
1 of 2 checks passed
@krzysztofzablocki
Copy link
Owner

Oh I misread it, forgot it existed ¯_(ツ)_/¯

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.

2 participants