[PATCH] gitweb: apply fallback encoding before highlight

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] gitweb: apply fallback encoding before highlight

Shin Kojima
Some multi-byte character encodings (such as Shift_JIS and GBK) have
characters whose final bytes is an ASCII '\' (0x5c), and they
will be displayed as funny-characters even if $fallback_encoding is
correct.  This is because `highlight` command always expects UTF-8
encoded strings from STDIN.

    $ echo 'my $v = "申";' | highlight --syntax perl | w3m -T text/html -dump
    my $v = "申";

    $ echo 'my $v = "申";' | iconv -f UTF-8 -t Shift_JIS | highlight \
        --syntax perl | iconv -f Shift_JIS -t UTF-8 | w3m -T text/html -dump

    iconv: (stdin):9:135: cannot convert
    my $v = "

This patch prepare git blob objects to be encoded into UTF-8 before
highlighting in the manner of `to_utf8` subroutine.
---
 gitweb/gitweb.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05d7910..2fddf75 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3935,6 +3935,9 @@ sub run_highlighter {
 
  close $fd;
  open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
+          quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
+            '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
+            '--', "-fe=$fallback_encoding")." | ".
           quote_command($highlight_bin).
           " --replace-tabs=8 --fragment --syntax $syntax |"
  or die_error(500, "Couldn't open file or run syntax highlighter");
--
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gitweb: apply fallback encoding before highlight

Junio C Hamano
Shin Kojima <[hidden email]> writes:

> Some multi-byte character encodings (such as Shift_JIS and GBK) have
> characters whose final bytes is an ASCII '\' (0x5c), and they
> will be displayed as funny-characters even if $fallback_encoding is
> correct.  This is because `highlight` command always expects UTF-8
> encoded strings from STDIN.
>
>     $ echo 'my $v = "申";' | highlight --syntax perl | w3m -T text/html -dump
>     my $v = "申";
>
>     $ echo 'my $v = "申";' | iconv -f UTF-8 -t Shift_JIS | highlight \
>         --syntax perl | iconv -f Shift_JIS -t UTF-8 | w3m -T text/html -dump
>
>     iconv: (stdin):9:135: cannot convert
>     my $v = "
>
> This patch prepare git blob objects to be encoded into UTF-8 before
> highlighting in the manner of `to_utf8` subroutine.
> ---

The single liner Perl invoked from the script felt a bit too dense
to my taste but other than that I have no complaints to what the
patched code does.

Jakub, does it look good to you, too?

Please sign-off your patch (see Documentation/SubmittingPatches).

Thanks.


>  gitweb/gitweb.perl | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 05d7910..2fddf75 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3935,6 +3935,9 @@ sub run_highlighter {
>  
>   close $fd;
>   open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> +          quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
> +            '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
> +            '--', "-fe=$fallback_encoding")." | ".
>            quote_command($highlight_bin).
>            " --replace-tabs=8 --fragment --syntax $syntax |"
>   or die_error(500, "Couldn't open file or run syntax highlighter");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gitweb: apply fallback encoding before highlight

Jakub Narębski
On Mon, May 2, 2016 at 7:49 PM, Junio C Hamano <[hidden email]> wrote:

> Shin Kojima <[hidden email]> writes:
>
>> This patch prepare git blob objects to be encoded into UTF-8 before
>> highlighting in the manner of `to_utf8` subroutine.
>> ---
>
> The single liner Perl invoked from the script felt a bit too dense
> to my taste but other than that I have no complaints to what the
> patched code does.
>
> Jakub, does it look good to you, too?

Yes, it looks all right to me. $^X is current Perl. -CO means that
the output is utf8 (for `highlight` command), -p means read all lines
and print them (it could be replaced by "print" command in one-liner),
-s is here to pass $fallback_encoding as $fe (it could be replaced,
but it would require some fiddling with quoting $s), -e '...' means
execute one line.

> Please sign-off your patch (see Documentation/SubmittingPatches).
>
> Thanks.
>
>
>>  gitweb/gitweb.perl | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 05d7910..2fddf75 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -3935,6 +3935,9 @@ sub run_highlighter {
>>
>>       close $fd;
>>       open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
>> +               quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
>> +                 '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
>> +                 '--', "-fe=$fallback_encoding")." | ".
>>                 quote_command($highlight_bin).
>>                 " --replace-tabs=8 --fragment --syntax $syntax |"
>>               or die_error(500, "Couldn't open file or run syntax highlighter");



--
Jakub Narebski
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] gitweb: apply fallback encoding before highlight

Shin Kojima
In reply to this post by Junio C Hamano
Some multi-byte character encodings (such as Shift_JIS and GBK) have
characters whose final bytes is an ASCII '\' (0x5c), and they
will be displayed as funny-characters even if $fallback_encoding is
correct.  This is because `highlight` command always expects UTF-8
encoded strings from STDIN.

    $ echo 'my $v = "申";' | highlight --syntax perl | w3m -T text/html -dump
    my $v = "申";

    $ echo 'my $v = "申";' | iconv -f UTF-8 -t Shift_JIS | highlight \
        --syntax perl | iconv -f Shift_JIS -t UTF-8 | w3m -T text/html -dump

    iconv: (stdin):9:135: cannot convert
    my $v = "

This patch prepare git blob objects to be encoded into UTF-8 before
highlighting in the manner of `to_utf8` subroutine.

Signed-off-by: Shin Kojima <[hidden email]>
---

Changes for v2:
    - Add Signed-off-by

Thanks,
Shin Kojima

 gitweb/gitweb.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05d7910..2fddf75 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3935,6 +3935,9 @@ sub run_highlighter {
 
  close $fd;
  open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
+          quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
+            '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
+            '--', "-fe=$fallback_encoding")." | ".
           quote_command($highlight_bin).
           " --replace-tabs=8 --fragment --syntax $syntax |"
  or die_error(500, "Couldn't open file or run syntax highlighter");
--
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

Junio C Hamano
Shin Kojima <[hidden email]> writes:

> Some multi-byte character encodings (such as Shift_JIS and GBK) have
> characters whose final bytes is an ASCII '\' (0x5c), and they
> will be displayed as funny-characters even if $fallback_encoding is
> correct.

Just out of curiosity, do people still use Shift_JIS aka MS-Kanji?
It feels so last-decade, if not last-century ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

Shin Kojima
On Tue, May 03, 2016 at 11:33:42AM -0700, Junio C Hamano wrote:
> Shin Kojima <[hidden email]> writes:
>
> > Some multi-byte character encodings (such as Shift_JIS and GBK) have
> > characters whose final bytes is an ASCII '\' (0x5c), and they
> > will be displayed as funny-characters even if $fallback_encoding is
> > correct.
>
> Just out of curiosity, do people still use Shift_JIS aka MS-Kanji?
> It feels so last-decade, if not last-century ;-)

Yes, they do. There are still tons of code from 90's lying around.

For migrating our codebase from cp932 (Windows31J/MS-Kanji), I keep
failing to persuade my boss saying it has no incentives to do so.

I can say this patch, to consider $fallback_encoding while
highlighting, is fairly rational.  But I also feel this is too much
just for specific outdated character encodings, it is completely
useless for the most part of gitweb users in the world.

I would rather prefer to generate feedback from you all to convince
our management if this patch is not acceptable.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

Junio C Hamano
Shin Kojima <[hidden email]> writes:

> I can say this patch, to consider $fallback_encoding while
> highlighting, is fairly rational.  But I also feel this is too much
> just for specific outdated character encodings, it is completely
> useless for the most part of gitweb users in the world.

Oh, don't get me wrong. I do think what the patch does is very
sensible and have no intention of rejecting it.

Unless somebody finds a new bug in it, but in that case, we won't be
rejecting it but would be improving on it.

As I said, the question was "Just out of curiosity", since it's been
so long since I was in any part of software work done in Japan.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

Shin Kojima
> Oh, don't get me wrong. I do think what the patch does is very
> sensible and have no intention of rejecting it.

I'm sorry for making you worry, my poor English had caused some
misunderstanding.  I raised this Shift_JIS related problem (a.k.a
"ダメ文字" in Japanese) might attract your interest knowingly.

I would like to hear frank opinions from engineers who have high
abilities like you. ;)

> As I said, the question was "Just out of curiosity", since it's been
> so long since I was in any part of software work done in Japan.

Having said that, many people in Japan are still suffering from these
character encoding barriers.  This was a huge shock for me, since
I was studying information science in China as a Japanese foreign
student.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html