rtfpessoa / diff2html

Pretty diff to html javascript library (diff2html)
https://diff2html.xyz
MIT License
2.9k stars 280 forks source link

GitHub-ify Generated HTML #484

Closed Lordfirespeed closed 1 year ago

Lordfirespeed commented 1 year ago

There are some notable differences in the HTML generated by diff2html and GitHub. These changes cause the generated HTML to be much more similar in structure to GitHub's generated structure:

This makes applying different themes to diff2html using CSS overrides simpler and more effective. For example, before this change, it isn't possible to have different background colours for line numbers / line content as the line numbers don't take up the same height as the content (I should provide a screenshot, sorry 😬 )

I haven't updated the testcases to match the changes in generated structure yet - just want to check you're OK with the changes before I do.

Also, I'm sorry about the commit messages - I'm a bit of a git newbie 😓 - if you'd like me to change them and can tell me how, I'd love to fix them to comply with the contribution guidelines.

rtfpessoa commented 1 year ago

Overall improvements to the CSS are welcome since I am mostly a CSS newbie.

After taking a look to this changes seems like multiple things are broken (ie. double escaping, side-by-side break line, etc) so we need to be careful to avoid releasing bugs.

My suggestion is that you try to propose small changes so that it can be easier to review and to test since we need to keep support for the current setup. If you could split this PR would be ideal.

If you plan on adding support to GitHub style line breaks instead of scroll it should be behind a flag and the default should be the current scroll behavior.

Make sure you test with both line by line and side by side.