Add support for .swift-format-ignore files#1156
Add support for .swift-format-ignore files#1156bkhouri wants to merge 1 commit intoswiftlang:mainfrom
Conversation
8ba8eac to
23077c1
Compare
ef2c9fe to
aec3db3
Compare
|
@allevato : GitHub is not allowing me to directly reply to one of you comments,
I removed the locking mechanism, and also marked the |
|
@rintaro : I am unable to reply directly to one of your comments
I removed the locking mechanism, and also marked the |
aec3db3 to
d4662ec
Compare
| // Apply ignore filtering | ||
| if self.ignoreManager.shouldIgnore(file: item, isDirectory: false) { | ||
| output = nil | ||
| continue // Skip this file and continue to next | ||
| } |
There was a problem hiding this comment.
This will be checked in next() again. I don't think we need this check here.
There was a problem hiding this comment.
in next(), we check whether directories should be ignored. Here, we are checking whether files should be ignored. Both are needed.
There was a problem hiding this comment.
You removed the another check for files in next().
That means swift-format path/to/File.swift would perform formatting the specified file even if that file is .swift-format-ignore. Is that the desired behavior?
If so, what about the directories? E.g. swift-format --recursive path/to/Sources/MyLibrary/generated/ Should this directory be formatted even if it's ignored in .swift-format-ignore in the parent directories?
There was a problem hiding this comment.
If a file matches a pattern found in .swift-format-ignore file, it should not be formatted unless the pattern is negated.
A pattern in .swift-format-ignore can apply to files, or to directories.
| urls: [URL], | ||
| followSymlinks: Bool, | ||
| workingDirectory: URL = URL(fileURLWithPath: "."), | ||
| ignoreManager: IgnoreManager |
There was a problem hiding this comment.
| ignoreManager: IgnoreManager | |
| ignoreManager: IgnoreManager = IgnoreManager() |
then we don't need the additional init(urls:followSymlinks:workingDirectory:) below. Though I still not sure if this needs to be injectable.
There was a problem hiding this comment.
I kept this here in the event we want to have dependency injection testing. I will change the access modifier to package
2ba9d9d to
0207c96
Compare
7fbf1b4 to
331ddb4
Compare
| if currentDir.isRoot { | ||
| break // We've reached the root |
There was a problem hiding this comment.
I thought you once implemented to make "baseDirectory" the path having .swift-format file. But apparently you removed it? Now baseDirectoryCache: [URL: URL] is not used anymore.
There was a problem hiding this comment.
Stopping at .swift-format directory is important because otherwise the behavior would be unstable. For example the root directory / might have .swift-format-ignore ignoring everything. The tests would fail in such machines.
There was a problem hiding this comment.
I re-introduced this, and added automated test cases that verify some integration with .swift-format and .swift-format-ignore. Thanks for bringing this my attention.
5772f0d to
2ee590a
Compare
Add supports for file-based ignoring using `.swift-format-ignore` files. These files allow a developer to specify patterns for files and directories that should be completely excluded from formatting and linting operations. Fixes: swiftlang#870
2ee590a to
6c614da
Compare
Add supports for file-based ignoring using
.swift-format-ignorefiles. These files allow a developer to specify patterns for files and directories that should be completely excluded from formatting and linting operations.Fixes: #870