[PATCH 0/6] highlighting cleanups + help update

[PATCH 0/6] highlighting cleanups + help update

[Eric Wong]

Experimenting with using Markdown-style "```$LANG" blocks
support for our user documentation.  It makes the CSS
example easier-to-follow when the CSS source is in front
of the user.

Markdown-style blocks definitely won't be enabled by default for
emails.  I don't want to encourage people to use Markdown in
emails (as that inevitably ends with "which flavor?") and the
same mess with privacy/compatibility problems have with HTML
mail.

But preserving the WYSIWYG nature of plain-text while allowing a
tiny subset of Markdown (we already respect it for
linkification) might be useful for mailing lists which forward
messages from Markdown-supported forums/trackers (e.g. Redmine
and ruby-core mailing list).

Eric Wong (6):
  viewvcs: cleanup utf8 handling
  hlmod: hoist out do_hl_lang sub
  hlmod: make into a singleton
  hlmod: do_hl* performs src_escape immediately
  hlmod: support "```$LANG" blocks in text
  wwwtext: inline sample CSS and use highlight

 contrib/css/216dark.css        | 30 ++++++++++--------
 lib/PublicInbox/HlMod.pm       | 70 ++++++++++++++++++++++++++++--------------
 lib/PublicInbox/UserContent.pm | 16 +++++-----
 lib/PublicInbox/ViewVCS.pm     | 14 ++-------
 lib/PublicInbox/WwwText.pm     | 35 ++++++++++-----------
 t/hl_mod.t                     | 34 ++++++++++++--------
 6 files changed, 114 insertions(+), 85 deletions(-)

-- 
EW

[PATCH 1/6] viewvcs: cleanup utf8 handling

[Eric Wong]

Favor in-place utf8::decode since it's a bit faster without
method dispatch overhead; and don't care about validity just
yet.

HlMod->do_hl itself should return "utf8" strings, since other
parts of our code can use it, so it's not the job of ViewVCS to
post-process HlMod output.
---
 lib/PublicInbox/HlMod.pm   | 7 ++++++-
 lib/PublicInbox/ViewVCS.pm | 6 ++----
 t/hl_mod.t                 | 1 +
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/HlMod.pm b/lib/PublicInbox/HlMod.pm
index 237ffac..decfd71 100644
--- a/lib/PublicInbox/HlMod.pm
+++ b/lib/PublicInbox/HlMod.pm
@@ -107,7 +107,12 @@ sub do_hl {
 		$g->setEncoding('utf-8');
 		$g;
 	};
-	\($gen->generateString($$str))
+
+	# we assume $$str is valid UTF-8, but the SWIG binding doesn't
+	# know that, so ensure it's marked as UTF-8 even if it isnt...
+	my $out = $gen->generateString($$str);
+	utf8::decode($out);
+	\$out;
 }
 
 # SWIG instances aren't reference-counted, but $self is;
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index d67b5eb..acdd822 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -16,7 +16,6 @@
 package PublicInbox::ViewVCS;
 use strict;
 use warnings;
-use Encode qw(find_encoding);
 use PublicInbox::SolverGit;
 use PublicInbox::WwwStream;
 use PublicInbox::Linkify;
@@ -33,7 +32,6 @@ END { $hl = undef };
 
 my %QP_MAP = ( A => 'oid_a', B => 'oid_b', a => 'path_a', b => 'path_b' );
 my $max_size = 1024 * 1024; # TODO: configurable
-my $enc_utf8 = find_encoding('UTF-8');
 my $BIN_DETECT = 8000; # same as git
 
 sub html_page ($$$) {
@@ -122,14 +120,14 @@ sub solve_result {
 		return html_page($ctx, 200, \$log);
 	}
 
-	$$blob = $enc_utf8->decode($$blob);
+	# TODO: detect + convert to ensure validity
+	utf8::decode($$blob);
 	my $nl = ($$blob =~ tr/\n/\n/);
 	my $pad = length($nl);
 
 	$l->linkify_1($$blob);
 	my $ok = $hl->do_hl($blob, $path) if $hl;
 	if ($ok) {
-		$$ok = $enc_utf8->decode($$ok);
 		src_escape($$ok);
 		$blob = $ok;
 	} else {
diff --git a/t/hl_mod.t b/t/hl_mod.t
index 80f8890..c402f1f 100644
--- a/t/hl_mod.t
+++ b/t/hl_mod.t
@@ -19,6 +19,7 @@ my $orig = $str;
 {
 	my $ref = $hls->do_hl(\$str, 'foo.perl');
 	is(ref($ref), 'SCALAR', 'got a scalar reference back');
+	ok(utf8::valid($$ref), 'resulting string is utf8::valid');
 	like($$ref, qr/I can see you!/, 'we can see ourselves in output');
 	like($$ref, qr/&&/, 'escaped');
 
-- 
EW

[PATCH 2/6] hlmod: hoist out do_hl_lang sub

[Eric Wong]

We'll want to use to support highlighting syntax used by
Markdown and possibly other markup languages (while retaining
the raw plain-text layout and formatting).
---
 lib/PublicInbox/HlMod.pm | 15 ++++++++++-----
 t/hl_mod.t               |  2 ++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/HlMod.pm b/lib/PublicInbox/HlMod.pm
index decfd71..284e4b1 100644
--- a/lib/PublicInbox/HlMod.pm
+++ b/lib/PublicInbox/HlMod.pm
@@ -83,19 +83,24 @@ sub _path2lang ($$) {
 sub do_hl {
 	my ($self, $str, $path) = @_;
 	my $lang = _path2lang($self, $path) if defined $path;
+	do_hl_lang($self, $str, $lang);
+}
+
+sub do_hl_lang {
+	my ($self, $str, $lang) = @_;
+
 	my $dir = $self->{-dir};
 	my $langpath;
+
 	if (defined $lang) {
 		$langpath = $dir->getLangPath("$lang.lang") or return;
-		$langpath = undef unless -f $langpath;
+		$lang = undef unless -f $langpath
 	}
-	unless (defined $langpath) {
+	unless (defined $lang) {
 		$lang = _shebang2lang($self, $str) or return;
 		$langpath = $dir->getLangPath("$lang.lang") or return;
-		$langpath = undef unless -f $langpath;
+		return unless -f $langpath
 	}
-	return unless defined $langpath;
-
 	my $gen = $self->{$langpath} ||= do {
 		my $g = highlight::CodeGenerator::getInstance($highlight::HTML);
 		$g->setFragmentCode(1); # generate html fragment
diff --git a/t/hl_mod.t b/t/hl_mod.t
index c402f1f..238f8ec 100644
--- a/t/hl_mod.t
+++ b/t/hl_mod.t
@@ -22,6 +22,8 @@ my $orig = $str;
 	ok(utf8::valid($$ref), 'resulting string is utf8::valid');
 	like($$ref, qr/I can see you!/, 'we can see ourselves in output');
 	like($$ref, qr/&&/, 'escaped');
+	my $lref = $hls->do_hl_lang(\$str, 'perl');
+	is($$ref, $$lref, 'do_hl_lang matches do_hl');
 
 	use PublicInbox::Spawn qw(which);
 	if (eval { require IPC::Run } && which('w3m')) {
-- 
EW

[PATCH 3/6] hlmod: make into a singleton

[Eric Wong]

It turns out there's no point in having multiple instances of
this or having to worry about destruction or destruction
ordering.

This will make it easier to reuse the one instance we have
across different modules.
---
 lib/PublicInbox/HlMod.pm   | 33 +++++++++++++--------------------
 lib/PublicInbox/ViewVCS.pm |  5 -----
 t/hl_mod.t                 | 15 ---------------
 3 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/lib/PublicInbox/HlMod.pm b/lib/PublicInbox/HlMod.pm
index 284e4b1..13f27d1 100644
--- a/lib/PublicInbox/HlMod.pm
+++ b/lib/PublicInbox/HlMod.pm
@@ -16,6 +16,7 @@ package PublicInbox::HlMod;
 use strict;
 use warnings;
 use highlight; # SWIG-generated stuff
+my $hl;
 
 sub _parse_filetypes ($) {
 	my $ft_conf = $_[0]->searchFile('filetypes.conf') or
@@ -52,16 +53,20 @@ sub _parse_filetypes ($) {
 	(\%ext2lang, \@shebang);
 }
 
+# We only need one instance, so we don't need to do
+# highlight::CodeGenerator::deleteInstance
 sub new {
 	my ($class) = @_;
-	my $dir = highlight::DataDir->new;
-	$dir->initSearchDirectories('');
-	my ($ext2lang, $shebang) = _parse_filetypes($dir);
-	bless {
-		-dir => $dir,
-		-ext2lang => $ext2lang,
-		-shebang => $shebang,
-	}, $class;
+	$hl ||= do {
+		my $dir = highlight::DataDir->new;
+		$dir->initSearchDirectories('');
+		my ($ext2lang, $shebang) = _parse_filetypes($dir);
+		bless {
+			-dir => $dir,
+			-ext2lang => $ext2lang,
+			-shebang => $shebang,
+		}, $class;
+	};
 }
 
 sub _shebang2lang ($$) {
@@ -120,16 +125,4 @@ sub do_hl_lang {
 	\$out;
 }
 
-# SWIG instances aren't reference-counted, but $self is;
-# so we need to delete all the CodeGenerator instances manually
-# at our own destruction
-sub DESTROY {
-	my ($self) = @_;
-	foreach my $gen (values %$self) {
-		if (ref($gen) eq 'highlight::CodeGenerator') {
-			highlight::CodeGenerator::deleteInstance($gen);
-		}
-	}
-}
-
 1;
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index acdd822..0fb6b64 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -25,11 +25,6 @@ my $hl = eval {
 	PublicInbox::HlMod->new;
 };
 
-# we need to trigger highlight::CodeGenerator::deleteInstance
-# in HlMod::DESTROY before the rest of Perl shuts down to avoid
-# a segfault at shutdown
-END { $hl = undef };
-
 my %QP_MAP = ( A => 'oid_a', B => 'oid_b', a => 'path_a', b => 'path_b' );
 my $max_size = 1024 * 1024; # TODO: configurable
 my $BIN_DETECT = 8000; # same as git
diff --git a/t/hl_mod.t b/t/hl_mod.t
index 238f8ec..f2eb5f9 100644
--- a/t/hl_mod.t
+++ b/t/hl_mod.t
@@ -40,19 +40,4 @@ my $orig = $str;
 	}
 }
 
-my $nr = $ENV{TEST_MEMLEAK};
-if ($nr && -r "/proc/$$/status") {
-	my $fh;
-	open $fh, '<', "/proc/$$/status";
-	diag "starting at memtest at ".join('', grep(/VmRSS:/, <$fh>));
-	PublicInbox::HlMod->new->do_hl(\$orig) for (1..$nr);
-	open $fh, '<', "/proc/$$/status";
-	diag "creating $nr instances: ".join('', grep(/VmRSS:/, <$fh>));
-	my $hls = PublicInbox::HlMod->new;
-	$hls->do_hl(\$orig) for (1..$nr);
-	$hls = undef;
-	open $fh, '<', "/proc/$$/status";
-	diag "reused instance $nr times: ".join('', grep(/VmRSS:/, <$fh>));
-}
-
 done_testing;
-- 
EW

[PATCH 4/6] hlmod: do_hl* performs src_escape immediately

[Eric Wong]

We want to be able to take advantage of this in other modules
---
 lib/PublicInbox/HlMod.pm   | 2 ++
 lib/PublicInbox/ViewVCS.pm | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/HlMod.pm b/lib/PublicInbox/HlMod.pm
index 13f27d1..014d82f 100644
--- a/lib/PublicInbox/HlMod.pm
+++ b/lib/PublicInbox/HlMod.pm
@@ -16,6 +16,7 @@ package PublicInbox::HlMod;
 use strict;
 use warnings;
 use highlight; # SWIG-generated stuff
+use PublicInbox::Hval qw(src_escape);
 my $hl;
 
 sub _parse_filetypes ($) {
@@ -122,6 +123,7 @@ sub do_hl_lang {
 	# know that, so ensure it's marked as UTF-8 even if it isnt...
 	my $out = $gen->generateString($$str);
 	utf8::decode($out);
+	src_escape($out);
 	\$out;
 }
 
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 0fb6b64..f6a7694 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -19,7 +19,7 @@ use warnings;
 use PublicInbox::SolverGit;
 use PublicInbox::WwwStream;
 use PublicInbox::Linkify;
-use PublicInbox::Hval qw(ascii_html to_filename src_escape);
+use PublicInbox::Hval qw(ascii_html to_filename);
 my $hl = eval {
 	require PublicInbox::HlMod;
 	PublicInbox::HlMod->new;
@@ -123,7 +123,6 @@ sub solve_result {
 	$l->linkify_1($$blob);
 	my $ok = $hl->do_hl($blob, $path) if $hl;
 	if ($ok) {
-		src_escape($$ok);
 		$blob = $ok;
 	} else {
 		$$blob = ascii_html($$blob);
-- 
EW

[PATCH 5/6] hlmod: support "```$LANG" blocks in text

[Eric Wong]

This is compatible with Markdown; but we still keep the WYSIWYG
nature of plain-text with this.  This is only intended for use
with our documentation.  Enabling any type of Markdown support
for emails can lead to incompatibilities or interopability
problems with alternative implementations.
---
 lib/PublicInbox/HlMod.pm | 21 ++++++++++++++++++++-
 t/hl_mod.t               | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/HlMod.pm b/lib/PublicInbox/HlMod.pm
index 014d82f..36e3110 100644
--- a/lib/PublicInbox/HlMod.pm
+++ b/lib/PublicInbox/HlMod.pm
@@ -16,7 +16,7 @@ package PublicInbox::HlMod;
 use strict;
 use warnings;
 use highlight; # SWIG-generated stuff
-use PublicInbox::Hval qw(src_escape);
+use PublicInbox::Hval qw(src_escape ascii_html);
 my $hl;
 
 sub _parse_filetypes ($) {
@@ -127,4 +127,23 @@ sub do_hl_lang {
 	\$out;
 }
 
+# Highlight text, but support Markdown "```$LANG" notation
+# while preserving WYSIWYG of plain-text documentation.
+# This is NOT to be enabled by default or encouraged for parsing
+# emails, since it is NOT stable and can lead to standards
+# proliferation of email.
+sub do_hl_text {
+	my ($self, $str) = @_;
+
+	$$str = join('', map {
+		if (/\A(``` ?)(\w+)\s*?\n(.+)(^```\s*)\z/sm) {
+			my ($pfx, $lang, $code, $post) = ($1, $2, $3, $4);
+			my $hl = do_hl_lang($self, \$code, $lang) || \$code;
+			$pfx . $lang . "\n" . $$hl . $post;
+		} else {
+			ascii_html($_);
+		}
+	} split(/(^``` ?\w+\s*?\n.+?^```\s*$)/sm, $$str));
+}
+
 1;
diff --git a/t/hl_mod.t b/t/hl_mod.t
index f2eb5f9..84a4b57 100644
--- a/t/hl_mod.t
+++ b/t/hl_mod.t
@@ -40,4 +40,24 @@ my $orig = $str;
 	}
 }
 
+if ('experimental, only for help text') {
+	my $tmp = <<'EOF';
+:>
+```perl
+my $foo = 1 & 2;
+```
+:<
+EOF
+	$hls->do_hl_text(\$tmp);
+	my @hl = split(/^/m, $tmp);
+	is($hl[0], ":&gt;\n", 'first line escaped');
+	is($hl[1], "```perl\n", '2nd line preserved');
+	like($hl[2], qr/<span\b/, 'code highlighted');
+	like($hl[2], qr/&amp;/, 'ampersand escaped');
+	is($hl[3], "```\n", '4th line preserved');
+	is($hl[4], ":&lt;\n", '5th line escaped');
+	is(scalar(@hl), 5, 'no extra line');
+
+}
+
 done_testing;
-- 
EW

[PATCH 6/6] wwwtext: inline sample CSS and use highlight

[Eric Wong]

For user documentation regarding CSS; showing users the sample
CSS with comments is probably more helpful than having
standalone documentation on CSS classes.
---
 contrib/css/216dark.css        | 30 +++++++++++++++++-------------
 lib/PublicInbox/UserContent.pm | 16 +++++++++-------
 lib/PublicInbox/WwwText.pm     | 35 +++++++++++++++++------------------
 3 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/contrib/css/216dark.css b/contrib/css/216dark.css
index 882fbc4..b18b057 100644
--- a/contrib/css/216dark.css
+++ b/contrib/css/216dark.css
@@ -13,22 +13,22 @@
 a { color:#69f; text-decoration:none }
 a:visited { color:#96f }
 
-/* quoted text gets a different color */
+/* quoted text in emails gets a different color */
 *.q { color:#09f }
 
 /*
- * these may be used with cgit, too
+ * these may be used with cgit <https://git.zx2c4.com/cgit/>, too.
  * (cgit uses <div>, public-inbox uses <span>)
  */
-*.add { color:#0ff }
-*.del { color:#f0f }
-*.head { color:#fff }
-*.hunk { color:#c93 }
+*.add { color:#0ff } /* diff post-image lines */
+*.del { color:#f0f } /* diff pre-image lines */
+*.head { color:#fff } /* diff header (metainformation) */
+*.hunk { color:#c93 } /* diff hunk-header */
 
 /*
- * highlight 3.x colors (tested 3.18)
- * this doesn't use most of the colors available (I find too many
- * colors overwhelming).  So the #ccc default is commented out.
+ * highlight 3.x colors (tested 3.18) for displaying blobs.
+ * This doesn't use most of the colors available, as I find too
+ * many colors overwhelming, so the default is commented out.
  */
 .hl.num { color:#f30 } /* number */
 .hl.esc { color:#f0f } /* escape character */
@@ -36,11 +36,15 @@ a:visited { color:#96f }
 .hl.ppc { color:#f0f } /* preprocessor */
 .hl.pps { color:#f30 } /* preprocessor string */
 .hl.slc { color:#09f } /* single-line comment */
-.hl.com { color:#09f }
-/* .hl.opt { color:#ccc } */
-/* .hl.ipl { color:#ccc } */
-/* .hl.lin { color:#ccc } */
+.hl.com { color:#09f } /* multi-line comment */
+/* .hl.opt { color:#ccc } */ /* operator */
+/* .hl.ipl { color:#ccc } */ /* interpolation */
+
+/* keyword groups kw[a-z] */
 .hl.kwa { color:#ff0 }
 .hl.kwb { color:#0f0 }
 .hl.kwc { color:#ff0 }
 /* .hl.kwd { color:#ccc } */
+
+/* line-number (unused by public-inbox) */
+/* .hl.lin { color:#ccc } */
diff --git a/lib/PublicInbox/UserContent.pm b/lib/PublicInbox/UserContent.pm
index df0429c..468e6cf 100644
--- a/lib/PublicInbox/UserContent.pm
+++ b/lib/PublicInbox/UserContent.pm
@@ -38,9 +38,9 @@ sub CSS () {
 	*.hunk { color:#c93 }
 
 	/*
-	 * highlight 3.x colors (tested 3.18)
-	 * this doesn't use most of the colors available (I find too many
-	 * colors overwhelming).  So the #ccc default is commented out.
+	 * highlight 3.x colors (tested 3.18) for displaying blobs.
+	 * This doesn't use most of the colors available (I find too many
+	 * colors overwhelming), so the #ccc default is commented out.
 	 */
 	.hl.num { color:#f30 } /* number */
 	.hl.esc { color:#f0f } /* escape character */
@@ -48,10 +48,12 @@ sub CSS () {
 	.hl.ppc { color:#f0f } /* preprocessor */
 	.hl.pps { color:#f30 } /* preprocessor string */
 	.hl.slc { color:#09f } /* single-line comment */
-	.hl.com { color:#09f }
-	/* .hl.opt { color:#ccc } */
-	/* .hl.ipl { color:#ccc } */
-	/* .hl.lin { color:#ccc } */
+	.hl.com { color:#09f } /* multi-line comment */
+	/* .hl.opt { color:#ccc } */ /* operator */
+	/* .hl.ipl { color:#ccc } */ /* interpolation */
+	/* .hl.lin { color:#ccc } */ /* line-number (unused by public-inbox) */
+
+	/* keyword groups kw[a-z] */
 	.hl.kwa { color:#ff0 }
 	.hl.kwb { color:#0f0 }
 	.hl.kwc { color:#ff0 }
diff --git a/lib/PublicInbox/WwwText.pm b/lib/PublicInbox/WwwText.pm
index d3413ad..adadc37 100644
--- a/lib/PublicInbox/WwwText.pm
+++ b/lib/PublicInbox/WwwText.pm
@@ -10,6 +10,10 @@ use PublicInbox::WwwStream;
 use PublicInbox::Hval qw(ascii_html);
 our $QP_URL = 'https://xapian.org/docs/queryparser.html';
 our $WIKI_URL = 'https://en.wikipedia.org/wiki';
+my $hl = eval {
+	require PublicInbox::HlMod;
+	PublicInbox::HlMod->new
+};
 
 # /$INBOX/_/text/$KEY/ # KEY may contain slashes
 # For now, "help" is the only supported $KEY
@@ -61,7 +65,13 @@ sub get_text {
 
 sub _do_linkify {
 	my $l = PublicInbox::Linkify->new;
-	$_[0] = $l->linkify_2(ascii_html($l->linkify_1($_[0])));
+	$l->linkify_1($_[0]);
+	if ($hl) {
+		$hl->do_hl_text(\($_[0]));
+	} else {
+		$_[0] = ascii_html($_[0]);
+	}
+	$_[0] = $l->linkify_2($_[0]);
 }
 
 sub _srch_prefix ($$) {
@@ -91,7 +101,8 @@ sub _srch_prefix ($$) {
 sub _colors_help ($$) {
 	my ($ctx, $txt) = @_;
 	my $ibx = $ctx->{-inbox};
-	my $base_url = $ibx->base_url($ctx->{env});
+	my $env = $ctx->{env};
+	my $base_url = $ibx->base_url($env);
 	$$txt .= "color customization for $base_url\n";
 	$$txt .= <<EOF;
 
@@ -104,23 +115,11 @@ to control the colors they see:
 
 	${base_url}userContent.css
 
-CSS classes
------------
-
-	   span.q - quoted text in email messages
-
-For diff highlighting, we try to match class names with those
-used by cgit: https://git.zx2c4.com/cgit/
-
-	 span.add - diff post-image lines
-
-	 span.del - diff pre-image lines
-
-	span.head - diff header (metainformation)
-
-	span.hunk - diff hunk-header
-
+CSS sample
+----------
+```css
 EOF
+	$$txt .= PublicInbox::UserContent::sample($ibx, $env) . "```\n";
 }
 
 sub _default_text ($$$) {
-- 
EW