Colors in cacheFileReport cannot be switched off

[amenk]

I am using

vendor/bin/phpcs --report="diff" --extensions="php" --no-colors --report-diff=var/phpcs_report_680bb28374ffe.diff

to see some progress. In this case, it was not possible to switch of colors in the diff file.

Eventually related to #787

Description

Allow switching of colors in report files

Suggested changelog entry

Allow switching of colors in report files

Related issues/external references

Fixes #

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] This change is only breaking for integrators, not for external standards or end-users.
• [ ] Documentation improvement

PR checklist

• [x] I have checked there is no other PR open for the same change.
• [ ] I have read the Contribution Guidelines.
• [x] I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• [ ] I have added tests to cover my changes.
• [ ] I have verified that the code complies with the projects coding standards.
• [ ] [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

@amenk Thank you for your PR and your willingness to contribute to PHPCS. Currently CI does not pass on this PR, this needs to be fixed before someone will look at the PR.

[amenk]

@jrfnl thanks, updated

[jrfnl]

@amenk Thanks for the PR. I've been trying to reproduce the issue, but am failing. I've also tried to see if this fixes #787, which I can reproduce, but no such luck either.

Please add more detailed reproduction steps to the PR as I currently cannot validate the supposed bug, nor the fix.

The way I've tried to reproduce this:

Test file test.inc:

<?PHP

echo   'hello'    ;

$var = 10+1;

Tests tried:

1. phpcs -ps ./test.inc --standard=squiz --report=diff --colors Result: coloured diff ✅
2. phpcs -ps ./test.inc --standard=squiz --report=diff --no-colors Result: diff without colours ✅
3. phpcs -ps ./test.inc --standard=squiz --colors --report-diff=test.diff Result: diff saved to file without colours ✅
4. phpcs -ps ./test.inc --standard=squiz --no-colors --report-diff=test.diff Result: diff saved to file without colours ✅
5. phpcs -ps ./test.inc --standard=squiz --report=diff --colors --report-file=test.diff Result: diff saved to file without colours ✅
6. phpcs -ps ./test.inc --standard=squiz --report=diff --no-colors --report-file=test.diff Result: diff saved to file without colours ✅

I've also tried with having both the --report=diff + the --report-diff=test.diff CLI arguments and that made no difference.

Next, I tried to reproduce this with the JSON report, so --report=json / --report-json=test.json and the colour codes are still in the report, both with, as well as without this patch.

Tested against master and this PR branch, using PHP 8.4.6 on Windows (but with a little hack to the Common::prepareForOutput() method (commenting out the Windows code) to be able to reproduce the json report issue).

[amenk]

@jrfnl thanks for the elaborate tests.

In the meanwhile I fixed all the violations in my codebase where this appeared. With minimal changes to have a violation, I no longer can reproduce it. Whenever - when I switch back to a older versions which had lots of violations and I cat the .diff while phpcs is still running, I see colors.

Anyways, my main motivation to get a color-less diff was, to be able to apply it via patch to fix the violations. Later I discovered, that there is already phpcbf which does this job much better.

So this MR is kind of obsolete, but there still might be cases hidden which lead to colors in the diff output.

Is the file maybe only cleaned after fully written?

[jrfnl]

Is the file maybe only cleaned after fully written?

It is indeed, though it depends on the report type used and such, but if there are colours to content with, the cleaning happens before the final file write.

Anyways, my main motivation to get a color-less diff was, to be able to apply it via patch to fix the violations. Later I discovered, that there is already phpcbf which does this job much better.

Ah, glad you found that 😉 And that it helped clean up the code base.

So this MR is kind of obsolete, but there still might be cases hidden which lead to colors in the diff output.

I'd be happy to look at it if someone comes up with a good reproducible case (like the json one), but for now, I can't see this PR fixing anything.

I'll close this PR now and wish you much success using PHP_CodeSniffer (and the fixer).