sindresorhus/refined-github

`no-useless-split-diff-view` is broken on GitHub Enterprise #4483

fregante posted onGitHub

here's how a recent diff looks with this feature enabled:

chrome_2021-06-17T090052

What do you think changed in this commit? Well it looks like that line was added - but it wasn't. The normal split diff looks like this:

chrome_2021-06-17T090115

You can see there's a subtle difference between the commits which is actually very important (there was a bug!)

Originally posted by @danielsamuels in https://github.com/sindresorhus/refined-github/issues/4399#issuecomment-863023847

Strange, can't replicate.

Diff that's correctly unified: https://github.com/sindresorhus/refined-github/commit/f60cb71e0bc2708ebda622d6c684489241fd367a

Reportedly problematic diff: https://github.com/sindresorhus/refined-github/commit/26e778fd85f7db6c3d5bc6f508e23c77f3ed4a20

Originally posted by @fregante in https://github.com/sindresorhus/refined-github/issues/4399#issuecomment-863172312

This is on a GitHub Enterprise instance running 2.22.10, perhaps it's an incompatibility with that version?

Originally posted by @danielsamuels in https://github.com/sindresorhus/refined-github/pull/4399#issuecomment-863173685

I think I know what's happening. It checks the left column first, it doesn't find the expected element ([data-split-side="${side}"]:is(.blob-code-addition, .blob-code-deletion)) and therefore it assumes it's empty.

Originally posted by @fregante in https://github.com/sindresorhus/refined-github/pull/4399#issuecomment-863175358


The side detection code needs to be rethought to avoid future issues with this. With the current code, if GitHub changes the selector, this bug will happen again.

For example, it should check whether the selector exists in at least one side. Alternative solutions welcome.

posted by fregante almost 4 years ago

and disabling this feature seems to have no effect... 🤷‍♂️

posted by simbo almost 4 years ago

Sorry about that, the feature was named no-useless-split-diff-view, not hide-empty-split-diff-side as its PR title suggested.

I disabled it globally.

posted by fregante almost 4 years ago

Fund this Issue

$0.00
Funded

Pull requests