Closed DabeDotCom closed 1 year ago
Hi Dabe - I've always tried to avoid external dependencies wherever possible. I see what you're trying to do here, but I'm not really very keen on merging it. There's clearly an issue with wide characters, but that code is already a bit of a hack (and likely better handled using an upstream tool that actually understands the diff, rather than something like colordiff which instead tries to interpret meaning solely from the text output). That OK?
I see what you're trying to do here, but I'm not really very keen on merging it. There's clearly an issue with wide characters, but that code is already a bit of a hack
I've always tried to avoid external dependencies wherever possible [...] and likely better handled using an upstream tool that actually understands the diff, rather than something like colordiff which instead tries to interpret meaning solely from the text output
I agree third-party dependencies like Text::Tabs
are easy enough to inline, but I figured Text::VisualWidth
(::PP
) has a little bit more "meat" on it... (And even it isn't perfect; \N{VARIATION SELECTOR-16}
, for instance, will always be a mess. «sigh»)
But I'm a firm believer in, "Just Because You Can't Do *EVERY**thing* Doesn't Mean You Can't Do *ANY**thing*..."
I'm more than happy running my own patched copy, locally... Thanks for providing such excellent shoulders to stand on, Dave! :-D
I noticed colordiff was getting confused by UTF-8 / Unicode — in particular,
diff -y
takes "wide" characters into account when aligning its>|<
marker, butcolordiff
was miscalculating where to look for that column.N.B. I wasn't sure how you'd feel about 3rd-party dependencies like
Text::VisualWidth::PP
... You'll either think this is a reasonably clever fall-back — though it doesn't fix the problem in all cases — or you'll throw up in your mouth a little; I'm fine with either! 🙃Thanks!