[PATCH] ox-html.el: Fix display of language indicators for source blocks

[PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

CSS rules like

pre.src-awk:before { content: 'Awk'; }

in `org-html-style-default' don't work in the current Org version,
because the class attribute has been moved from the `pre' element to a
`code' child of that element.  As a result, the name of the language
isn't dipslayed when hovering over a source block in the HTML file.  I
am currently doing

(setq org-html-keep-old-src t)

but I think it's a kludge.

A better way out would be to move back the `class' attribute from the
`code' element to its `pre' parent in `org-src-html-block', or to change
the CSS selectors in `org-html-style-default'.  This patch does the
former.  (As for the latter, AFAIK there is no CSS selector which picks
precisely those `pre' elements that have `code' children with attributes
`src' and `src-LANG'.)

Cheers,
Raghu.

----------------------------------------------------------------------

* lisp/ox-html.el (org-html-src-block): Move the `class' attribute of
  the `code' element to its parent `pre' element.

TINYCHANGE
---
 lisp/ox-html.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lisp/ox-html.el b/lisp/ox-html.el
index 3ad3ffe..857775b 100644
--- a/lisp/ox-html.el
+++ b/lisp/ox-html.el
@@ -3394,12 +3394,17 @@ contextual information."
 			      listing-number
 			      (org-trim (org-export-data caption info))))))
 		;; Contents.
-		(let ((open (if org-html-keep-old-src "<pre" "<pre><code"))
+		(let ((open (format "<pre class=\"src src-%s\"%s>%s"
+				    lang label
+				    (if org-html-keep-old-src
+					""
+				      (format "<code%s>"
+					      (if (and klipsify
+						       (string= lang "html"))
+						  " data-editor-type=\"html\""
+						"")))))
 		      (close (if org-html-keep-old-src "</pre>" "</code></pre>")))
-		  (format "%s class=\"src src-%s\"%s%s>%s%s"
-			  open lang label (if (and klipsify (string= lang "html"))
-					      " data-editor-type=\"html\"" "")
-			  code close)))))))
+		  (concat open code close)))))))
 
 ;;;; Statistics Cookie
 
-- 
2.7.4

----------------------------------------------------------------------
-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

At 2017-06-06T20:50:21+05:30, N. Raghavendra wrote:

> CSS rules like
>
> pre.src-awk:before { content: 'Awk'; }
>
> in `org-html-style-default' don't work in the current Org version,
> because the class attribute has been moved from the `pre' element to a
> `code' child of that element.

It appears that this change was introduced in commit
d5bbf365533ba45d72ebed8121c7612c860ea944.  It also introduced the
insertion of the attribute "data-editor-type" in the HTML export, when

  1. `org-html-klipsify-src' is true, and

  2. an HTML source block is being exported.

Now, "data-editor-type" is not a valid attribute with respect to the
XHTML1 Strict DTD (or any other W3C X/HTML DTD).  Therefore, when 1 and
2 are true, this attribute renders the exported HTML files invalid with
respect to the DTDs in `org-html-doctype-alist'.  I think this would
count as a drawback to those who care about HTML validation, especially
since ox-html indicates its concern for validation through
`org-html-validation-link', etc.

I don't know what is the best way to deal with this problem.  One
possibility is to put a warning in the documentation of
`org-html-klipsify-src' that setting it to true would lead to invalid
HTML exports when the Org file contains an HTML source block.

Cheers,
Raghu.

-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

At 2017-06-06T20:50:21+05:30, N. Raghavendra wrote:

> CSS rules like
>
> pre.src-awk:before { content: 'Awk'; }
>
> in `org-html-style-default' don't work in the current Org version,
> because the class attribute has been moved from the `pre' element to a
> `code' child of that element.  As a result, the name of the language
> isn't dipslayed when hovering over a source block in the HTML file.

Continuing the soliloquy, I was wondering if any of the maintainers have
seen this patch, and the bug that it addresses.  If they have, I'd
appreciate some indicator of what is being done about it.

Cheers,
Raghu.

-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[Nicolas Goaziou]

Hello,

"N. Raghavendra" <nyraghu27132@gmail.com> writes:

> Continuing the soliloquy, I was wondering if any of the maintainers have
> seen this patch,

I saw it.

> and the bug that it addresses.

That is above my pay-grade. I'm Cc'ing Matt Price, who introduced klipse
feature, for some insight.

> If they have, I'd appreciate some indicator of what is being done
> about it.

You posted the patch only two days ago. I hope you understand a "bump"
may be a bit early at this point.

Anyway, thank you for the patch. I hope this can be sorted out quickly.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

At 2017-06-08T21:24:31+02:00, Nicolas Goaziou wrote:

> I'm Cc'ing Matt Price, who introduced klipse feature, for some
> insight.

Thanks for the reply, and for the information.  I'll wait for Matt
Price's response.

> You posted the patch only two days ago. I hope you understand a "bump"
> may be a bit early at this point.

Certainly.  It was less a bump than an expression of concern that my
messages may have been lost in the general mileu of the mailing list.

Cheers,
Raghu.

-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[Matt Price]

Thanks for CCing me, Nicholas. I'll take a look at this in the next 24
hours -- have been away form my computer all day and have lots of stuff
to respond to!

matt


On 06/08/2017 03:24 PM, Nicolas Goaziou wrote:
> Hello,
>
> "N. Raghavendra" <nyraghu27132@gmail.com> writes:
>
>> Continuing the soliloquy, I was wondering if any of the maintainers have
>> seen this patch,
> I saw it.
>
>> and the bug that it addresses.
> That is above my pay-grade. I'm Cc'ing Matt Price, who introduced klipse
> feature, for some insight.
>
>> If they have, I'd appreciate some indicator of what is being done
>> about it.
> You posted the patch only two days ago. I hope you understand a "bump"
> may be a bit early at this point.
>
> Anyway, thank you for the patch. I hope this can be sorted out quickly.
>
> Regards,
>

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[Matt Price]

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

Hi,

You raise two important-seeming points about my patch:

1) a feature (display of language on hover) is removed by the patch
2) the patch also breaks XHTML validation

That patch was submitted to add support for live code examples in exported
HTML (using the klipse.js  library).  As it stands right now, both changes
(addition of `data-editor-type` attribute and moving the language name
class from `pre` to `code`) are necessary to make klipse work (cf.
https://github.com/viebel/klipse).  I'd rather not lose that functionality
completely. Can you think of a way to solve both the problems without
losing klipse funcitonality? If not, would you be satisfied with an
improved docstring and maybe turning klipse off by default?


On Thu, Jun 8, 2017 at 3:42 PM, N. Raghavendra <nyraghu27132@gmail.com>
wrote:

> At 2017-06-08T21:24:31+02:00, Nicolas Goaziou wrote:
>
> > I'm Cc'ing Matt Price, who introduced klipse feature, for some
> > insight.
>
> Thanks for the reply, and for the information.  I'll wait for Matt
> Price's response.
>
> > You posted the patch only two days ago. I hope you understand a "bump"
> > may be a bit early at this point.
>
> Certainly.  It was less a bump than an expression of concern that my
> messages may have been lost in the general mileu of the mailing list.
>
> Cheers,
> Raghu.
>
> --
> N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
> Harish-Chandra Research Institute, http://www.hri.res.in/
>
>

[-- Attachment #2: Type: text/html, Size: 2270 bytes --]

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

At 2017-06-12T12:45:13-04:00, Matt Price wrote:

> As it stands right now, both changes (addition of `data-editor-type`
> attribute and moving the language name class from `pre` to `code`) are
> necessary to make klipse work

I tried with some examples, and the editing facility of klipse seems to
work even if you put the class attributes in the pre element instead of
in its code child.  Let me check it again and provide an MWE, but it'll
take a few days, because I am busy at work this week.  If what I said is
indeed correct, I have some suggestions on how to deal with the problem.

Cheers,
Raghu.

-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

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

At 2017-06-14T23:02:12+05:30, N. Raghavendra wrote:

> At 2017-06-12T12:45:13-04:00, Matt Price wrote:
>
>> As it stands right now, both changes (addition of `data-editor-type`
>> attribute and moving the language name class from `pre` to `code`)
>> are necessary to make klipse work
>
> I tried with some examples, and the editing facility of klipse seems
> to work even if you put the class attributes in the pre element
> instead of in its code child.  Let me check it again and provide an
> MWE, but it'll take a few days, because I am busy at work this week.
> If what I said is indeed correct, I have some suggestions on how to
> deal with the problem.

I am sorry for the inexcusable delay.

1. After rereading your message, I felt that one way out is restore the
   treatment of source code blocks, when klipse is not used, to what it
   was before you made commit d5bbf36553.  This means that the blocks
   are exported as content of pre elements, when klipse is not used.

2. When klipse is used, your code kicks in, and puts the source code
   blocks of the klipse languages in code children, with appropriate
   attributes, of pre elements.  Blocks of non-klipse languages are
   still put directly into pre elements as before, with the appropriate
   class attribute.

3. It remains to decide when klipse is being used.  It is used when
   `org-html-klipsify-src' has a non-nil value.

4. Lastly, we are not changing the export of code blocks when klipse is
   not being used.  Therefore, `org-html-keep-old-src' is not needed.

Attached is a patch that implements these changes.

I have also put a test file, and two exported HTML files, with and
without klipse, at http://www.retrotexts.net/tmp/ox-html-patch/
The Makefile there should be self-explanatory.  Please see if
klipse.html agrees with what you want, modulo syntax colouring (it was
produced in batch mode).  The file noklipse.html validates (it can be
checked at the W3C validator), and has language indicators.

I hope this takes care of our different requirements.

Cheers,
Raghu.

--
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-html.el-Partially-revert-d5bbf36553.patch --]
[-- Type: text/x-diff, Size: 2479 bytes --]

From 7f352785b7ac5d9858a57da83778d4cfa572d7a5 Mon Sep 17 00:00:00 2001
From: "N. Raghavendra" <raghu@hri.res.in>
Date: Tue, 19 Sep 2017 05:00:50 +0530
Subject: [PATCH] ox-html.el: Partially revert d5bbf36553

* lisp/ox-html.el (org-html-src-block): Unless klipse is used, export
source code blocks as content of `pre' elements, and not as content of
`code' children of `pre' elements.  Restores the previous way of
exporting source code blocks, and fixes the display of language
indicators while hovering over the blocks in the exported HTML file,
when klipse is not used.
(org-html-keep-old-src): Remove it.  Not needed because it is now the
default, unless klipse is used.
---
 lisp/ox-html.el | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/lisp/ox-html.el b/lisp/ox-html.el
index ebb233b..be36d66 100644
--- a/lisp/ox-html.el
+++ b/lisp/ox-html.el
@@ -174,7 +174,6 @@
     (:html-klipsify-src nil nil org-html-klipsify-src)
     (:html-klipse-css nil nil org-html-klipse-css)
     (:html-klipse-js nil nil org-html-klipse-js)
-    (:html-klipse-keep-old-src nil nil org-html-keep-old-src)
     (:html-klipse-selection-script nil nil org-html-klipse-selection-script)
     (:infojs-opt "INFOJS_OPT" nil nil)
     ;; Redefine regular options.
@@ -1572,12 +1571,6 @@ https://developer.mozilla.org/en-US/docs/Mozilla/Mobile/Viewport_meta_tag"
   :package-version '(Org . "9.1")
   :type 'string)
 
-(defcustom org-html-keep-old-src nil
-  "When non-nil, use <pre class=\"\"> instead of <pre><code class=\"\">."
-  :group 'org-export-html
-  :package-version '(Org . "9.1")
-  :type 'boolean)
-
 
 ;;;; Todos
 
@@ -3384,12 +3377,16 @@ contextual information."
 			      listing-number
 			      (org-trim (org-export-data caption info))))))
 		;; Contents.
-		(let ((open (if org-html-keep-old-src "<pre" "<pre><code"))
-		      (close (if org-html-keep-old-src "</pre>" "</code></pre>")))
-		  (format "%s class=\"src src-%s\"%s%s>%s%s"
-			  open lang label (if (and klipsify (string= lang "html"))
-					      " data-editor-type=\"html\"" "")
-			  code close)))))))
+		(if klipsify
+		    (format "<pre><code class=\"src src-%s\"%s%s>%s</code></pre>"
+			    lang
+			    label
+			    (if (and klipsify (string= lang "html"))
+				" data-editor-type=\"html\""
+			      "")
+			    code)
+		  (format "<pre class=\"src src-%s\"%s>%s</pre>"
+                          lang label code)))))))
 
 ;;;; Statistics Cookie
 
-- 
2.7.4

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[N. Raghavendra]

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

At 2017-09-19T06:24:37+05:30, N. Raghavendra wrote:

> Attached is a patch that implements these changes.

Attached is a simplified patch with an unnecessary conditional removed.

Raghu.

-- 
N. Raghavendra <raghu@hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-html.el-Partially-revert-d5bbf36553.patch --]
[-- Type: text/x-diff, Size: 2464 bytes --]

From 392d26f3c112c51661e494f4cbb0c4b30e2a8302 Mon Sep 17 00:00:00 2001
From: "N. Raghavendra" <raghu@hri.res.in>
Date: Tue, 19 Sep 2017 22:37:02 +0530
Subject: [PATCH] ox-html.el: Partially revert d5bbf36553

* lisp/ox-html.el (org-html-src-block): Unless klipse is used, export
source code blocks as content of `pre' elements, and not as content of
`code' children of `pre' elements.  Restores the previous way of
exporting source code blocks, and fixes the display of language
indicators while hovering over the blocks in the exported HTML file,
when klipse is not used.
(org-html-keep-old-src): Remove it.  Not needed because it is now the
default, unless klipse is used.
---
 lisp/ox-html.el | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/lisp/ox-html.el b/lisp/ox-html.el
index ebb233b..d91ca73 100644
--- a/lisp/ox-html.el
+++ b/lisp/ox-html.el
@@ -174,7 +174,6 @@
     (:html-klipsify-src nil nil org-html-klipsify-src)
     (:html-klipse-css nil nil org-html-klipse-css)
     (:html-klipse-js nil nil org-html-klipse-js)
-    (:html-klipse-keep-old-src nil nil org-html-keep-old-src)
     (:html-klipse-selection-script nil nil org-html-klipse-selection-script)
     (:infojs-opt "INFOJS_OPT" nil nil)
     ;; Redefine regular options.
@@ -1572,12 +1571,6 @@ https://developer.mozilla.org/en-US/docs/Mozilla/Mobile/Viewport_meta_tag"
   :package-version '(Org . "9.1")
   :type 'string)
 
-(defcustom org-html-keep-old-src nil
-  "When non-nil, use <pre class=\"\"> instead of <pre><code class=\"\">."
-  :group 'org-export-html
-  :package-version '(Org . "9.1")
-  :type 'boolean)
-
 
 ;;;; Todos
 
@@ -3384,12 +3377,16 @@ contextual information."
 			      listing-number
 			      (org-trim (org-export-data caption info))))))
 		;; Contents.
-		(let ((open (if org-html-keep-old-src "<pre" "<pre><code"))
-		      (close (if org-html-keep-old-src "</pre>" "</code></pre>")))
-		  (format "%s class=\"src src-%s\"%s%s>%s%s"
-			  open lang label (if (and klipsify (string= lang "html"))
-					      " data-editor-type=\"html\"" "")
-			  code close)))))))
+		(if klipsify
+		    (format "<pre><code class=\"src src-%s\"%s%s>%s</code></pre>"
+			    lang
+			    label
+			    (if (string= lang "html")
+				" data-editor-type=\"html\""
+			      "")
+			    code)
+		  (format "<pre class=\"src src-%s\"%s>%s</pre>"
+                          lang label code)))))))
 
 ;;;; Statistics Cookie
 
-- 
2.7.4

Re: [PATCH] ox-html.el: Fix display of language indicators for source blocks

[Nicolas Goaziou]

Hello,

"N. Raghavendra" <nyraghu27132@gmail.com> writes:

> Subject: [PATCH] ox-html.el: Partially revert d5bbf36553

Applied. Thank you.

Regards,

-- 
Nicolas Goaziou