allow searching/filtering linters displayed by name.
Details
Details
- Reviewers
joshuaspence epriestley - Group Reviewers
Blessed Reviewers - Commits
- rARC1ed98937c461: arc linters: Filter by name, make display one-line
$ arc linters --verbose --search JSon --search ruby (Lots of text about many linters)
$ arc linters --search JSon ruby (error message)
$ arc linters python (no results, "try --search").
$ arc linters ruby (Verbose text about the "ruby" linter).
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
FWIW, I agree with the idea behind this diff. I would request a couple of minor changes however, although ultimately @epriestley has the final say.
| src/workflow/ArcanistLintersWorkflow.php | ||
|---|---|---|
| 58–59 | This should maybe move to a getAllLinters method. | |
| 70–72 | I think that this would be better written as: $search = $this->getArgument('search'); if ($search) { $linter_info = $this->filterByNames($linter_info, $search); } | |
| 216–230 | A couple of points:
| |
Comment Actions
- use stripos
- test before calling filter()
- improve doc
- extract getLintersInfo() out of run()
| src/workflow/ArcanistLintersWorkflow.php | ||
|---|---|---|
| 58–59 | I'm not 100% sold on this, but I gave it a go. | |
Comment Actions
LGTM
| src/workflow/ArcanistLintersWorkflow.php | ||
|---|---|---|
| 216 | These parameters should be type-hinted as you are assuming that they are arrays anyway. | |
Comment Actions
also search full description, so "arc linters python" shows flake8, and "arc linters javascript" shows something.
Comment Actions
The search feels a little weird to me as a default. In particular, I don't like that arc linters ruby may return a lot of results and there's no way to return exactly one hit (for example, if I'm debugging over IRC or something). What if we did something like this?
- Default (non-verbose) output is one linter per line.
- Arguments must match linter key case-insensitively (e.g., arc linters ruby can get info on a specific linter, even if many linters match "ruby").
- If you still want it, put search on a --search flag? And have the error say "No linters match query exactly, try --search." or whatever.
- Maybe arguments imply --verbose?
I don't feel strongly about this, the current signature just feels a little fuzzy and the relatively verbose default behavior of arc linters also doesn't seem especially useful.
Comment Actions
The primary use-case I was thinking about is "Is there a linter installed that I can use here", so a general search is what I'm after; currently, I'd end up with arc linters | less and grep there. I'd like a --search option, which acts like this change.
I agree with your suggestions, provided we make that one line a little more informative than it currently is (i.e., replace flake8 (Flake8) with flake8 (Comprehensive Python linter) and jshint (JSHint) with jshint (Javascript)). I think I'll do that anyway.
Comment Actions
- Make non-verbose text one-liners
- Use --search for search
- make arc linters ruby show only exact match, and be verbose.
Making --search imply --verbose looks a little over-kill.