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

Issued around tput #6

Closed
gregoa opened this issue Sep 10, 2019 · 8 comments
Closed

Issued around tput #6

gregoa opened this issue Sep 10, 2019 · 8 comments

Comments

@gregoa
Copy link

gregoa commented Sep 10, 2019

Besides the problem with the shell redirection, which I already reported at https://rt.cpan.org/Public/Bug/Display.html?id=130463, there are more problems with the use of tput.

As https://bugs.debian.org/939939 and the linked log at https://ci.debian.net/data/autopkgtest/testing/amd64/libt/libterm-choose-perl/2922560/log.gz show, tput fails if TERM is not set. Interestingly there is a difference between tput -V and other tput calls:

% env -u TERM tput -V ; echo $?                          
ncurses 6.1.20181013                            
0
% env -u TERM tput ; echo $?
tput: No value for $TERM and no -T specified
2
% env -u TERM tput rev ; echo $?
tput: No value for $TERM and no -T specified
2

This means that the failures don't come from the tpuz -V in the if() but from the actual calls later in the code.

When looking at the code in lib/Term/Choose/Screen.pm I was a bit confused; is the logic of the if()in line 37 really how it's meant to be? If I'm reading this correctly, what's happening is (in pseudo-code):

if (we are on windows or TC_ANSI_ESCAPES is set or we have a working tput) {
    use ansi escapes
} else {
    use tput
}

Shouldn't this be or we don't have a working tput? -- Maybe something like the following might work (untested):

--- a/lib/Term/Choose/Screen.pm
+++ b/lib/Term/Choose/Screen.pm
@@ -34,7 +34,7 @@ my (


 BEGIN {
-    if ( $^O eq 'MSWin32' || $ENV{TC_ANSI_ESCAPES} || system( "tput -V &> /dev/null" ) ) {
+    if ( $^O eq 'MSWin32' || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || ! system( "tput -V >/dev/null 2>&1" ) ) {
         @up    = ( "\e[", "A" );
         @down  = ( "\e[", "B" );
         @right = ( "\e[", "C" );

Cheers,
gregor, Debian Perl Group

@kuerbis
Copy link
Owner

kuerbis commented Sep 10, 2019

Hi gregor, if I am not wrong system returns zero on successful execution.

Would id be OK to write it like this?

if ( $^O eq 'MSWin32' || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || system( "tput -V >/dev/null 2>&1" ) != 0 ) {

@gregoa
Copy link
Author

gregoa commented Sep 10, 2019

Hi gregor, if I am not wrong system returns zero on successful execution.

Oh, right :/

Would id be OK to write it like this?
if ( $^O eq 'MSWin32' || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || system( "tput -V >/dev/null 2>&1" ) != 0 ) {

I think this should work. -- Let's try a few things:

% env TERM=foo perl -E 'if ( $^O eq "MSWin32" || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || system( "tput -V >/dev/null 2>&1" ) != 0 ) { say "use ansi escapes"; } else { say "use tput"; }'
use tput
% env -u TERM perl -E 'if ( $^O eq "MSWin32" || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || system( "tput -V >/dev/null 2>&1" ) != 0 ) { say "use ansi escapes"; } else { say "use tput"; }'
use ansi escapes
% env TERM=foo perl -E 'if ( $^O eq "MSWin32" || $ENV{TC_ANSI_ESCAPES} || ! $ENV{TERM} || system( "trallala -V >/dev/null 2>&1" ) != 0 ) { say "use ansi escapes"; } else { say "use tput"; }'                                                         
use ansi escapes

Yeah, looks good :)

Thanks,
gregor

@kuerbis
Copy link
Owner

kuerbis commented Sep 11, 2019

If I use system( "tput cuu >/dev/null 2>&1" ) != 0 I could skip the ! $ENV{TERM}.
Would you recommend to use the ! $ENV{TERM} || system( "tput -V >/dev/null 2>&1" ) != 0 or to use system( "tput cuu >/dev/null 2>&1" ) != 0?

@gregoa
Copy link
Author

gregoa commented Sep 11, 2019 via email

@kuerbis
Copy link
Owner

kuerbis commented Sep 12, 2019

! qx(tput cuu 2>/dev/null) is a little shorter and looks a little simpler then system( "tput cuu >/dev/null 2>&1" ) != 0.
If there is something wrong with ! qx(tput cuu 2>/dev/null) please write a comment else I will use ! qx(tput cuu 2>/dev/null).

@gregoa
Copy link
Author

gregoa commented Sep 12, 2019 via email

kuerbis added a commit that referenced this issue Sep 12, 2019
… the undef and empty string. Bugix in line_fold: make colored output work with a single prompt line. Set col_width_plus only once.
@kuerbis
Copy link
Owner

kuerbis commented Sep 12, 2019

Thx!

@kuerbis kuerbis closed this as completed Sep 12, 2019
@gregoa
Copy link
Author

gregoa commented Sep 12, 2019 via email

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

2 participants