Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look into using difftastic for better diffs #57

Open
jimwins opened this issue Oct 25, 2024 · 6 comments
Open

Look into using difftastic for better diffs #57

jimwins opened this issue Oct 25, 2024 · 6 comments

Comments

@jimwins
Copy link
Member

jimwins commented Oct 25, 2024

As requested by @Girgias.

https://difftastic.wilfred.me.uk/

@Girgias
Copy link
Member

Girgias commented Nov 9, 2024

Possibly one immediate improvement would be to ignore whitespace changes? Might also need to do something like that for the (new) rev-check @alfsb

@alfsb
Copy link
Member

alfsb commented Nov 9, 2024

Although it has to do with revchecks, both doc-base and web-doc, as they generate links, I think it can be initially developed exclusively in web-doc, by extrincating diff capabilities from https://doc.php.net/revcheck.php into a separate prettydiff.php file, and going from there. A diff page that does not contain headers and navbar for all languages, already helps.

About whitespaces in diffs, I think this can be an option. The separated diff page may contain, for example, checkboxes for Colored output and Ignore whitespace, that if checked, causes the page to reload with new setup.

Whitespace is always a PITA, because it is always important. While it is good to be able to observe only textual differences, having a way to observe WS changes has also its value. For example, the pt_BR translation has a tool to trim right and sync left white space, on line basis, to automatize indentation syncing..

@alfsb
Copy link
Member

alfsb commented Nov 9, 2024

About specifically ignoring whitespace changes, it would be possible, but with caveats. Some internal whitespaces changes are important (for example, ws inside some tags that breaks automatic linking) and even changes that may or may not be synced in translations (for example) https://github.com/php/doc-en/pull/3837/files

But most or inter textual ws changes can be ignored. It's possible, for example, in the case of outdated files, to compare the contents of two versions of an en file, the last and the annotated in revtag, but trimming all lines and blank lines before comparison. If two "trimmed" files compare equals, there are no textual changes, and these files would not be marked outdated.

But it will slow, even more so as these cases of "line trim equals" accumulate. Some form of automatic hash bumping will be necessary after a while.

@jimwins
Copy link
Member Author

jimwins commented Nov 9, 2024

We're already using --ignore-space-at-eol for the diff, we could look at the other options the stock git diff has for ignoring whitespace changes.

Maybe we could also use --word-diff=plain or --word-diff=porcelain to show changed words instead of just lines.

@alfsb
Copy link
Member

alfsb commented Nov 9, 2024

The descriptions of --ignore-space-change/-b options are a bit misleading. It will ignore ws changes on existing ws runs, but will report insertion or remotion of ws runs. This suffices for detecting significant ws changes and also ignoring insignificant ws changes in most cases that we are interested in.

It will ignore ws changing within lines, and that's unfortunate.

An option like --ignore-space-at-sol exists only as patchs. Maybe exists a combination of -w and --word-diff-regex=, so that makes git consider one "word" any text between lines, with ws within but excluding ws after and before new lines, but neither my git or regex knowledge is high enough to create this regex.

@alfsb
Copy link
Member

alfsb commented Nov 10, 2024

git diff -b does a good job for (almost) all cases. I may implement some "only prefix/suffix" ws change on revcheck, to ignore some cases with it.

image

It also would be an inicial alternative for while, or if difftastic is not possible.

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

No branches or pull requests

3 participants