irssi / scripts.irssi.org

Script Repository for Irssi
https://scripts.irssi.org
492 stars 234 forks source link

Perl 5.38 breaks Irssi locale [Negative repeat count does nothing at trackbar.pl line 435.] #857

Closed hrw closed 7 months ago

hrw commented 12 months ago

Upgraded to Fedora 39 Beta and irssi greets me with this:

Negative repeat count does nothing at /home/marcin/.irssi/scripts/autorun/trackbar.pl line 435.

09:45 marcin@puchatek:~$ irssi --version
irssi 1.4.4 (20230331 1404)
09:45 marcin@puchatek:~$ perl --version

This is perl 5, version 38, subversion 0 (v5.38.0) built for x86_64-linux-thread-multi
vague666 commented 12 months ago

You might need to rebuild irssi with this: https://github.com/irssi/irssi/pull/1474

hrw commented 12 months ago

It already has that patch.

ailin-nemui commented 12 months ago

it's easy to hide the warning with an extra check in the code, the curious question in which case the value would end up negative there 🤔

hrw commented 12 months ago

Switching between windows ends with:

10:48 Negative repeat count does nothing at /home/marcin/.irssi/scripts/autorun/trackbar.pl line 435.
10:48
10:48 Negative repeat count does nothing at /home/marcin/.irssi/scripts/autorun/trackbar.pl line 435.
10:48

Two occurrences each time.

vague666 commented 12 months ago

It would be interesting to see what value $times has at various points around line 435 Can you add a few print $times; after the declaration on line 429?

hrw commented 12 months ago

My terminal is 189x68 characters.

$width = 189 $length = -1 $times = -189

hrw commented 12 months ago

Altered 'line' function to set $length to be positive if it is negative:


sub line {
    my ($width, $time)  = @_;
    my $string = $config{string};
    $string = ' ' unless length $string;
    $time ||= time;

    Encode::_utf8_on($string) if is_utf8();
    my $length = c_length($string);
    $length = -$length if $length < 0;

    my $format = '';
    if ($config{print_timestamp}) {
        $format = $config{timestamp_str};
        $format =~ y/%/\01/;
        $format =~ s/\01\01/%/g;
        $format = strftime($format, localtime $time);
        $format =~ y/\01/%/;
    }

    my $times = $width / $length;
    $times += 1 if $times != int $times;
    my $style = "$config{style}";
    Encode::_utf8_on($style) if is_utf8();
    $format .= $style;
    $width -= c_length($format);
    $string x= $times;
    chop $string while length $string && c_length($string) > $width;
    return $format . $string;
}

Now works as expected.

ailin-nemui commented 12 months ago

why is the length negative in first place? what's the value of your $string ? sounds like a more fundamental bug/problem somewhere

the length is supposed to measure the length of the string, which should be at least 1...

is your locale configured correctly?

ailin-nemui commented 12 months ago

(that said, the new version of Irssi supports a better method to get the length which could be used in this script to replace Text::CharWidth::mbswidth---Irssi::string_width)

hrw commented 12 months ago

The string is "─" (probably the default as I do not remember changing it).

When it comes to locale it is PL_pl.UTF-8 as before.

hrw commented 12 months ago

Replacing c_length with length also makes it work.

ailin-nemui commented 12 months ago

can you run some more tests:

/exec locale
/script exec print Text::CharWidth::mbswidth("─")

?

hrw commented 12 months ago
11:30 LANG=pl_PL.UTF-8
11:30 LC_CTYPE="pl_PL.UTF-8"
11:30 LC_NUMERIC="pl_PL.UTF-8"
11:30 LC_TIME="pl_PL.UTF-8"
11:30 LC_COLLATE="pl_PL.UTF-8"
11:30 LC_MONETARY="pl_PL.UTF-8"
11:30 LC_MESSAGES="pl_PL.UTF-8"
11:30 LC_PAPER="pl_PL.UTF-8"
11:30 LC_NAME="pl_PL.UTF-8"
11:30 LC_ADDRESS="pl_PL.UTF-8"
11:30 LC_TELEPHONE="pl_PL.UTF-8"
11:30 LC_MEASUREMENT="pl_PL.UTF-8"
11:30 LC_IDENTIFICATION="pl_PL.UTF-8"
11:30 LC_ALL=
11:30 -!- Irssi: process 0 (locale) terminated with return code 0

And "-1" for second.

ailin-nemui commented 12 months ago

now at least we know where the -1 comes from but not why this function replies incorrectly.. one last idea from me:

/script exec use POSIX qw(locale_h); print "Perl character locale: " . setlocale(LC_CTYPE)
hrw commented 12 months ago

11:34 Perl character locale: pl_PL.UTF-8

hrw commented 12 months ago
11:35 marcin@puchatek:del$ cat test.pl
use Text::CharWidth qw(mbwidth mbswidth mblen);

print Text::CharWidth::mbswidth("─")
11:35 marcin@puchatek:del$ perl test.pl ;echo
1
11:35 marcin@puchatek:del$
ailin-nemui commented 12 months ago

that's pretty mysterious...

since you upgraded your system, did you also restart irssi? (I guess you did)

maybe it's also worth checking

print $INC{"Text/CharWidth.pm"}

does it differ in any way between system and from within irssi (/script exec)?

vague666 commented 12 months ago

I wonder if /set term_charset, /set recode, and/or /recode might affect perl in some way

hrw commented 12 months ago

/set term_charset:

11:44 [lookandfeel]
11:44                     term_charset UTF-8

/set recode:

11:44 [misc]
11:44                           recode ON
11:44           recode_autodetect_utf8 ON
11:44                  recode_fallback ISO-8859-2
11:44       recode_out_default_charset UTF-8
11:44             recode_transliterate OFF

/recode:

11:44 Target                         Character set

Which reminded me to disable recode as I no longer use non-utf8 channels.

ailin-nemui commented 12 months ago

I wonder if /set term_charset, /set recode, and/or /recode might affect perl in some way

at least recode shouldn't affect Perl

hrw commented 12 months ago

that's pretty mysterious...

since you upgraded your system, did you also restart irssi? (I guess you did)

Restarted whole system

maybe it's also worth checking

print $INC{"Text/CharWidth.pm"}

does it differ in any way between system and from within irssi (/script exec)?

11:46 /usr/lib64/perl5/vendor_perl/Text/CharWidth.pm
$ perl test.pl
/usr/lib64/perl5/vendor_perl/Text/CharWidth.pm
vague666 commented 12 months ago

Can you test /script exec use Text::CharWidth; print Text::CharWidth::mbswidth("─") with a clean config? start irssi with irssi --home ~/.irssi-new

hrw commented 12 months ago

11:55 -!-  ___           _
11:55 -!- |_ _|_ _ _____(_)
11:55 -!-  | || '_(_-<_-< |
11:55 -!- |___|_| /__/__/_|
11:55 -!- Irssi v1.4.4 - https://irssi.org
11:55 -!- Irssi: The following settings were initialized
11:55                        real_name Marcin Juszkiewicz
11:55                        user_name marcin
11:55                             nick marcin
11:55 -1
vague666 commented 12 months ago

Where is your irssi from? Did you build it yourself or is it a system package?

ailin-nemui commented 12 months ago

if I run LANG=C perl test.pl I get -1

that's why I think "something" with locales goes wrong, but no idea where/how/why

I think there are more things wrong but I fail to understand it. for example: if I type/paste 一一一 into irssi's prompt on fedora, it seems I cannot navigate properly with the arrow keys

hrw commented 12 months ago

Irssi binary comes from Fedora 39 repository:

12:17 marcin@puchatek:~$ rpm -qf $(which irssi)
irssi-1.4.4-5.fc39.x86_64
hrw commented 12 months ago

I think there are more things wrong but I fail to understand it. for example: if I type/paste 一一一 into irssi's prompt on fedora, it seems I cannot navigate properly with the arrow keys

Same here - typed "1234", pasted "一一一", typed 5678. Press Home, RightArrow goes up to 6 and acts weird on each of "一" characters.

Screenshot_20230921_121920

ailin-nemui commented 12 months ago

that's not good at all :/

ailin-nemui commented 12 months ago

sounds like this might be https://github.com/Perl/perl5/issues/21366

hemingdao commented 12 months ago

Shouldn't it be: LC_COLLATE=C

ailin-nemui commented 11 months ago

@hrw I have two suggestions, maybe you can try if they fix the issue for you

1) patch perl: https://github.com/Perl/perl5/issues/21366#issuecomment-1732560904 2) add a "workaround" to irssi (very much a hack): https://github.com/irssi/irssi/pull/1498

hrw commented 11 months ago

I use https://github.com/irssi/scripts.irssi.org/issues/857#issuecomment-1729204086 as a workaround.

Length() returns proper value. Maybe not proper fix but does the job for now.

ailin-nemui commented 11 months ago

ok but please keep in mind that this bug may also cause other rendering glitches e.g. with emojis or Chinese characters, entirely unrelated to trackbar script

hrw commented 11 months ago

Reported bug in Fedora bugtracker: https://bugzilla.redhat.com/show_bug.cgi?id=2240458

cbean commented 1 month ago

Replacing c_length with length also makes it work.

This did the trick for me ...

ailin-nemui commented 1 month ago

Replacing c_length with length also makes it work.

This did the trick for me ...

please be aware that this is not the proper solution and will not fix the underlying issue