revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-29 17:00:58 +01:00
Code Issues Releases Wiki Activity

Remove "arc lint --only-changed"

Browse source 
Summary:
Ref T12996. See also T9749. This option is the opposite of `--lintall`, and means "don't show warnings on changed lines".

However, it's confusing (and I think no one uses it), and we can reasonably infer what you mean if you give us `--rev`. Simplify this logic a bit and choose the "all / only changed" behavior based on the only sensible thing we can reasonably do given the flags.

As with all the other cruft here, I'm pretty sure this came from FB.

Test Plan: Grepped for `only-changed`, `lintAll`, created this revision.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12996

Differential Revision: https://secure.phabricator.com/D18644
This commit is contained in:
epriestley 2017-09-25 11:35:20 -07:00
parent be7987b25a
commit a053fcc4d5
1 changed files with 21 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
src/workflow/ArcanistLintWorkflow.php
View file

@ -13,7 +13,6 @@ final class ArcanistLintWorkflow extends ArcanistWorkflow {
const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;
private $unresolvedMessages;
private $shouldLintAll;
private $shouldAmendChanges = false;
private $shouldAmendWithoutPrompt = false;
private $shouldAmendAutofixesWithoutPrompt = false;
@ -61,17 +60,6 @@ EOTEXT
'help' => pht(
'Show all lint warnings, not just those on changed lines. When '.
'paths are specified, this is the default behavior.'),
'conflicts' => array(
'only-changed' => true,
),
),
'only-changed' => array(
'help' => pht(
'Show lint warnings just on changed lines. When no paths are '.
'specified, this is the default.'),
'conflicts' => array(
'lintall' => true,
),
),
'rev' => array(
'param' => 'revision',
@ -177,25 +165,34 @@ EOTEXT
'--everything'));
}
if ($rev && $paths) {
throw new ArcanistUsageException(
pht('Specify either %s or paths.', '--rev'));
if ($rev !== null) {
$this->parseBaseCommitArgument(array($rev));
}
// Sometimes, we hide low-severity messages which occur on lines which
// were not changed. This is the default behavior when you run "arc lint"
// with no arguments: if you touched a file, but there was already some
// minor warning about whitespace or spelling elsewhere in the file, you
// don't need to correct it.
// NOTE: When the user specifies paths, we imply --lintall and show all
// warnings for the paths in question. This is easier to deal with for
// us and less confusing for users.
$this->shouldLintAll = $paths ? true : false;
// In other modes, notably "arc lint <file>", this is not the defualt
// behavior. If you ask us to lint a specific file, we show you all the
// lint messages in the file.
// You can change this behavior with various flags, including "--lintall",
// "--rev", and "--everything".
if ($this->getArgument('lintall')) {
$this->shouldLintAll = true;
} else if ($this->getArgument('only-changed')) {
$this->shouldLintAll = false;
$lint_all = true;
} else if ($rev !== null) {
$lint_all = false;
} else if ($paths || $everything) {
$lint_all = true;
} else {
$lint_all = false;
}
if ($everything) {
$paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles());
$this->shouldLintAll = true;
} else {
$paths = $this->selectPathsForWorkflow($paths, $rev);
}
@ -209,7 +206,7 @@ EOTEXT
// This is used so that the lint engine can drop warning messages
// concerning lines that weren't in the change.
$engine->setPaths($paths);
if (!$this->shouldLintAll) {
if ($lint_all) {
foreach ($paths as $path) {
// Note that getChangedLines() returns null to indicate that a file
// is binary or a directory (i.e., changed lines are not relevant).