search

IBDecodable / IBLinter

A linter tool for Interface Builder
MIT License
950 stars 40 forks source link

Support dynamic paths #135

Closed jml5qh closed 2 years ago

jml5qh commented 4 years ago

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.

jml5qh commented 4 years ago

@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.

jml5qh commented 4 years ago

Could you show me the equivalent code on swiftlint, with same option name?

SwiftLint has a --path Option that you can use for one file (https://github.com/realm/SwiftLint/blob/master/Source/swiftlint/Commands/LintCommand.swift#L43) and then also a non-named Argument that you can use for multiple files (https://github.com/realm/SwiftLint/blob/master/Source/swiftlint/Commands/LintCommand.swift#L67). I first tried the Argument approach, but had issues with file paths with spaces.

phimage commented 4 years ago

We have a --path option already but maybe you cannot specify a file, only directory (and then it could do change how to find config file)

It will be better if we could work like swiftlint maybe the paths argument as you mentionned https://github.com/realm/SwiftLint/blob/c7c0ac838b8acd59f4082e913a014a3989fe042e/Source/swiftlint/Helpers/CommonOptions.swift#L10

jml5qh commented 4 years ago

We have a --path option already but maybe you cannot specify a file, only directory (and then it could do change how to find config file)

It will be better if we could work like swiftlint maybe the paths argument as you mentionned https://github.com/realm/SwiftLint/blob/c7c0ac838b8acd59f4082e913a014a3989fe042e/Source/swiftlint/Helpers/CommonOptions.swift#L10

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"].

kateinoigakukun commented 4 years ago

I basically prefer to use arguments instead of option to specify files.

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"].

@jml5qh I think it's not Commandant issue, you can pass path including space if you surround it with " or escape the space with \.

jml5qh commented 4 years ago

I basically prefer to use arguments instead of option to specify files.

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"].

@jml5qh I think it's not Commandant issue, you can pass path including space if you surround it with " or escape the space with \.

Ah you're exactly right - just switched it to use arguments.

jml5qh commented 4 years ago

@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.