Skip to content

Rework CLI input and scope handling#33

Open
NickSdot wants to merge 2 commits into
php:mainfrom
NickSdot:stack-02-cli-scope
Open

Rework CLI input and scope handling#33
NickSdot wants to merge 2 commits into
php:mainfrom
NickSdot:stack-02-cli-scope

Conversation

@NickSdot

Copy link
Copy Markdown
Contributor

Makes diff the default input, detects piped diffs automatically, adds --wide, and separates input planning from execution.

Depends on #29, #30, #31, and #32.
This PR diff will shrink as preceding PRs are merged and this branch is rebased.

As discussed --diff-behaviour is now the default. Piping a diff is still possible and detected automatically -- it no longer requires a --diff flag and diffs can be piped, hence --diff had no more function and was removed. Passing paths works as before. Since diff is not default, full runs are now covered by either docbook-cs . or docbook-cs path/docs-root/. What we discussed as --greedy is now --wider. The table shows how it works.

Input --wide Full File(s) References Meaning
none diff against branch point
none files and all referenced XML
<path> full file or directory
<path> full file or directory plus referenced XML
git diff | … piped diff scope
git diff | … piped diff files and referenced XML

@NickSdot
NickSdot force-pushed the stack-02-cli-scope branch from c26bc1e to 78d850b Compare July 22, 2026 07:39
@NickSdot
NickSdot marked this pull request as ready for review July 22, 2026 07:39
@NickSdot

Copy link
Copy Markdown
Contributor Author

Rebased and ready for review.

Comment thread src/Application.php
* @throws \RuntimeException if redirected stdin cannot be read.
* @api
*/
public static function fromGlobals(array $argv): self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

withArguments would be better since Globals could mean something different.

Comment thread src/Application.php
array $argv,
mixed $stdout = null,
mixed $stderr = null,
?string $stdin = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Having $stdin as a string variable doesn't make sense. We should rename this to $diff or $diffContents.

Comment thread src/Application.php
try {
$runner = new SniffRunner($progress);
$report = $runner->run($config, $overridePaths, $diff);
$report = $runner->run($runPlan);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$runPlanner

Comment thread phpunit.xml.dist
<directory>tests/Integration</directory>
</testsuite>
<testsuite name="feature">
<directory>tests/Feature</directory>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As mentioned in an earlier PR, I prefer to have this in Unit as well.

}

$candidates[] = sprintf('refs/remotes/%s/main', $remote);
$candidates[] = sprintf('refs/remotes/%s/master', $remote);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is checking remote. Which wouldn't be a bad thing to do. But when having multiple remotes (origin & upstream) which one wil this choose?

I did notice that my upstream was not up-to-date with origin. Docbook-cs did report false positives because of that. For that reason it might be better to check locally.

For branches, check with the master/main. For the master, check the current uncommitted (or not pushed) changes.

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.

2 participants