Gnus development mailing list
 help / color / mirror / Atom feed
* [PATCH] shr: render table with style
@ 2010-12-06 17:30 Julien Danjou
  2010-12-06 17:39 ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-06 17:30 UTC (permalink / raw)
  To: ding; +Cc: Julien Danjou

Signed-off-by: Julien Danjou <julien@danjou.info>
---

Lars, is the following acceptable?
Tested with:

<body bgcolor="yellow">
Yellow background
<div style="background-color: green;">Green backgrounds</div>
<div style="color: red; background-color: white;">
  Red on white
  <table bgcolor="black"><tr><td>black background table with red text</td></tr></table>
  <div style="color: white;">
    white text on white background
    <font color="orange">Orange text on white background</font>
    <div>Same old, same old</div>
  </div>
  <div style="color: blue;">
    Blue text on white background
  </div>
</body>


You can test how it renders now and how it renders with that patch. :)

 lisp/ChangeLog |    5 +++++
 lisp/shr.el    |   42 ++++++++++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 8f28c7e..d9bd701 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2010-12-06  Julien Danjou  <julien@danjou.info>
+
+	* shr.el (shr-insert-table-ruler, shr-insert-table): Insert table
+	content with style.
+
 2010-12-06  Lars Magne Ingebrigtsen  <larsi@gnus.org>
 
 	* shr.el (shr-tag-font): Colorize the region.
diff --git a/lisp/shr.el b/lisp/shr.el
index a860616..117ded1 100644
--- a/lisp/shr.el
+++ b/lisp/shr.el
@@ -202,13 +202,9 @@ redirects somewhere else."
     (if (fboundp function)
 	(funcall function (cdr dom))
       (shr-generic (cdr dom)))
-    (let ((color (cdr (assq 'color shr-stylesheet)))
-	  (background (cdr (assq 'background-color
-				 shr-stylesheet))))
-      (when (and style
-		 shr-stylesheet
-		 (or color background))
-	(shr-colorize-region start (point) color background)))))
+    (when (and style
+               shr-stylesheet)
+      (shr-colorize-region-with-style start (point)))))
 
 (defun shr-generic (cont)
   (dolist (sub cont)
@@ -293,6 +289,17 @@ redirects somewhere else."
     (unless (string-match "[ \t\n]\\'" text)
       (delete-char -1)))))
 
+(defun shr-colorize-with-style (start end)
+  (shr-colorize-region start end
+                       (cdr (assq 'color shr-stylesheet))
+                       (cdr (assq 'background-color
+                                  shr-stylesheet))))
+
+(defun shr-insert-with-style (&rest text)
+  (let ((start (point)))
+    (apply 'insert text)
+    (shr-colorize-with-style start (point))))
+
 (defun shr-find-fill-point ()
   (when (> (move-to-column shr-width) shr-width)
     (backward-char 1))
@@ -892,6 +899,7 @@ ones, in case fg and bg are nil."
 	 (body (or (cdr (assq 'tbody cont)) cont))
 	 (footer (cdr (assq 'tfoot cont)))
          (bgcolor (cdr (assq :bgcolor cont)))
+         (shr-stylesheet (list (cons 'background-color bgcolor)))
 	 (nheader (if header (shr-max-columns header)))
 	 (nbody (if body (shr-max-columns body)))
 	 (nfooter (if footer (shr-max-columns footer))))
@@ -953,7 +961,8 @@ ones, in case fg and bg are nil."
 		    max)))
       (dotimes (i height)
 	(shr-indent)
-	(insert shr-table-vertical-line "\n"))
+	(shr-insert-with-style
+         (concat (char-to-string shr-table-vertical-line) "\n")))
       (dolist (column row)
 	(goto-char start)
 	(let ((lines (nth 2 column))
@@ -962,7 +971,8 @@ ones, in case fg and bg are nil."
 	  (dolist (line lines)
 	    (setq overlay-line (pop overlay-lines))
 	    (end-of-line)
-	    (insert line shr-table-vertical-line)
+	    (shr-insert-with-style
+             (concat line (char-to-string shr-table-vertical-line)))
 	    (dolist (overlay overlay-line)
 	      (let ((o (make-overlay (- (point) (nth 0 overlay) 1)
 				     (- (point) (nth 1 overlay) 1)))
@@ -974,8 +984,9 @@ ones, in case fg and bg are nil."
 	  ;; possibly.
 	  (dotimes (i (- height (length lines)))
 	    (end-of-line)
-	    (insert (make-string (string-width (car lines)) ? )
-		    shr-table-vertical-line)
+	    (shr-insert-with-style
+             (concat (make-string (string-width (car lines)) ? )
+                     (char-to-string shr-table-vertical-line)))
 	    (forward-line 1)))))
     (shr-insert-table-ruler widths)))
 
@@ -983,10 +994,13 @@ ones, in case fg and bg are nil."
   (when (and (bolp)
 	     (> shr-indentation 0))
     (shr-indent))
-  (insert shr-table-corner)
+  (shr-insert-with-style
+   (char-to-string shr-table-corner))
   (dotimes (i (length widths))
-    (insert (make-string (aref widths i) shr-table-horizontal-line)
-	    shr-table-corner))
+    (shr-insert-with-style
+     (concat
+      (make-string (aref widths i) shr-table-horizontal-line)
+      (char-to-string shr-table-corner))))
   (insert "\n"))
 
 (defun shr-table-widths (table suggested-widths)
-- 
1.7.2.3




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-06 17:30 [PATCH] shr: render table with style Julien Danjou
@ 2010-12-06 17:39 ` Lars Magne Ingebrigtsen
  2010-12-06 17:42   ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-06 17:39 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

>   <table bgcolor="black"><tr><td>black background table with red text</td></tr></table>

I didn't even know that table took a bgcolor parameter.  :-)  Do all
elements take bgcolor/fgcolor parameters?

> +         (shr-stylesheet (list (cons 'background-color bgcolor)))

That should be a (nconc ... shr-stylesheet), otherwise you're not
inheriting from the parents.

> +	    (shr-insert-with-style
> +             (concat line (char-to-string shr-table-vertical-line)))

I'm not really sure I understand the point here.  Surely tag-table
should just colourise all its contents?  Which it does now if you say
<table style="background: blue">, doesn't it?

So: Render the entire table, and then colourise it afterwards, just like
with all other tags...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-06 17:39 ` Lars Magne Ingebrigtsen
@ 2010-12-06 17:42   ` Lars Magne Ingebrigtsen
  2010-12-06 18:07     ` Lars Magne Ingebrigtsen
  2010-12-07  9:13     ` Julien Danjou
  0 siblings, 2 replies; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-06 17:42 UTC (permalink / raw)
  To: ding

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> I didn't even know that table took a bgcolor parameter.  :-)  Do all
> elements take bgcolor/fgcolor parameters?

I mean, if they do, then that should just be moved to `shr-descend', and
we can drop the special-casing in -tag-body, too.

But I must admit I can only recall seeing bgcolor in <body>.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-06 17:42   ` Lars Magne Ingebrigtsen
@ 2010-12-06 18:07     ` Lars Magne Ingebrigtsen
  2010-12-07  9:19       ` Julien Danjou
  2010-12-07  9:13     ` Julien Danjou
  1 sibling, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-06 18:07 UTC (permalink / raw)
  To: ding

The method I imagined for the colours is perhaps somewhat unclear.  :-)

For each element, we see whether we should colourise ourselves.  If we
decide to, then we look at the color/background-color settings inherited
from our parents, too, when computing what colours to actually render
ourselves at.

In return, when the parents render themselves they do exactly the same
(it's all so recursive), but make sure that none of the colours already
rendered are overridden, because then the children will be cranky.

So I don't really see that any special-casing should be necessary for
<table> elements at all.  But there may be something subtle here I'm not
seeing... 

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-06 17:42   ` Lars Magne Ingebrigtsen
  2010-12-06 18:07     ` Lars Magne Ingebrigtsen
@ 2010-12-07  9:13     ` Julien Danjou
  1 sibling, 0 replies; 20+ messages in thread
From: Julien Danjou @ 2010-12-07  9:13 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

On Mon, Dec 06 2010, Lars Magne Ingebrigtsen wrote:

> I mean, if they do, then that should just be moved to `shr-descend', and
> we can drop the special-casing in -tag-body, too.

No, only table and body have bgcolor AFAIK.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-06 18:07     ` Lars Magne Ingebrigtsen
@ 2010-12-07  9:19       ` Julien Danjou
  2010-12-07 10:50         ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07  9:19 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On Mon, Dec 06 2010, Lars Magne Ingebrigtsen wrote:

> So I don't really see that any special-casing should be necessary for
> <table> elements at all.  But there may be something subtle here I'm not
> seeing... 

I gave an non-working HTML example in my patch so you can test, and
debug yourself to judge my patch. :) you should try, I think it's rather
clear what's wrong with the border.

And I did not talk about "bordercolor" attribute yet. :)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07  9:19       ` Julien Danjou
@ 2010-12-07 10:50         ` Lars Magne Ingebrigtsen
  2010-12-07 11:16           ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 10:50 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> I gave an non-working HTML example in my patch so you can test, and
> debug yourself to judge my patch. :) you should try, I think it's rather
> clear what's wrong with the border.

If you replace bgcolor with style="background-color: black", it looks
like I would expect...  but perhaps I'm expecting the wrong thing?

> And I did not talk about "bordercolor" attribute yet. :)

Eek.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 10:50         ` Lars Magne Ingebrigtsen
@ 2010-12-07 11:16           ` Julien Danjou
  2010-12-07 11:31             ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 11:16 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

> If you replace bgcolor with style="background-color: black", it looks
> like I would expect...  but perhaps I'm expecting the wrong thing?

Yes it does. The problem is that bgcolor attribute reading is not done
at the same level than style parsing. So, unless you want to special
case bgcolor for <table> in shr-descend, which would be ugly, I do not
see how to deal with that.

Well, I do have an idea: I think the colorization is not done at the
right level. Maybe that should be the responsibility of shr-insert to
colorize what it inserted, based on the current value of shr-stylesheet.

That would solve the <table bgcolor=> problem, at least partially.
Because the table code use `insert' rather than `shr-insert' for things
like borders, they would not be colorized. This is why in my patches
I've introduced another shr-insert-with-style function, and because
shr-insert do some weird treatment on text.

I let you think about that. :)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 11:16           ` Julien Danjou
@ 2010-12-07 11:31             ` Lars Magne Ingebrigtsen
  2010-12-07 11:38               ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 11:31 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> Yes it does. The problem is that bgcolor attribute reading is not done
> at the same level than style parsing. So, unless you want to special
> case bgcolor for <table> in shr-descend, which would be ugly, I do not
> see how to deal with that.

No, that's fine.  There are (apparently) only three tags with pre-CSS
colour specs -- <body>, <font> and <table>, and special-casing those
three makes perfect sense.  It's only a couple lines extra in each case.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 11:31             ` Lars Magne Ingebrigtsen
@ 2010-12-07 11:38               ` Julien Danjou
  2010-12-07 11:48                 ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 11:38 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

> No, that's fine.  There are (apparently) only three tags with pre-CSS
> colour specs -- <body>, <font> and <table>, and special-casing those
> three makes perfect sense.  It's only a couple lines extra in each case.

Hm, are you sure you're no special casing to by-pass a bad design
choice?

There's also <td> and <tr> which have bgcolor.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 11:38               ` Julien Danjou
@ 2010-12-07 11:48                 ` Lars Magne Ingebrigtsen
  2010-12-07 12:08                   ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 11:48 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> Hm, are you sure you're no special casing to by-pass a bad design
> choice?

Of course I'm not.  :-)

> There's also <td> and <tr> which have bgcolor.

A 90% solution that's fast enough to actually use is infinitely more
useful than a 100% solution that's so slow that you can't use it.  Table
rendering (if you have nested tables) is already so slow that it's
painful, and that has to be fixed.  Doing <tr bgcolor> just seems too
obscure with that goal in mind.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 11:48                 ` Lars Magne Ingebrigtsen
@ 2010-12-07 12:08                   ` Julien Danjou
  2010-12-07 12:14                     ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 12:08 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

> A 90% solution that's fast enough to actually use is infinitely more
> useful than a 100% solution that's so slow that you can't use it.  Table
> rendering (if you have nested tables) is already so slow that it's
> painful, and that has to be fixed.  Doing <tr bgcolor> just seems too
> obscure with that goal in mind.

Nothing has ever been slow in shr, to my eyes.

"Premature optimization is the root of all evil."

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 12:08                   ` Julien Danjou
@ 2010-12-07 12:14                     ` Lars Magne Ingebrigtsen
  2010-12-07 12:25                       ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 12:14 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> Nothing has ever been slow in shr, to my eyes.

In my work capacity, I get email from marketers (peeps wanna sell us
stuff!  how rude!), and they have tools that generate these HTML table
monstrosities.  One of them take like ten seconds to render.  (Before I
put the table rendering cache thing in it took several minutes.)

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 12:14                     ` Lars Magne Ingebrigtsen
@ 2010-12-07 12:25                       ` Julien Danjou
  2010-12-07 12:32                         ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 12:25 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

>> Nothing has ever been slow in shr, to my eyes.
>
> In my work capacity, I get email from marketers (peeps wanna sell us
> stuff!  how rude!), and they have tools that generate these HTML table
> monstrosities.  One of them take like ten seconds to render.  (Before I
> put the table rendering cache thing in it took several minutes.)

That just prove your cache implementation of the table rendering is a
good choice. Without any profiling of the code, there is no way to know
how to optimize the table rendering further then.

And this does not back up your previous statement:

> A 90% solution that's fast enough to actually use is infinitely more
> useful than a 100% solution that's so slow that you can't use it.  Table
> rendering (if you have nested tables) is already so slow that it's
> painful, and that has to be fixed.  Doing <tr bgcolor> just seems too
> obscure with that goal in mind.

Without profiling and tests, there is no data right now that can support
that doing things with no special casing and a differently though design
will be slower.

That is just your feelings. Which might be correct after all.

But you can't throw them at my face and call them argumentation. ;-)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 12:25                       ` Julien Danjou
@ 2010-12-07 12:32                         ` Lars Magne Ingebrigtsen
  2010-12-07 15:27                           ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 12:32 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> Without profiling and tests, there is no data right now that can support
> that doing things with no special casing and a differently though design
> will be slower.

Well, if there's more code being run in the common paths, it's going to
be slower.  :-)  The balance we're looking for is between "render as
prettily and readably as possible" vs "render fast enough for actual
use".  And that's a real trade-off. 

> That is just your feelings. Which might be correct after all.
>
> But you can't throw them at my face and call them argumentation. ;-)

Hey!  This is contradiction.  If you want argumentation, that's the next
room.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 12:32                         ` Lars Magne Ingebrigtsen
@ 2010-12-07 15:27                           ` Julien Danjou
  2010-12-07 16:34                             ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 15:27 UTC (permalink / raw)
  To: ding


[-- Attachment #1.1: Type: text/plain, Size: 827 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

> Well, if there's more code being run in the common paths, it's going to
> be slower.  :-)  The balance we're looking for is between "render as
> prettily and readably as possible" vs "render fast enough for actual
> use".  And that's a real trade-off. 

I agree, but I'm just suggesting to optimize things based on real data
rather than throwing "it will be slower that way".

It's obvious that it will be slower if we add more code (thanks). But if
the table code execution time (you say it's slow) and what I'm proposing
to add are having a 90/10 execution time ratio, arguing that adding more
stuff is bloating the execution time is not really helpful. :-D

Anyhow, rather than speaking forever I've implemented what I've in mind.

Here is a patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-shr-correct-table-color-rendering.patch --]
[-- Type: text/x-diff, Size: 15398 bytes --]

From ff42dfacd377a71782416bbc73e289b3bacd1e14 Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Tue, 7 Dec 2010 15:07:44 +0100
Subject: [PATCH] shr: correct table color rendering

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/shr.el |  249 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 140 insertions(+), 109 deletions(-)

diff --git a/lisp/shr.el b/lisp/shr.el
index 4f3af11..83165dc 100644
--- a/lisp/shr.el
+++ b/lisp/shr.el
@@ -201,12 +201,7 @@ redirects somewhere else."
 	(setq style nil)))
     (if (fboundp function)
 	(funcall function (cdr dom))
-      (shr-generic (cdr dom)))
-    ;; If style is set, then this node has set the color.
-    (when style
-      (shr-colorize-region start (point)
-			   (cdr (assq 'color shr-stylesheet))
-			   (cdr (assq 'background-color shr-stylesheet))))))
+      (shr-generic (cdr dom)))))
 
 (defun shr-generic (cont)
   (dolist (sub cont)
@@ -237,59 +232,73 @@ redirects somewhere else."
   (load "kinsoku" nil t))
 
 (defun shr-insert (text)
-  (when (and (eq shr-state 'image)
-	     (not (string-match "\\`[ \t\n]+\\'" text)))
-    (insert "\n")
-    (setq shr-state nil))
-  (cond
-   ((eq shr-folding-mode 'none)
-    (insert text))
-   (t
-    (when (and (string-match "\\`[ \t\n]" text)
-	       (not (bolp))
-	       (not (eq (char-after (1- (point))) ? )))
-      (insert " "))
-    (dolist (elem (split-string text))
-      (when (and (bolp)
-		 (> shr-indentation 0))
-	(shr-indent))
-      ;; The shr-start is a special variable that is used to pass
-      ;; upwards the first point in the buffer where the text really
-      ;; starts.
-      (unless shr-start
-	(setq shr-start (point)))
-      ;; No space is needed behind a wide character categorized as
-      ;; kinsoku-bol, between characters both categorized as nospace,
-      ;; or at the beginning of a line.
-      (let (prev)
-	(when (and (eq (preceding-char) ? )
-		   (or (= (line-beginning-position) (1- (point)))
-		       (and (shr-char-breakable-p
-			     (setq prev (char-after (- (point) 2))))
-			    (shr-char-kinsoku-bol-p prev))
-		       (and (shr-char-nospace-p prev)
-			    (shr-char-nospace-p (aref elem 0)))))
-	  (delete-char -1)))
-      (insert elem)
-      (let (found)
-	(while (and (> (current-column) shr-width)
-		    (progn
-		      (setq found (shr-find-fill-point))
-		      (not (eolp))))
-	  (when (eq (preceding-char) ? )
-	    (delete-char -1))
-	  (insert "\n")
-	  (unless found
-	    (put-text-property (1- (point)) (point) 'shr-break t)
-	    ;; No space is needed at the beginning of a line.
-	    (when (eq (following-char) ? )
-	      (delete-char 1)))
-	  (when (> shr-indentation 0)
-	    (shr-indent))
-	  (end-of-line))
-	(insert " ")))
-    (unless (string-match "[ \t\n]\\'" text)
-      (delete-char -1)))))
+  (let ((start (point)))
+    (when (and (eq shr-state 'image)
+               (not (string-match "\\`[ \t\n]+\\'" text)))
+      (insert "\n")
+      (setq shr-state nil))
+    (cond
+     ((eq shr-folding-mode 'none)
+      (insert text))
+     (t
+      (when (and (string-match "\\`[ \t\n]" text)
+                 (not (bolp))
+                 (not (eq (char-after (1- (point))) ? )))
+        (insert " "))
+      (dolist (elem (split-string text))
+        (when (and (bolp)
+                   (> shr-indentation 0))
+          (shr-indent))
+        ;; The shr-start is a special variable that is used to pass
+        ;; upwards the first point in the buffer where the text really
+        ;; starts.
+        (unless shr-start
+          (setq shr-start (point)))
+        ;; No space is needed behind a wide character categorized as
+        ;; kinsoku-bol, between characters both categorized as nospace,
+        ;; or at the beginning of a line.
+        (let (prev)
+          (when (and (eq (preceding-char) ? )
+                     (or (= (line-beginning-position) (1- (point)))
+                         (and (shr-char-breakable-p
+                               (setq prev (char-after (- (point) 2))))
+                              (shr-char-kinsoku-bol-p prev))
+                         (and (shr-char-nospace-p prev)
+                              (shr-char-nospace-p (aref elem 0)))))
+            (delete-char -1)))
+        (insert elem)
+        (let (found)
+          (while (and (> (current-column) shr-width)
+                      (progn
+                        (setq found (shr-find-fill-point))
+                        (not (eolp))))
+            (when (eq (preceding-char) ? )
+              (delete-char -1))
+            (insert "\n")
+            (unless found
+              (put-text-property (1- (point)) (point) 'shr-break t)
+              ;; No space is needed at the beginning of a line.
+              (when (eq (following-char) ? )
+                (delete-char 1)))
+            (when (> shr-indentation 0)
+              (shr-indent))
+            (end-of-line))
+          (insert " ")))
+      (unless (string-match "[ \t\n]\\'" text)
+        (delete-char -1))))
+    ;; If style is set, then this insert has set the color.
+    (shr-colorize-with-style start (point))))
+
+(defun shr-colorize-with-style (start end)
+  (shr-colorize-region start end
+                       (cdr (assq 'color shr-stylesheet))
+                       (cdr (assq 'background-color
+                                  shr-stylesheet))))
+
+(defun shr-insert-with-style (&rest text)
+  (let ((start (point)))
+    (apply 'insert text)
+    (shr-colorize-with-style start (point))))
 
 (defun shr-find-fill-point ()
   (when (> (move-to-column shr-width) shr-width)
@@ -379,7 +388,7 @@ redirects somewhere else."
 
 (defun shr-ensure-newline ()
   (unless (zerop (current-column))
-    (insert "\n")))
+    (shr-insert-with-style "\n")))
 
 (defun shr-ensure-paragraph ()
   (unless (bobp)
@@ -387,16 +396,16 @@ redirects somewhere else."
 	(unless (save-excursion
 		  (forward-line -1)
 		  (looking-at " *$"))
-	  (insert "\n"))
+	  (shr-insert-with-style "\n"))
       (if (save-excursion
 	    (beginning-of-line)
 	    (looking-at " *$"))
-	  (insert "\n")
-	(insert "\n\n")))))
+	  (shr-insert-with-style "\n")
+	(shr-insert-with-style "\n\n")))))
 
 (defun shr-indent ()
   (when (> shr-indentation 0)
-    (insert (make-string shr-indentation ? ))))
+    (shr-insert-with-style (make-string shr-indentation ? ))))
 
 (defun shr-fontize-cont (cont &rest types)
   (let (shr-start)
@@ -472,9 +481,9 @@ redirects somewhere else."
 	  ;; beginning of the line.
 	  (when (and (> (current-column) 0)
 		     (> (car (image-size image t)) 400))
-	    (insert "\n"))
+	    (shr-insert-with-style "\n"))
 	  (insert-image image (or alt "*"))))
-    (insert alt)))
+    (shr-insert-with-style alt)))
 
 (defun shr-rescale-image (data)
   (if (or (not (fboundp 'imagemagick-types))
@@ -633,10 +642,9 @@ ones, in case fg and bg are nil."
   (let* ((start (point))
 	 (fgcolor (cdr (assq :fgcolor cont)))
 	 (bgcolor (cdr (assq :bgcolor cont)))
-	 (shr-stylesheet (list (cons 'color fgcolor)
-			       (cons 'background-color bgcolor))))
-    (shr-generic cont)
-    (shr-colorize-region start (point) fgcolor bgcolor)))
+         (shr-stylesheet (shr-style-set 'color fgcolor))
+         (shr-stylesheet (shr-style-set 'background-color bgcolor)))
+    (shr-generic cont)))
 
 (defun shr-tag-style (cont)
   )
@@ -691,6 +699,15 @@ ones, in case fg and bg are nil."
 		    plist)))))
       plist)))
 
+(defun shr-style-set (element value)
+  "Set style ELEMENT to VALUE in `shr-stylesheet'.
+Return a modified copy of `shr-stylesheet'.
+
+If VALUE is nil, do nothing."
+  (if value
+      `((,element . ,value) ,@(remove (assq element shr-stylesheet) shr-stylesheet))
+    shr-stylesheet))
+
 (defun shr-tag-a (cont)
   (let ((url (cdr (assq :href cont)))
         (title (cdr (assq :title cont)))
@@ -726,7 +743,7 @@ ones, in case fg and bg are nil."
 		 (cdr (assq :src cont))))
     (when (and (> (current-column) 0)
 	       (not (eq shr-state 'image)))
-      (insert "\n"))
+      (shr-insert-with-style "\n"))
     (let ((alt (cdr (assq :alt cont)))
 	  (url (or url (cdr (assq :src cont)))))
       (let ((start (point-marker)))
@@ -743,7 +760,7 @@ ones, in case fg and bg are nil."
 		image)
 	    (if (or (not shr-content-function)
 		    (not (setq image (funcall shr-content-function url))))
-		(insert alt)
+		(shr-insert-with-style alt)
 	      (shr-put-image image alt))))
 	 ((or shr-inhibit-images
 	      (and shr-blocked-images
@@ -756,7 +773,7 @@ ones, in case fg and bg are nil."
 	 ((url-is-cached (shr-encode-url url))
 	  (shr-put-image (shr-get-image-data url) alt))
 	 (t
-	  (insert alt)
+	  (shr-insert-with-style alt)
 	  (ignore-errors
 	    (url-retrieve (shr-encode-url url) 'shr-image-fetched
 			  (list (current-buffer) start (point-marker))
@@ -805,12 +822,12 @@ ones, in case fg and bg are nil."
 		(setq shr-list-mode (1+ shr-list-mode)))
 	    "* "))
 	 (shr-indentation (+ shr-indentation (length bullet))))
-    (insert bullet)
+    (shr-insert-with-style bullet)
     (shr-generic cont)))
 
 (defun shr-tag-br (cont)
   (unless (bobp)
-    (insert "\n")
+    (shr-insert-with-style "\n")
     (shr-indent))
   (shr-generic cont))
 
@@ -834,7 +851,12 @@ ones, in case fg and bg are nil."
 
 (defun shr-tag-hr (cont)
   (shr-ensure-newline)
-  (insert (make-string shr-width shr-hr-line) "\n"))
+  (shr-insert-with-style (make-string shr-width shr-hr-line) "\n"))
+
+(defun shr-tag-td (cont)
+  (let* ((bgcolor (cdr (assq :bgcolor cont)))
+         (shr-stylesheet (shr-style-set 'background-color bgcolor)))
+    (shr-generic cont)))
 
 (defun shr-tag-title (cont)
   (shr-heading cont 'bold 'underline))
@@ -842,12 +864,8 @@ ones, in case fg and bg are nil."
 (defun shr-tag-font (cont)
   (let* ((start (point))
          (color (cdr (assq :color cont)))
-         (shr-stylesheet (nconc (list (cons 'color color))
-				shr-stylesheet)))
-    (shr-generic cont)
-    (when color
-      (shr-colorize-region start (point) color
-			   (cdr (assq 'background-color shr-stylesheet))))))
+         (shr-stylesheet (shr-style-set 'color color)))
+    (shr-generic cont)))
 
 ;;; Table rendering algorithm.
 
@@ -896,7 +914,10 @@ ones, in case fg and bg are nil."
 	 (body (or (cdr (assq 'tbody cont)) cont))
 	 (footer (cdr (assq 'tfoot cont)))
          (bgcolor (cdr (assq :bgcolor cont)))
-	 (nheader (if header (shr-max-columns header)))
+         (border-color (cdr (assq :bordercolor cont)))
+         (shr-stylesheet (shr-style-set 'border-color border-color))
+         (shr-stylesheet (shr-style-set 'background-color bgcolor))
+         (nheader (if header (shr-max-columns header)))
 	 (nbody (if body (shr-max-columns body)))
 	 (nfooter (if footer (shr-max-columns footer))))
     (shr-tag-table-1
@@ -947,6 +968,13 @@ ones, in case fg and bg are nil."
 	     (setq result (nconc (shr-find-elements (cdr elem) type) result)))))
     (nreverse result)))
 
+(defun shr-insert-table-border (&rest borders)
+  ;; HACK: we set border-color as being color so
+  ;; `shr-insert-with-style' colorize borders.
+  (let ((shr-stylesheet
+         (shr-style-set 'color (cdr (assq 'border-color shr-stylesheet)))))
+    (apply 'shr-insert-with-style borders)))
+
 (defun shr-insert-table (table widths)
   (shr-insert-table-ruler widths)
   (dolist (row table)
@@ -957,7 +985,7 @@ ones, in case fg and bg are nil."
 		    max)))
       (dotimes (i height)
 	(shr-indent)
-	(insert shr-table-vertical-line "\n"))
+	(shr-insert-table-border shr-table-vertical-line "\n"))
       (dolist (column row)
 	(goto-char start)
 	(let ((lines (nth 2 column))
@@ -966,7 +994,8 @@ ones, in case fg and bg are nil."
 	  (dolist (line lines)
 	    (setq overlay-line (pop overlay-lines))
 	    (end-of-line)
-	    (insert line shr-table-vertical-line)
+            (insert line)
+            (shr-insert-table-border shr-table-vertical-line)
 	    (dolist (overlay overlay-line)
 	      (let ((o (make-overlay (- (point) (nth 0 overlay) 1)
 				     (- (point) (nth 1 overlay) 1)))
@@ -978,20 +1007,20 @@ ones, in case fg and bg are nil."
 	  ;; possibly.
 	  (dotimes (i (- height (length lines)))
 	    (end-of-line)
-	    (insert (make-string (string-width (car lines)) ? )
+	    (shr-insert-table-border (make-string (string-width (car lines)) ? )
 		    shr-table-vertical-line)
 	    (forward-line 1)))))
     (shr-insert-table-ruler widths)))
 
 (defun shr-insert-table-ruler (widths)
-  (when (and (bolp)
-	     (> shr-indentation 0))
-    (shr-indent))
-  (insert shr-table-corner)
-  (dotimes (i (length widths))
-    (insert (make-string (aref widths i) shr-table-horizontal-line)
-	    shr-table-corner))
-  (insert "\n"))
+    (when (and (bolp)
+               (> shr-indentation 0))
+      (shr-indent))
+    (shr-insert-table-border shr-table-corner)
+    (dotimes (i (length widths))
+      (shr-insert-table-border (make-string (aref widths i) shr-table-horizontal-line)
+                             shr-table-corner))
+    (shr-insert-table-border "\n"))
 
 (defun shr-table-widths (table suggested-widths)
   (let* ((length (length suggested-widths))
@@ -1023,21 +1052,23 @@ ones, in case fg and bg are nil."
     widths))
 
 (defun shr-make-table (cont widths &optional fill)
-  (let ((trs nil))
+  (let (trs)
     (dolist (row cont)
       (when (eq (car row) 'tr)
-	(let ((tds nil)
-	      (columns (cdr row))
-	      (i 0)
-	      column)
-	  (while (< i (length widths))
-	    (setq column (pop columns))
-	    (when (or (memq (car column) '(td th))
-		      (null column))
-	      (push (shr-render-td (cdr column) (aref widths i) fill)
-		    tds)
-	      (setq i (1+ i))))
-	  (push (nreverse tds) trs))))
+	(let* ((tds nil)
+               (columns (cdr row))
+               (i 0)
+               (bgcolor (cdr (assq :bgcolor (cdr row))))
+               (shr-stylesheet (shr-style-set 'background-color bgcolor))
+               column)
+          (while (< i (length widths))
+            (setq column (pop columns))
+            (when (or (memq (car column) '(td th))
+                      (null column))
+              (push (shr-render-td (cdr column) (aref widths i) fill)
+                    tds)
+              (setq i (1+ i))))
+          (push (nreverse tds) trs))))
     (nreverse trs)))
 
 (defun shr-render-td (cont width fill)
@@ -1065,12 +1096,12 @@ ones, in case fg and bg are nil."
 	;; If the buffer is totally empty, then put a single blank
 	;; line here.
 	(if (zerop (buffer-size))
-	    (insert (make-string width ? ))
+	    (shr-insert-with-style (make-string width ? ))
 	  ;; Otherwise, fill the buffer.
 	  (while (not (eobp))
 	    (end-of-line)
 	    (when (> (- width (current-column)) 0)
-	      (insert (make-string (- width (current-column)) ? )))
+	      (shr-insert-with-style (make-string (- width (current-column)) ? )))
 	    (forward-line 1))))
       (if fill
 	  (list max
-- 
1.7.2.3


[-- Attachment #1.3: Type: text/plain, Size: 2084 bytes --]


I've used the following data example to test the rendering:

#+begin_src html
<body bgcolor="yellow">
Yellow background
<div style="background-color: green;">Green background with a green HR
<hr>
<ul><li>Does the point has green bg</li></ul>
</div>
<div style="color: red; background-color: white;">
  Red on white
  <table bgcolor="black" bordercolor="pink">
    <tr><td>black background table with red text</td><td>bar</td></tr>
    <tr><td bgcolor="blue"> blue bg td with red text</td><td>foobeat</td></tr>
    <tr bgcolor="orange"><td>orange bg tr with red text</td><td>baz</td></tr>
  </table>
  <div style="color: white;">
    white text on white background
    <font color="orange">Orange text on white background</font>
    <div>Same old, same old</div>
  </div>
  <div style="color: blue;">
    Blue text on white background
  </div>
</body>
#+end_src

I've counted 15 color rendering points in it that should be rendered
correctly. Based on that, I've benchmarked the execution time and the
number of rendering point correctly done on current master version and
with my patched version.

So based on my version of the ACID test :-) this is the result I got:

|---------+---------------------+-------------------|
| Version | Execution time x100 | Correct rendering |
|---------+---------------------+-------------------|
| master  |                0.96 | 7/15              |
| jd      |                1.64 | 15/15             |
|---------+---------------------+-------------------|

Benchmark runs with:
(benchmark-run-compiled 100
  (shr-insert-document
    (libxml-parse-html-region
       (point-min) (point-max))))

With my patch the code is 58 % slower but passes twice more tests (and
probably more, actually, I did not bother to make the numbers show how
lame the current code is :-P).

Of course, you're free to reject my patch… or enhance it. :) I've tried
a bit of elp on it, but I do not see anything obvious to cut.

Cheers,
-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 15:27                           ` Julien Danjou
@ 2010-12-07 16:34                             ` Lars Magne Ingebrigtsen
  2010-12-07 17:00                               ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-07 16:34 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

> I agree, but I'm just suggesting to optimize things based on real data
> rather than throwing "it will be slower that way".

Real data is great, but it takes a lot of typing to get to it.  Thank
you for doing so.  :-)

> It's obvious that it will be slower if we add more code (thanks). But if
> the table code execution time (you say it's slow) and what I'm proposing
> to add are having a 90/10 execution time ratio, arguing that adding more
> stuff is bloating the execution time is not really helpful. :-D

I find that in Gnus development, you really can't keep carping
"developers, develop...", er, I mean, "speed, speed, speed" enough.  One
adds one more obscure extra functionality because it's nice to have, and
then you find that everything is 50% slower than before.  It's happened
a lot...  and mostly with code I write myself.  :-)

> I've used the following data example to test the rendering:

Nice.

> I've counted 15 color rendering points in it that should be rendered
> correctly. Based on that, I've benchmarked the execution time and the
> number of rendering point correctly done on current master version and
> with my patched version.

I'm not sure I'm seeing all the rendering points, but the obvious ones
were the table/td/td {b,f}gcolor ones, right?

> Benchmark runs with:
> (benchmark-run-compiled 100
>   (shr-insert-document
>     (libxml-parse-html-region
>        (point-min) (point-max))))
>
> With my patch the code is 58 % slower but passes twice more tests (and
> probably more, actually, I did not bother to make the numbers show how
> lame the current code is :-P).

I just did the obvious thing and altered -tag-table and -render-td to do
colourisation like -tag-font, and, tada, with the following and your
test HTML.  Before:

(let ((doc
       (save-excursion
	 (libxml-parse-html-region
	  (goto-char (point-min)) (search-forward "\n\n")))))
  (benchmark-run-compiled 1000 (with-temp-buffer
				 (shr-insert-document doc))))
=> (3.0290269999999997 19 1.0066839999999786)

After adding the -tag-table and -render-td colourisations (that should
be a word):

=> (3.0097929999999997 19 1.0296220000000034)

So that's, er, faster.  Ok, `benchmark-run-compiled' may not be the most
precise benchmarking tool in the world, but...

I didn't try to do TR handling, though, so yours is more ACID-ey.  Or
bordercolor, but I don't really think the latter matters much...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 16:34                             ` Lars Magne Ingebrigtsen
@ 2010-12-07 17:00                               ` Julien Danjou
  2010-12-16 17:27                                 ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Danjou @ 2010-12-07 17:00 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 3015 bytes --]

On Tue, Dec 07 2010, Lars Magne Ingebrigtsen wrote:

> I find that in Gnus development, you really can't keep carping
> "developers, develop...", er, I mean, "speed, speed, speed" enough.  One
> adds one more obscure extra functionality because it's nice to have, and
> then you find that everything is 50% slower than before.  It's happened
> a lot...  and mostly with code I write myself.  :-)

I tend to agree, but I do not always suffer from it, so… :-)
OTOH, sometimes, we should just go on emacs-devel and yield that elisp
is too slow maybe. ;)

>> I've counted 15 color rendering points in it that should be rendered
>> correctly. Based on that, I've benchmarked the execution time and the
>> number of rendering point correctly done on current master version and
>> with my patched version.
>
> I'm not sure I'm seeing all the rendering points, but the obvious ones
> were the table/td/td {b,f}gcolor ones, right?

- Yellow background
- Green background
- Green HR
- Green bg LI (the star should be on green bg)
- red on white
- table must have pink borders
- table must have black background
- first left cell must be red on black
- second left cell must be red on blue
- third left and right cells should be red on orange
- white text on white background
- orange text on orange background
- white text on white background (same old)
- blue text on white background

And one I forgot. Or it's 14.

> I just did the obvious thing and altered -tag-table and -render-td to do
> colourisation like -tag-font, and, tada, with the following and your
> test HTML.  Before:
>
> (let ((doc
>        (save-excursion
> 	 (libxml-parse-html-region
> 	  (goto-char (point-min)) (search-forward "\n\n")))))
>   (benchmark-run-compiled 1000 (with-temp-buffer
> 				 (shr-insert-document doc))))
> => (3.0290269999999997 19 1.0066839999999786)
>
> After adding the -tag-table and -render-td colourisations (that should
> be a word):
>
> => (3.0097929999999997 19 1.0296220000000034)
>
> So that's, er, faster.  Ok, `benchmark-run-compiled' may not be the most
> precise benchmarking tool in the world, but...

0.02 on 3 in 1000 execution times… Well… :) 

> I didn't try to do TR handling, though, so yours is more ACID-ey.  Or
> bordercolor, but I don't really think the latter matters much...

Hehe. I understand, but where to draw the line is complicated then. What
we are doing is not that complicated after all, so I do not understand
why things are so slow, but well.

As I said, that's your call. You can still hack on your version to try
to pass my ACID test. But then I'll make the test harder so you'll end
up crazy with your special casing everywhere and I'll laugh at you.

Or just embrace the force and use my version which should work with
everything.

/me raises his hand in front of Lars and says:

   This is not the droid you're looking for.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-07 17:00                               ` Julien Danjou
@ 2010-12-16 17:27                                 ` Lars Magne Ingebrigtsen
  2010-12-17  9:23                                   ` Julien Danjou
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Magne Ingebrigtsen @ 2010-12-16 17:27 UTC (permalink / raw)
  To: ding

Julien Danjou <julien@danjou.info> writes:

>> => (3.0290269999999997 19 1.0066839999999786)
>>
>> After adding the -tag-table and -render-td colourisations (that should
>> be a word):
>>
>> => (3.0097929999999997 19 1.0296220000000034)
>>
>> So that's, er, faster.  Ok, `benchmark-run-compiled' may not be the most
>> precise benchmarking tool in the world, but...
>
> 0.02 on 3 in 1000 execution times… Well… :) 

I just meant that the benchmarking wasn't reliable, since it's obviously
not true that adding colours to the table stuff makes it faster.  :-)

> As I said, that's your call. You can still hack on your version to try
> to pass my ACID test. But then I'll make the test harder so you'll end
> up crazy with your special casing everywhere and I'll laugh at you.

Or I could just say "good enough" and leave it be.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shr: render table with style
  2010-12-16 17:27                                 ` Lars Magne Ingebrigtsen
@ 2010-12-17  9:23                                   ` Julien Danjou
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Danjou @ 2010-12-17  9:23 UTC (permalink / raw)
  To: ding

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]

On Thu, Dec 16 2010, Lars Magne Ingebrigtsen wrote:

> Or I could just say "good enough" and leave it be.  :-)

Your call then. I respect that.

But that's just not me. ;-)

-- 
Julien Danjou
❱ http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2010-12-17  9:23 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-06 17:30 [PATCH] shr: render table with style Julien Danjou
2010-12-06 17:39 ` Lars Magne Ingebrigtsen
2010-12-06 17:42   ` Lars Magne Ingebrigtsen
2010-12-06 18:07     ` Lars Magne Ingebrigtsen
2010-12-07  9:19       ` Julien Danjou
2010-12-07 10:50         ` Lars Magne Ingebrigtsen
2010-12-07 11:16           ` Julien Danjou
2010-12-07 11:31             ` Lars Magne Ingebrigtsen
2010-12-07 11:38               ` Julien Danjou
2010-12-07 11:48                 ` Lars Magne Ingebrigtsen
2010-12-07 12:08                   ` Julien Danjou
2010-12-07 12:14                     ` Lars Magne Ingebrigtsen
2010-12-07 12:25                       ` Julien Danjou
2010-12-07 12:32                         ` Lars Magne Ingebrigtsen
2010-12-07 15:27                           ` Julien Danjou
2010-12-07 16:34                             ` Lars Magne Ingebrigtsen
2010-12-07 17:00                               ` Julien Danjou
2010-12-16 17:27                                 ` Lars Magne Ingebrigtsen
2010-12-17  9:23                                   ` Julien Danjou
2010-12-07  9:13     ` Julien Danjou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).