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

Added a command line option to specify a path to lint #44

Merged
merged 8 commits into from
May 29, 2015

Conversation

larslockefeer
Copy link
Contributor

This pull request adds a command-line option to specify an (absolute or relative) path to a file or directory to lint.

This enables use cases such as:

  • Linting only the file that you are currently working on
  • Linting only modified files in an Xcode Run Script build phase
  • Linting modified files as a pre-commit hook

Usage examples:

  • swiftlint lint --path Source/swiftlint/LintCommand.swift
  • swiftlint lint --path Source/SwiftLintFramework

This PR relates to issues #6, #16 and #20

Usage example: `swiftlint lint --path
Source/swiftlint/LintCommand.swift`
@jpsim
Copy link
Collaborator

jpsim commented May 28, 2015

Thanks for the PR, @larslockefeer! This is definitely on the right track, but needs a bit of work:

  1. Please run SwiftLint on LintCommand.swift itself, as a number of your additions are causing violations. I'll be sure to add this to our CONTRIBUTING.md document once we add one :).
  2. Please add a single newline at the end of the file. We have a SwiftLint rule for this, but it's not working at the moment (see TrailingNewlineRule.swift doesn't seem to work #43).
  3. I'd rather you used NSString.absolutePathRepresentation(rootDirectory:) which is already available in SourceKittenFramework, or at least NSString.absolutePath rather than combining path to currentDirectoryPath

Otherwise, I have a few minor comments that I'll add inline to your PR now.

Again, thanks for doing this!


static func evaluate(m: CommandMode) -> Result<LintOptions, CommandantError<()>> {
return create
<*> m <| Option(key: "path", defaultValue: "", usage: "theff path to the file or directory to lint")
Copy link
Collaborator

Choose a reason for hiding this comment

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

theff should be the

@larslockefeer
Copy link
Contributor Author

Thanks for the feedback @jpsim!

I have processed your comments and synced with the upstream again.

I have discovered one additional issue after syncing with the upstream: files are now linted twice. I suspect that it is due to fileManager.allFilesRecursively yielding both contentsOfDirectoryAtPath and subpathsOfDirectoryAtPath but I haven't looked into that in detail yet.

Also, I see that the tests failed. On my machine, they ran fine so this could be an issue with Travis. I see the following error in their console output:

The command "script/cibuild" exited with 2.

@segiddins
Copy link
Contributor

Failure looks unrelated, I restarted the build.

}
}

func filesToLintAtPath(path: String) -> [String] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this function could be simplified:

func filesToLintAtPath(path: String) -> [String] {
    let absolutePath = path.absolutePathRepresentation()
    var isDirectory: ObjCBool = false
    if fileManager.fileExistsAtPath(absolutePath, isDirectory: &isDirectory) {
        if isDirectory {
            return fileManager.allFilesRecursively(directory: absolutePath).filter {
                $0.isSwiftFile()
            }
        } else if standardizedPath.isSwiftFile() {
            return [standardizedPath]
        }
    }
    return []
 }

@jpsim
Copy link
Collaborator

jpsim commented May 29, 2015

This is great, @larslockefeer. My final comment is about perhaps simplifying filesToLintAtPath(_:). It would also be great if you could add an entry to CHANGELOG.md. Thanks for this.

@larslockefeer
Copy link
Contributor Author

Thanks again @jpsim, I have processed your feedback.

jpsim added a commit that referenced this pull request May 29, 2015
Added a command line option to specify a path to lint
@jpsim jpsim merged commit e338fcc into realm:master May 29, 2015
@jpsim
Copy link
Collaborator

jpsim commented May 29, 2015

Hurray! Great work.

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