On Thu, May 27, 2021 at 8:42 AM Nikita Popov <nikita....@gmail.com> wrote:
> The most prudent and BC-safe thing would be to add another function >> `var_dump_escaped()` or even to prefer/advise json_encode() when content >> safety is relevant, but additional type information is not (most of the >> uses of var_dump we have). Unfortunately, this doesn't actually fix the >> initial problem you stated, which is just getting useful data out of CI >> failures. >> > > Well, we had https://wiki.php.net/rfc/readable_var_representation a few > weeks ago, which is basically a better var_dump() with escaping and all -- > but you can tell how it went ;) > > I don't specifically remember that thread, but suddenly I feel like I can guess everything I need to know about the outcome. :( Rather than change the output at all, why not just have a post-process step >> in our test runner that transforms the output in the test report? Then we >> could be as aggressive as we want, going as far as escaping all >> non-printables plus backslash since at that point we're in it for the human >> readability (and knowing specific byte sequences is essential) and we break >> zero BC for anyone else? >> > > Transforming the test output (such that the transformed output is checked > in EXPECT) would indeed be a way to address the original motivation. > There's two caveats though: > 1. It does not address the issue for other uses of var_dump(). People > being confused by var_dump((array) $object) output because of the null > bytes in the property names is practically a PHP classic. Of course, fixing > this would take away from the authentic PHP experience :) > True, but neither (IMO) would just escaping null byte, which again I regard as a half-measure. It has all the negative impact of breaking BC (your point about extension phpts is a good one), without going far enough to address the human-readability problem for many common cases (CR/NL chief among them). Insert Karate Kid "squash like a bug" reference here. Escape it all, or escape none of it. > 2. Changes to run-tests.php behavior also affect extensions, and I think > this kind of change would be even harder to address for PECL extensions > than a simple var_dump() behavior change (where you can at least easily > fork two versions of the file for the tests that need it). > > Would it? The report at the end of the run is for human eyes, generally speaking. A human can interpret escaping (or not) as needed. Also, we could make the post-processing optional. run-test.php --escape-test-output Then we use that mode in CI where we need git-friendly diffs, and it works "normally" elsewhere. -Sara
