Support dynamic paths #135
Conversation
|
@kateinoigakukun @giginet - I'm not too familiar with the Github PR UI and can't figure out how to add you as reviewers. I also ran the build checks / tests in my fork (https://github.com/jml5qh/IBLinter/commit/04cbce1455fbcbb860a26aa97dc7b062b696af65/checks?check_suite_id=373173148), but I'm not sure how to get that result added to this PR. |
| @@ -37,7 +37,7 @@ extension Rules { | |||
| let violation: [Violation] = { | |||
| guard let baseClassesForElement = baseClasses[view.elementClass] else { return [] } | |||
| guard let customClass = view.customClass else { | |||
| let message = "CustomClass is not set to \(viewName(of: view))" | |||
| let message = "You must use a subclass of \(viewName(of: view))" | |||
There was a problem hiding this comment.
I can also make this part into a separate PR if you prefer. I was a little confused when I ran the linter and saw this error since it says something like "CustomClass is not set to UILabel" even though we actually want the developer to set the custom class to something other than UILabel. Open to suggestions / improvements!
| @@ -26,11 +26,14 @@ struct ValidateCommand: CommandProtocol { | |||
| fatalError("\(workDirectoryString) is not directory.") | |||
| } | |||
|
|
|||
| let config = Config(options: options) ?? Config.default | |||
| var config = Config(options: options) ?? Config.default | |||
There was a problem hiding this comment.
Maybe it is possible to do all the job in constructor and keep let keyword
There was a problem hiding this comment.
I couldn't figure out how to make that work since the included property actually gets set in the init(from decoder: Decoder) method which is automatically called via the YAMLDecoder object. Let me know if you have any ideas and I can give it a try!
| if config.disableWhileBuildingForIB && | ||
| ProcessInfo.processInfo.compiledForInterfaceBuilder { | ||
| return .success(()) | ||
| } | ||
| if config.included.isEmpty { | ||
| config.included = options.included |
There was a problem hiding this comment.
because all you do is modified config object from information defined by options
SwiftLint has a |
|
We have a --path option already but maybe you cannot specify a file, only directory It will be better if we could work like swiftlint |
I've never been able to get the pathsArgument to work with file paths that have spaces in them. For example, in our app we have something like: /TV App/TV.storyboard. The Commandant Argument splits this path into two strings, even when trying to escape the parameters - ["TV", "App/TV.storyboard"]. |
|
I basically prefer to use arguments instead of option to specify files.
@jml5qh I think it's not Commandant issue, you can pass path including space if you surround it with |
Ah you're exactly right - just switched it to use arguments. |
|
@kateinoigakukun @phimage - Any other changes you'd like to see before this PR can get merged? Our team would love to start using IBLinter but are holding off until we can use this change. |
The use case for this new flag is to only lint the files that have changed. This could help in larger projects where linting all xib/storyboard files could take a good amount of time. This is also a feature present in other linters like SwiftLint and xiblint.