Revert "Add highlighting for "osc diff" and similar commands"

[jengelh]

This reverts commit 42d778b.

Since that commit, I am constantly getting:

$ osc diff
"/tmp/tmpzc68zopu" may be a binary file.  See it anyway?

In the default openSUSE system, it is then shown like so:

ESC[1mIndex: omnibustype-jaldi-fonts.changesESC[0m
===================================================================
ESC[1m--- omnibustype-jaldi-fonts.changes       (revision 1)ESC[0m
ESC[1m+++ omnibustype-jaldi-fonts.changes       (working copy)ESC[0m

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: +0.01% 🎉

Comparison is base (99fb94e) 28.93% compared to head (70fe53a) 28.94%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1373      +/-   ##
==========================================
+ Coverage   28.93%   28.94%   +0.01%     
==========================================
  Files          45       45              
  Lines       17065    17051      -14     
==========================================
- Hits         4937     4935       -2     
+ Misses      12128    12116      -12     

Files Changed	Coverage Δ
osc/commandline.py	18.99% <0.00%> (ø)
osc/core.py	35.67% <ø> (+0.05%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dmach]

I'm trying to understand the root cause, because it works for me just fine on openSUSE Tumbleweed with PAGER=less.
What is your PAGER?
What is your version of openSUSE?
What are your locales? (Run: set | egrep '^LC_|^LANG')
If none of these helps to identify the root cause, could you make a reproducer in a container?

[jengelh]

$ cat /etc/os-release 
NAME="openSUSE Tumbleweed"
# VERSION="20230724"
ID="opensuse-tumbleweed"
ID_LIKE="opensuse suse"
VERSION_ID="20230724"
PRETTY_NAME="openSUSE Tumbleweed"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:opensuse:tumbleweed:20230724"
BUG_REPORT_URL="https://bugzilla.opensuse.org"
SUPPORT_URL="https://bugs.opensuse.org"
HOME_URL="https://www.opensuse.org"
DOCUMENTATION_URL="https://en.opensuse.org/Portal:Tumbleweed"
LOGO="distributor-logo-Tumbleweed"

$ echo $PAGER
less

$ locale
LANG=en_US.utf8
LC_CTYPE="en_US.utf8"
LC_NUMERIC="en_US.utf8"
LC_TIME="en_US.utf8"
LC_COLLATE=POSIX
LC_MONETARY="en_US.utf8"
LC_MESSAGES="en_US.utf8"
LC_PAPER=de_DE.UTF-8
LC_NAME="en_US.utf8"
LC_ADDRESS="en_US.utf8"
LC_TELEPHONE="en_US.utf8"
LC_MEASUREMENT=de_DE.UTF-8
LC_IDENTIFICATION="en_US.utf8"
LC_ALL=

the root cause

Easy:

osc diff | $PAGER : stdout is a pipe, the function highlight_diff [via isatty] in osc notices this and avoids emitting color codes.

osc diff: stdout is a tty, the function highlight_diff in osc notices this and does emit color codes. But now, osc diff is unconditionally invoking $PAGER anyway, so it bleeds color codes to $PAGER.

[dmach]

But now, osc diff is unconditionally invoking $PAGER anyway, so it bleeds color codes to $PAGER.

This statement is not valid. Nothing has changed in $PAGER invocation - it remained unconditional.
The color codes bleed was the goal of the original change.

I'd prefer to move forward, keep the color output and fix the error.
Unfortunately I'm still unable to reproduce the "may be a binary file" error.
Could you send me the exact steps reproducing the problem?

[jengelh]

still unable to reproduce the "may be a binary file" error. Could you send me the exact steps reproducing the problem?

I see less has some heuristics concerning a few specks of \e... try this larger output:

osc rdiff -r1:2 M17N:fonts/omnibustype-jaldi-fonts

[jengelh]

One can Ctrl-Z out of osc while it's showing and then manually invoke less as well before the tmpfile goes away.

less /tmp/tmpf4vjyg1z
<also binary>

but also:

perl -pe 's{\e}{\\e}g' </tmp/tmpf4vjyg1z >1
less 1
<binary problem is gone>

So bleeding color codes are the problem.

[dmach]

still unable to reproduce the "may be a binary file" error. Could you send me the exact steps reproducing the problem?

I see less has some heuristics concerning a few specks of \e... try this larger output:

osc rdiff -r1:2 M17N:fonts/omnibustype-jaldi-fonts

Interesting, this works just fine for me and the diff is displayed normally:

PAGER=less LANG=en_US.utf8 LC_CTYPE="en_US.utf8" LC_NUMERIC="en_US.utf8" LC_TIME="en_US.utf8" LC_COLLATE=POSIX LC_MONETARY="en_US.utf8" LC_MESSAGES="en_US.utf8" LC_PAPER=de_DE.UTF-8 LC_NAME="en_US.utf8" LC_ADDRESS="en_US.utf8" LC_TELEPHONE="en_US.utf8" LC_MEASUREMENT=de_DE.UTF-8 LC_IDENTIFICATION="en_US.utf8" LC_ALL= osc rdiff -r1:2 M17N:fonts/omnibustype-jaldi-fonts

[jengelh]

less-590 from Leap 15.x behaves differently than less-633 on TW.

[dmach]

less-590 from Leap 15.x behaves differently than less-633 on TW.

The problem seems to be elsewhere than in the version of the less package.
I've tried compiling and installing the TW version on Leap and it behaves identically.
The difference between the distros is that TW has the following env option set: LESS='-M -I -R'

[dmach]

I've submitted #1376 that should fix the problem on Leap.

[jengelh]

That looks like a good approach. Curious though, what if pager isn't less at all? e.g. PAGER=/usr/bin/joe (SUSE's staple editor apart from vim ;-) ).

[dmach]

That looks like a good approach. Curious though, what if pager isn't less at all? e.g. PAGER=/usr/bin/joe (SUSE's staple editor apart from vim ;-) ).

What's important, proper pages such as less or more work fine.
If there's a user that really uses an editor instead of a pager, I'll eventually fix their use case when they report an issue,
but I'm not planning to cover all edge cases just in case someone might want to shoot themselves in a leg.

[dmach]

Fixed in #1376