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

feat: "deno lint" subcommand #6125

Merged
merged 7 commits into from Jun 8, 2020
Merged

feat: "deno lint" subcommand #6125

merged 7 commits into from Jun 8, 2020

Conversation

@bartlomieju
Copy link
Contributor

bartlomieju commented Jun 5, 2020

Ref denoland/deno_lint#78
Closes #1880

cli/flags.rs Outdated Show resolved Hide resolved
cli/main.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added this to the 1.1.0 milestone Jun 6, 2020
Cargo.lock Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Contributor Author

bartlomieju commented Jun 7, 2020

Current output using error formatting facilities from CLI as suggested by @nayeemrmn Screenshot 2020-06-07 at 22 59 31

@ry
ry approved these changes Jun 8, 2020
Copy link
Collaborator

ry left a comment

LGTM - just improve the help message before landing

colors::yellow(line.to_string()),
colors::yellow(col.to_string())
)
}

This comment has been minimized.

Copy link
@ry

ry Jun 8, 2020

Collaborator

It’s unfortunate that we have two implementations of format_location.. but I guess the other is in JS

cli/flags.rs Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Contributor Author

bartlomieju commented Jun 8, 2020

We should probably support running deno lint without arguments - just like deno fmt searches for default files. I guess we'll iterate on linter a bit more before releasing 1.1 (PTAL at 0.2.0 milestone in deno_lint https://github.com/denoland/deno_lint/issues?q=is%3Aopen+is%3Aissue+milestone%3A0.2.0)

@bartlomieju bartlomieju merged commit 0e9da7e into denoland:master Jun 8, 2020
7 checks passed
7 checks passed
test_release macOS-latest
Details
test_release windows-2019
Details
test_release ubuntu-16.04
Details
test_debug ubuntu-16.04
Details
bench ubuntu-16.04
Details
lint ubuntu-16.04
Details
license/cla Contributor License Agreement is signed.
Details
@bartlomieju bartlomieju deleted the bartlomieju:deno_lint branch Jun 8, 2020
@@ -259,6 +262,8 @@ pub fn flags_from_vec_safe(args: Vec<String>) -> clap::Result<Flags> {
upgrade_parse(&mut flags, m);
} else if let Some(m) = matches.subcommand_matches("doc") {
doc_parse(&mut flags, m);
} else if let Some(m) = matches.subcommand_matches("lint") {
lint_parse(&mut flags, m);

This comment has been minimized.

Copy link
@yasinaydinnet

yasinaydinnet Jun 17, 2020

[Trivial and unrelated]
I don't know Rust, but is there an early return in Rust? If there is, wouldn't it be better if the file was something like:

...
  if let Some(m) = matches.subcommand_matches("doc") {
    return doc_parse(&mut flags, m);
  }
  if let Some(m) = matches.subcommand_matches("lint") {
    return lint_parse(&mut flags, m);
  }

This comment has been minimized.

Copy link
@bartlomieju

bartlomieju Jun 17, 2020

Author Contributor

This method returns Result<Flags> while each of the methods returns ().

This comment has been minimized.

Copy link
@yasinaydinnet

yasinaydinnet Jun 17, 2020

Got it. My focus was not on the returning part but on possibility of eliminating this many if-elses :) Perhaps a switch statement?
Again, this is totally trivial and unrelated. I was just looking at that file generally. Awesome work btw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.