Fossil Forum

Request for Enhancement: Markdown Fenced code blocks
Login

Request for Enhancement: Markdown Fenced code blocks

Request for Enhancement: Markdown Fenced code blocks

(1.1) By yds on 2018-10-27 18:12:54 edited from 1.0 [link] [source]

Without built in support for Fenced code blocks we have to resort to embedding the requisite HTML tags into the Markdown file:

<pre><code>
function test() {
  console.log("notice the blank line before this example?");
}
</code></pre>

With support for Fenced code blocks the Markdown file would be free of embedded HTML tags:

```ruby
require 'redcarpet'
markdown = Redcarpet.new("Hello World!")
puts markdown.to_html
```

The optional language hint would render the above example as a class to assist code highlighting:

<pre><code class="ruby">require 'redcarpet'
markdown = Redcarpet.new("Hello World!")
puts markdown.to_html
</code></pre>

Note, to avoid the extraneous blank like at the start of the codeblock, there must not be a newline after the <pre><code> in the generated HTML.

Here's a real world example where fossil's lack of support for Fenced code blocks requires embedding HTML tags for the desired output:

https://yds.necessitu.de/repo/authz_ipacl/artifact/c6a7394738c956c9?txt=1

Wrapping fossil's RAW artifact output with:

<pre><code>
...
</code></pre>

would also help JavaScript code highlighters such as highlight.js. Proposed patch here.

EDIT: Do'h! I must've missed this — perhaps cuz I didn't see it next to here. That makes this Request for Enhancement kinda moot since code blocks can be expressed cleanly without embedding HTML in the Markdown. Albeit without the functionality of fenced code block language hinting as described above. I still wouldn't mind getting commonmark-markdown merged into a release — perhaps as a build time option?

Same file as above now with Tab indented code blocks as per the docs:

https://yds.necessitu.de/repo/authz_ipacl/artifact/79756a00c44350a9?txt=1

(2) By anonymous on 2018-10-15 19:27:46 in reply to 1.0 [link] [source]

I would also find support for fenced code blocks to be very useful.

Thanks.

(3) By Richard Hipp (drh) on 2018-10-15 19:55:28 in reply to 2 [link] [source]

Patch suggestions are cheerfully accepted :-)

(9.4) By yds on 2018-10-27 16:10:15 edited from 9.3 in reply to 3 [source]

--- src/attach.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/attach.c	2018-09-22 13:40:11.000000000 -0400
@@ -610,16 +610,16 @@
   @ <blockquote>
   blob_zero(&attach);
   if( fShowContent ){
-    const char *z;
+    const char *z, *zClass;
+    char *ext;
     content_get(ridSrc, &attach);
     blob_to_utf8_no_bom(&attach, 0);
     z = blob_str(&attach);
+    zClass = (ext = strrchr(zName, '.')) ? mprintf(" class=\"%s\"", ext+1) : "";
     if( zLn ){
-      output_text_with_line_numbers(z, zLn);
+      output_text_with_line_numbers(z, zLn, zClass);
     }else{
-      @ <pre>
-      @ %h(z)
-      @ </pre>
+      @ <pre><code%s(zClass)>%h(z)</code></pre>
     }
   }else if( strncmp(zMime, "image/", 6)==0 ){
     int sz = db_int(0, "SELECT size FROM blob WHERE rid=%d", ridSrc);
--- src/diff.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/diff.c	2018-09-22 13:40:11.000000000 -0400
@@ -619,9 +619,7 @@
 static void sbsWriteColumn(Blob *pOut, Blob *pCol, int col){
   blob_appendf(pOut,
     "<td><div class=\"diff%scol\">\n"
-    "<pre>\n"
-    "%s"
-    "</pre>\n"
+    "<pre><code>%s</code></pre>\n"
     "</div></td>\n",
     (col % 3) ? (col == SBS_MKR ? "mkr" : "txt") : "ln",
     blob_str(pCol)
@@ -2393,6 +2391,8 @@
   const char *zRevision; /* Name of check-in from which to start annotation */
   const char *zCI;       /* The check-in containing zFilename */
   const char *zOrigin;   /* The origin of the analysis */
+  const char *zClass;    /* Language class to highlight */
+  char *ext;             /* Filename extension */
   int szHash;            /* Number of characters in %S display */
   char *zLink;
   Annotator ann;
@@ -2487,7 +2487,8 @@
     @ %z(href("%R/finfo?name=%h&ci=%!S", zFilename, zCI))%h(zFilename)</a>
     @ from check-in %z(href("%R/info/%!S",zCI))%S(zCI)</a>:</h2>
   }
-  @ <pre>
+  zClass = (ext = strrchr(zFilename, '.')) ? mprintf(" class=\"%s\"", ext+1) : "";
+  cgi_printf("<pre><code%s>", zClass);
   szHash = 10;
   for(i=0; i<ann.nOrig; i++){
     int iVers = ann.aOrig[i].iVers;
@@ -2527,7 +2528,7 @@
     @ %s(zPrefix) %h(z)
 
   }
-  @ </pre>
+  @ </code></pre>
   style_footer();
 }
 
--- src/info.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/info.c	2018-09-22 13:40:11.000000000 -0400
@@ -374,9 +374,7 @@
   }else{
     text_diff(&from, &to, &out, pRe,
            diffFlags | DIFF_LINENO | DIFF_HTML | DIFF_NOTTOOBIG);
-    @ <pre class="udiff">
-    @ %s(blob_str(&out))
-    @ </pre>
+    @ <pre class="udiff"><code class="diff">%s(blob_str(&out))</code></pre>
   }
   blob_reset(&from);
   blob_reset(&to);
@@ -1794,9 +1792,9 @@
         g.zTop, blob_str(&downloadName), zUuid);
   @ <hr />
   content_get(rid, &content);
-  @ <blockquote><pre>
+  cgi_append_content("<blockquote><pre><code>",-1);
   hexdump(&content);
-  @ </pre></blockquote>
+  @ </code></pre></blockquote>
   style_footer();
 }
 
@@ -1867,7 +1865,8 @@
 */
 void output_text_with_line_numbers(
   const char *z,
-  const char *zLn
+  const char *zLn,
+  const char *zClass
 ){
   int iStart, iEnd;    /* Start and end of region to highlight */
   int n = 0;           /* Current line number */
@@ -1903,7 +1902,7 @@
     if( iTop>iStart - 2 ) iTop = iStart-2;
   }
   db_finalize(&q);
-  @ <pre>
+  cgi_printf("<pre><code%s>", zClass);
   while( z[0] ){
     n++;
     db_prepare(&q,
@@ -1932,7 +1931,7 @@
     if( z[0]=='\n' ) z++;
   }
   if( n<iEnd ) cgi_printf("</div>");
-  @ </pre>
+  @ </code></pre>
   if( db_int(0, "SELECT EXISTS(SELECT 1 FROM lnos)") ){
     style_load_one_js_file("scroll.js");
   }
@@ -2140,14 +2139,14 @@
       zMime = mimetype_from_content(&content);
       @ <blockquote>
       if( zMime==0 ){
-        const char *z;
+        const char *z, *zClass;
+        char *ext;
         z = blob_str(&content);
+        zClass = (ext = strrchr(blob_str(&downloadName), '.')) ? mprintf(" class=\"%s\"", ext+1) : "";
         if( zLn ){
-          output_text_with_line_numbers(z, zLn);
+          output_text_with_line_numbers(z, zLn, zClass);
         }else{
-          @ <pre>
-          @ %h(z)
-          @ </pre>
+          @ <pre><code%s(zClass)>%h(z)</code></pre>
         }
       }else if( strncmp(zMime, "image/", 6)==0 ){
         @ <i>(file is %d(blob_size(&content)) bytes of image data)</i><br />
--- src/setup.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/setup.c	2018-09-22 13:40:11.000000000 -0400
@@ -1026,8 +1026,7 @@
   @ <li>Properties: "adunit", "adunit-right", "adunit-omit-if-admin", and
   @     "adunit-omit-if-user".
   @ <li>Suggested <a href="setup_skinedit?w=0">CSS</a> changes:
-  @ <blockquote><pre>
-  @ div.adunit_banner {
+  @ <blockquote><pre><code class="css">div.adunit_banner {
   @   margin: auto;
   @   width: 100%%;
   @ }
@@ -1036,19 +1035,16 @@
   @ }
   @ div.adunit_right_container {
   @   min-height: <i>height-of-right-column-ad-unit</i>;
-  @ }
-  @ </pre></blockquote>
+  @ }</code></pre></blockquote>
   @ <li>For a place-holder Ad-Unit for testing, Copy/Paste the following
   @ with appropriate adjustments to "width:" and "height:".
-  @ <blockquote><pre>
-  @ &lt;div style='
+  @ <blockquote><pre><code class="html">&lt;div style='
   @   margin: 0 auto;
   @   width: 600px;
   @   height: 90px;
   @   border: 1px solid #f11;
   @   background-color: #fcc;
-  @ '&gt;Demo Ad&lt;/div&gt;
-  @ </pre></blockquote>
+  @ '&gt;Demo Ad&lt;/div&gt;</code></pre></blockquote>
   @ </li>
   style_footer();
   db_end_transaction(0);
--- src/skins.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/skins.c	2018-09-22 13:40:11.000000000 -0400
@@ -805,9 +805,7 @@
     }else{
       text_diff(&from, &to, &out, 0,
              diffFlags | DIFF_LINENO | DIFF_HTML | DIFF_NOTTOOBIG);
-      @ <pre class="udiff">
-      @ %s(blob_str(&out))
-      @ </pre>
+      @ <pre class="udiff"><code class="diff">%s(blob_str(&out))</code></pre>
     }
     blob_reset(&from);
     blob_reset(&to);
--- src/wiki.c.orig	2018-09-22 13:40:11.000000000 -0400
+++ src/wiki.c	2018-09-22 13:40:11.000000000 -0400
@@ -927,9 +927,7 @@
   blob_zero(&d);
   diffFlags = construct_diff_flags(1);
   text_diff(&w2, &w1, &d, 0, diffFlags | DIFF_HTML | DIFF_LINENO);
-  @ <pre class="udiff">
-  @ %s(blob_str(&d))
-  @ <pre>
+  @ <pre class="udiff"><code class="diff">%s(blob_str(&d))</code></pre>
   manifest_destroy(pW1);
   manifest_destroy(pW2);
   style_footer();

(24.1) By yds on 2018-10-26 22:29:46 edited from 24.0 in reply to 9.2 [link] [source]

<html>
<head>
<base href="$baseurl/$current_page" />
<meta http-equiv="Content-Security-Policy" content="default-src 'self' data: 'unsafe-inline'" />
<title>$<project_name>: $<title></title>
<link rel="alternate" type="application/rss+xml" title="RSS Feed" href="$home/timeline.rss" />
<link rel="stylesheet" href="$stylesheet_url" type="text/css" media="screen" />
<th1>
if {"/$current_page" ne "/annotate" && "/$current_page" ne "/blame" && [getParameter ln] eq {}} {
  html "<link rel='stylesheet' href='/hljs/styles/github.css' type='text/css' media='screen' />\n"
  html "<script src='/hljs/highlight.pack.js'></script>\n"
  html "<script>hljs.initHighlightingOnLoad();</script>\n"
}
</th1></head>
<body>

(10.3) By yds on 2018-10-27 17:56:09 edited from 10.2 in reply to 3 [link] [source]

This patch addresses the last (but not least) of my Request for Enhancement:

Wrapping fossil's RAW artifact output with:

        <pre><code>
        ...
        </code></pre>

would also help JavaScript code highlighters such as highlight.js.

Inspired by @lmartin92 Syntax Highlighting post.

A running patched instance can be seen here — press Ctrl+U to see how highlight.js is conditionally included in the header only where it does not break the view.

(11) By llmII (lmartin92) on 2018-10-25 01:12:06 in reply to 10.1 [link] [source]

This patch breaks when line numbering is enabled (see that lines 3 to 7, 56, others, the line number is highlighted, instead of just the text, could get horrible in other languages).

Still reviewing the patch. Might update this post with anything noticed.

(12) By yds on 2018-10-25 01:40:01 in reply to 11 [link] [source]

This patch breaks when line numbering is enabled

True. Perhaps the way to fix it is the way the diff module handles it — line numbers are in a separate column and not mixed into the code block.

Another place this same issue crops up is in the /annotate or /blame views.

(13) By llmII (lmartin92) on 2018-10-25 01:52:03 in reply to 12 [link] [source]

That could work. The way I handled it was to use highlightjs line numbers thing and modify mine to take over most of the work (line numbers, link per line, scroll to top of line selected by url, multiple line selection, multiple selections, etc). Wonder would this work with the current way a selected line would be scrolled to top of page (the column thing) in the artifact view.

The other part is making sure it works even without syntax highlighting enabled (sorry, didn't check that part).

(14) By yds on 2018-10-25 02:07:13 in reply to 13 [link] [source]

Yah, I did not patch any of the highlight.js code — I suppose using your js code would work just as well with my patch as it does with yours. I basically refactored what you did in your patch and applied it throughout the code base.

(15) By llmII (lmartin92) on 2018-10-25 02:16:46 in reply to 14 [link] [source]

Still trying to figure out how <span class="hljs-number">1</span> is generated. Looking at the patch I can't seem to see where it's done, and I didn't thing hljs did line numbers out of the box (at least, the version I'm using didn't, which was the latest version when I pulled it).

(16) By yds on 2018-10-25 02:22:19 in reply to 15 [link] [source]

Still trying to figure out how <span class="hljs-number">1</span> is generated.

Gotta be from highlight.js — cuz my patch does nothing more than add <code> to any apropos <pre> tags.

(17) By llmII (lmartin92) on 2018-10-25 02:33:30 in reply to 16 [link] [source]

I see what it is, "hljs-number" is just some generic number, not specifically applicable only to line numbers, line numbers just happen to be numbers and thus get applied the number class. Which is incorrect.

The way I handle line numbers won't work with things like diffs (and even correctly highlighting them and implementing all the features fossil supports for them in both unified and side by side diff formats is daunting. Then again, I rarely look at diffs with color enabled because its less code and more seeing changes. Not sure if it makes sense for diffs to be highlighted but that's a matter of opinion. I'm unsure if anything else does syntax highlights when doing diffs.

The way my patch worked was it stripped away from Fossil the task of numbering lines in the case of a setting "syntax-hl" being enabled. Then the line numbering js script took over all line numbering tasks. In the case someone didn't want to have syntax highlighting, and in keeping with how things are currently defaulted (no syntax highlighting), the setting is set to false by default, and when false the old way of letting Fossil handle it is done.

Could you provide some links from your repo with examples of how this is highlighting all the things it does highlight (think you mentioned diffs, and some others, but I could quite easily have looked at code or pulled that out of a random thought). Might be that only one place needs the setting "syntax-hl" and the skip for when enabled as to Fossil handling line numbering, and if so... probably could do a simple change and be all set as to the remainder of things. Otherwise, I'll need to give it more thought tomorrow to come up with ideas and think through how to code it to make things play better (I think I've gotten too tired to find better words).

(18) By yds on 2018-10-25 02:46:09 in reply to 17 [link] [source]

I skipped over the "syntax-hl" functionality — my reasoning was that the patch outputs correct HTML5 regardless if a syntax highlighter is called. This way any syntax highlighter js can be used by patching the skin of one's choice. No need for the "syntax-hl" knob — just don't patch the skin with a syntax highlighter and nothing gets broken.

(19) By llmII (lmartin92) on 2018-10-25 03:04:32 in reply to 18 [link] [source]

Your patch does output correct HTML5, but doesn't do anything for handling line numbers, leaving such broken in the event that syntax highlighting is used. The reason for the "syntax-hl" knob is so Fossil knows to behave differently if a person is using syntax highlighters so as to allow them to handle line numbering in js in such a way that it doesn't have broken highlighting. The way to handle syntax highlighting with line numbers (from my point of view) was that if someone wants to use any other syntax highlighter they could, they just needed to customize it to deal with line numberings the way Fossil did when syntax highlighting wasn't enabled (scroll to top, selections, etc) as well as numbering the lines themselves. Doesn't matter which syntax highlighter is used, if Fossil is unaware of it, or if it's not an almost from scratch built purely for Fossil syntax highlighter, it will render line numbering wrong as far as syntax highlighting goes.

(20) By yds on 2018-10-25 03:45:49 in reply to 19 [link] [source]

Line numbers getting colored the same as the multi-line text block may not be pretty, but it could be seen as a "feature" from a certain angle, in a certain light. ;) That seems to be the only instance where this "breakage" occurs. I can live with that. Rather than having to patch js highlighters with line numbering know how — I believe the method Fossil already uses in the diff module is more versatile, cuz putting line numbers into a different column could also be used to put the code block into its own column in the /annotate and /blame views.

(21.1) By llmII (lmartin92) on 2018-10-25 14:05:27 edited from 21.0 in reply to 20 [link] [source]

Line numbers getting colored the same as the multi-line text block may not be pretty, but it could be seen as a "feature" from a certain angle, in a certain light.

If line numbers get colored (and this could look more and more abysmal in some languages where this might indicate an even grander syntax break) then something is broken. It isn't Fossil that is broken either. However, considering that most syntax highlighters are well established projects and suffer from wide useage, them changing to take into account how fossil works is extremely unlikely. It is far easier to work on Fossil, than to try to get a syntax highlighter project to accept something meant just to fix that it won't integrate with something that isn't going to necessarily matter to them. Everyone integrates syntax highlighters by making accomodations to them. Syntax highlighters make accomodations to very little. Maintaining a special Fossil only fork of some odd syntax highlighting project is a waste of effort IMO (may as well write one from scratch). The reason I view line numbering so critically is due to https://www.mail-archive.com/fossil-users@lists.fossil-scm.org/msg27742.html .

The reason for a knob is that there are some things where Fossil does need to know that syntax highlighting is taking place, despite any effort to make fossil just output code that works with most syntax highlighters. For example, unified diff won't work even with the column approach although things are wrapped in a <pre><code>. Side by side diff is showing a highlight for the url which is inside of a comment. Syntax highlighting with a diff, side by side or unified just doesn't make sense (you need the surrounding code context to establish the ability to correctly highlight, yet even when correctly highlighted, won't help a human since they won't see the context either necessarily). The only way to appropriately highlight diffs would involve cooperation between fossil and a highlighting system. When not enabled... it should fall back to its own internal method (or rather, when enabled, it should fall over to the syntax highlighter's method). I also don't believe that highlightjs or other syntax highlighters strip html code from a <pre><code> block for syntax highlighting purposes so the unified diff view would need extensive changing too, not to mention in the annotate and blame view the columns are inside the <pre><code> block as well meaning they'd be subject to the same issue. The only columnar view that supports this is side by side diff, and I don't think diffs should be highlighted (for reasons outlined above, https://yds.necessitu.de/repo/greyscanner/vdiff?from=3df63331b8db7124&to=3747ab374795de63 as example). As to annotate and blame being broken, see https://yds.necessitu.de/repo/greyscanner/annotate?filename=greyscanner.py&checkin=3df63331b8db7124 . The other reason to let syntax highlighters take care of line numbering (after creating a custom one to help do things the Fossil way) is because then the numbers and the syntax highlighter would be differently themed if done otherwise, leaving things looking broken of sorts too instead of integrated.

I think highlighting in markdown code blocks is a great idea, but for diffs/udiffs/annotate/blame I think highlighting will get in the way, or there will need to be quite an amount of rework either on Fossil's part or on part of the highlighter. I also think highlighting for code artifacts is a great idea, and that leaving numbering up to the highlighter is probably best for Fossil's purposes as well as for theming purposes and am sure more reasons could be fathomed.

(22.1) By yds on 2018-10-25 17:12:50 edited from 22.0 in reply to 21.1 [link] [source]

To better explore the side effects of wrapping artifacts with <pre><code> I cloned the Fossil repo to my patched server.

On 2018-06-28 07:11, Richard Hipp wrote:

> On 6/28/18, Lester L. Martin II i...@amlegion.org wrote:
>
>> This patch changes the way 'void artifact_page(void)' renders a files content.
>> Formerly a <blockquote><pre> was issued for content, whereas now a
>> <blockquote><pre><code class=$ext> is issued where $ext is the file's
>> extension (example, "blah.lua" extension would be "lua").
>
> But then the syntax highlighting goes away if you select line numbering, no?

No, syntax highlighting does not go away if you select line numbering. See: https://yds.necessitu.de/repo/fossil/artifact?udc=1&ln=on&name=7f1829c005cabfa6

For example, unified diff won't work even with the column approach although things are wrapped in a <pre><code>.

In my patch unified diffs get hardcoded to <pre class="udiff"><code class="diff"> which does the right thing: https://yds.necessitu.de/repo/fossil/fdiff?v1=b133f4c6524a7d9c&v2=414b1e249a3495d4&diff=1

Syntax highlighting with a diff, side by side or unified just doesn't make sense

Look pretty good to me: https://yds.necessitu.de/repo/fossil/fdiff?v1=b133f4c6524a7d9c&v2=414b1e249a3495d4 The line numbers are in columns of their own and don't interfere with the highlighting. diff parts always get a colored background and don't interfere with the syntax highlighting either. Similar to how GUIs like KDiff3 and WinMerge do coloring.

This same approach of isolating <pre><code> wrapped codeblocks from other info by putting each into their own table column could be used to fix the issues with line numbers changing colors and /annotate and /blame views. Neither of which looks all that broken with highlight.js as is, IMHO.

Besides, without <pre><code> wrapping of artifacts, if one were to integrate highlight.js by highlighting all <pre> blocks the results are worse:

  • not all <pre> blocks should trigger highlighting
  • there is no <code class="..."> language hinting, sometimes the wrong language gets auto selected.
  • all the same issues with line numbers, /annotate and /blame views still exist.

This patch breaks nothing and fixes a few bugs. There was at least one <pre> instead of </pre> and I was careful to eliminate any newlines immediately after <pre><code> to avoid any extraneous blank lines in the output. Users who do not use any syntax highlighting should see no difference. Those who do venture to integrate a syntax highlighter into their skin of choice will have a much easier time of it.

(23.2) By llmII (lmartin92) on 2018-10-25 18:26:06 edited from 23.1 in reply to 22.1 [link] [source]

No, syntax highlighting does not go away if you select line numbering.

I know, but with your patch, you shouldn't highlight whilst line numbering. I went down this path before, and found it so ugly and broken that I turned off highlighting whilst line numbering, then eventually created a way to do both simultaneously.

Example of how it is broken from lines 583 to 715 in the fossil repo you cloned: https://yds.necessitu.de/repo/fossil/artifact?udc=1&ln=on&name=7f1829c005cabfa6

Highlighting while some line numbers are showing doesn't mean its doing so in a decent manner, or that the way it is being done doesn't interfere with the highlighter itself.

In my patch unified diffs get hardcoded to <pre class="udiff"><code class="diff"> which does the right thing:

The problem with diffs, annotate and blame, is that they all use the same method of displaying numbers in a left column, wherein the numbers are wrapped in a <pre><code> block. I have yet to find where unified diff looks bad, but its using the same method as the others and thus is lumped in. Side by side is almost good, and in your example, it was good, but being good in one place doesn't mean that it is good in most places.

Annotation example showing illegibility of the metadata columns (commit hash, line numbers, etc) where it would be impossible to read the metadata coolumn without straining: https://yds.necessitu.de/repo/fossil/annotate?filename=src/info.c&checkin=a9e5a1eefd71c5b7

By illegible I mean the colors in the metadata columns are non-uniform making them difficult to try and read at a scroll, or even just look and find where that one line was changed in just one commit.

Blame running into a similar issue: https://yds.necessitu.de/repo/fossil/blame?filename=src/info.c&checkin=a9e5a1eefd71c5b7

Side by side where comments are highlighted not as comments, but some random jumble of stuff: https://yds.necessitu.de/repo/greyscanner/vdiff?from=3df63331b8db7124&to=3747ab374795de63

Good example of a side by side (yet the same code that yeilds it yeilds the previous broken one): https://yds.necessitu.de/repo/fossil/vdiff?from=35563f3db308ca33&to=1c32c5b83271f203

This same approach of isolating <pre><code> wrapped codeblocks from other info by putting each into their own table column could be used to fix the issues with line numbers changing colors and /annotate and /blame views. Neither of which looks all that broken with highlight.js as is, IMHO.

Do you mean the approach taken in side by side diffs? That might work (uses html tables, and I'm running on what I remember so feel free to say I'm wrong as to). The approach taken in blame, annotate, and unified diff, is not viable for integration however without creating a heavily modified highlightjs and maintaining it (or any other syntax highlighting library).

Besides, without <pre><code> wrapping of artifacts, if one were to integrate highlight.js by highlighting all <pre> blocks the results are worse, cuz not all <pre> blocks should trigger highlighting, and all the same issues with line numbers, /annotate and /blame views still exist.

I didn't mean it as don't wrap artifacts in <pre><code>, the issue is when the <pre><code> wraps <span>'s that have first span is a line number, second is the code (as done in diffs, annotate, blame), or when line numbers are just plain text inlined beside the actual code. Neither of these should be wrapped unless some method is developed to integrate it appropriately with syntax highlighting otherwise it'll get syntax highlighted when it shouldn't be due to the results looking highly broken.

This patch breaks nothing and fixes a few bugs.

No this doesn't break Fossil technically, however the point of the patch is to enable syntax highlighting of a few different views. The problem is that Fossil works fine without this code to display those, but once this patch is used as proposed, Fossil will render things that the syntax highlighter will turn around and re-render in an utterly broken manner as per examples above.

It took me less than 5 minutes to come up with my examples, so they're not really contrived edge cases, they're common. Edge cases take time to find, common cases don't take that long.

Those who do venture to integrate a syntax highlighter into their skin of choice will have a much easier time of it.

My original patch made it easy and avoided tough subjects such as diffs, annotate, and blame. It also made it easy to have something that I would be willing to say will display the subject I focused on, artifact highlighting, correctly most of the time, to the equivalence of other projects that include syntax highlighters working correctly most of the time.

Between my writing this post and people's viewing of this post, the skin theme has changed from when originally the post this post is replying to was made. Note that the visual defects are harder to see when rendered with light background dark text highlighting and/or website style than when rendered with dark background light text.

(25.5) By yds on 2018-10-27 00:51:04 edited from 25.4 in reply to 23.2 [link] [source]

"Highlight.js’ notable lack of line numbers support is not an oversight but a feature" seems to be the official stance on the matter from that camp.

No this doesn't break Fossil technically, however the point of the patch is to enable syntax highlighting of a few different views. The problem is that Fossil works fine without this code to display those, but once this patch is used as proposed, Fossil will render things that the syntax highlighter will turn around and re-render in an utterly broken manner as per examples above.

You're right in pointing out that highlight.js is kinda broken with line numbers. However I still believe there is no need for a global syntax-hl on/off switch since there's a plethora of fine grained knobs already baked into Fossil:

if {"/$current_page" ne "/annotate" && "/$current_page" ne "/blame" && [getParameter ln] eq {}} {...

See full example <head> section in this post.

I edited my patch above to add an new (optional) skin section called head.txt — but having gone through the exercise of adding convenient UI in the /setup_skin page where the <head> part can be worked on in isolation from the rest of the header.txt — I don't know if it's really needed. The same thing could be done in header.txt without the extra head.txt.

I hear what you're saying about broken line number handling, but that's not a Fossil issue — Fossil not wrapping artifacts with <pre><code> is a Fossil issue. The point of my patch is to fix that and that alone. The broken line numbering could be handled with creative application of TH1 in the <head> section to maybe conditionally load different js which does handle line numbering. Or turn off loading the highlight.js cruft altogether for certain pages where it breaks, like in my example above.

Click around the different views of my Fossil repo clone to see how the logic from this <head> disables loading highlight.js where it breaks things. Hit Ctrl+U on the different pages to see the <head> content change.

(26) By llmII (lmartin92) on 2018-10-27 04:18:33 in reply to 25.5 [link] [source]

Then we're back to what Richard Hipp wrote.

Now syntax highlighting doesn't work with line numbering. Which gets back to fossil needs to know when to output things differently to enable syntax highlighting with line numbering, hence the "syntax-hl" knob. No current knob changes the way fossil outputs HTML, nor can TH1 do the same in its own right to my knowledge.

I edited my patch above to add an new (optional) skin section called head.txt — but having gone through the exercise of adding convenient UI in the /setup_skin page where the <head> part can be worked on in isolation from the rest of the header.txt — I don't know if it's really needed. The same thing could be done in header.txt without the extra head.txt.

Probably easiest to just leave it to editing header.txt.

I hear what you're saying about broken line number handling, but that's not a Fossil issue — Fossil not wrapping artifacts with <pre><code> is a Fossil issue. The point of my patch is to fix that and that alone. The broken line numbering could be handled with creative application of TH1 in the <head> section to maybe conditionally load different js which does handle line numbering. Or turn off loading the highlight.js cruft altogether for certain pages where it breaks, like in my example above.

Nothing TH1 can do will change the output that Fossil generates that in turn some syntax highlighter is responsible for dealing with. So as far as it not being a Fossil issue, Fossil being incapable of generating something that can be syntax highlighted whilst numbering might could be considered an issue (the what Richard Hipp said thing above). Just conditionally loading different js won't work because there is no js library that I know of that will handle how Fossil does line numbering. Not to mention to get line numbering working elsewhere than the artifact view means rewriting a line numbering library several times, whereas getting Fossil to generate something that can be handled by a highlighter is far easier.

This is the correct path though if no knob is going to be added to help Fossil handle outputting for a highlighter. Sort of arrives (by different means) at what I was stuck on at first myself, that no highlighting be done with line numbers because of inherent brokenness, and that something needs to be created to make such possible (either within Fossil, as I did, or otherwise). It will be far easier to have Fossil output HTML that a highlighter can understand for these purposes than it will be to create and/or modify a line numbering script to do little more than changing the way it generates links (Fossil style) or handles selections and so forth. Some syntax highlighter out there may already have line numbering built in which would further complicate matters when it becomes only the js responsibility (means maintaining a fork of that highlighter) vs having Fossil output HTML that basically any syntax highlighter will understand. Using <pre><code> everywhere gets things started in the right direction, further work will need be done for with line numbering (and probably need knobs), and yet further would need to be done to make things happen for /annotate and /blame, and for diffs we arrive at the issue that a syntax highlighter has no business highlighting code within a diff due to it not having enough context to generate proper highlighting (yes, the ones linked look nice, but one will run up against the side by side I linked earlier that shows it just doesn't work).

(27) By yds on 2018-10-27 15:47:51 in reply to 26 [link] [source]

Probably easiest to just leave it to editing header.txt.

I edited my patch above to revert back to purely adding <pre><code> where needed to make syntax highlighting more controllable. That oughtta make it easier to audit for inadvertent bugs.

The point of my patch was never to tackle the issue of line numbering, or any other cases any particular highlighter is not supporting. If the /fdiff page with highlighting is not to one's liking on a particular repo — it's simple enough to exclude loading the highlighter altogether. It's also simple enough to not exclude highlighting on a repo where line numbering does not break too badly such as lua code. It's upto the repo admin to decide, once they hack their skin of choice to add syntax highlighting, when and where they want it on or not.

To make Fossil's line numbering view as well as /annotate and /blame work with any syntax highlighter, I believe the right approach is to implement those views the way Fossil already does in the /fdiff view — i.e. put the artifact to be highlighted in its own table column.

(28) By llmII (lmartin92) on 2018-10-27 18:15:03 in reply to 27 [link] [source]

The point of my patch was never to tackle the issue of line numbering, or any other cases any particular highlighter is not supporting. If the /fdiff page with highlighting is not to one's liking on a particular repo — it's simple enough to exclude loading the highlighter altogether. It's also simple enough to not exclude highlighting on a repo where line numbering does not break too badly such as lua code. It's upto the repo admin to decide, once they hack their skin of choice to add syntax highlighting, when and where they want it on or not.

Whilst highlightjs does not explicitly support it, there exist extensions to handle it (for normal cases, and fossil's case). Wherein the latter is based upon the former and the former I believe is fairly widely used. Then you have other syntax highlighters that implement line numbering as well (prismjs). The main aspect being that both of them expect a <pre><code> unadulturated with any markup within. The key here is the patch is supposed to wrap things so syntax highlighters can work, yet the way its doing so means that it only works with the circumstances a person chooses and it should support all circumstances where it makes sense such that disablement is a choice not due to brokenness, but due to not wanting. Diff highlighting remains a broken concept as mentioned in previous posts, simply because it may try to highlight the underlying language which it can't do due to lack of context. What I'm trying to say is that disabling something shouldn't be about it not working, it should be because that's what's wanted. Enablement should never happen for things where it doesn't make sense except as a consequence of the user explicitly doing so.

I believe the right approach is to implement those views the way Fossil already does in the /fdiff view — i.e. put the artifact to be highlighted in its own table column.

Implementing it that way could be a valid approach, but something needs to be done to make line numbering possible out of the box rather than "maybe it works, maybe it doesn't" if one decides to implement syntax highlighting for their site. However, the default for all diff, annotate, and blame views, need to be disabled by default, and diff should never be done excepting that Fossil hands over the full code for highlighting somehow. The only real reason to change to <pre><code> in places is to cooperate with syntax highlighters, otherwise the way it was done prior is more than okay. So sadly if implementing this it does actually need to tackle the issue of line numbering where it makes sense. I could probably modify prismjs's plugin in a day or so to detect the "ln" param on the url, and to implement linking like how I did with the highlightjs line numbering extension (I wouldn't however because I'm not planning to use prismjs anyway). It definitely does not fall to these syntax highlighting projects the responsibility of supporting Fossil's way of doing such as they are widely implemented and are needing to work in generic cases.

(4) By EganSolo on 2018-10-15 21:28:04 in reply to 1.0 [link] [source]

Ditto!

I have begun documenting in a wiki a large code-base and, yeah, having to code the markdown by hand is a pain.

I'd be happy to act as a tester for any patching done :)

Egan

(5) By PF (peter) on 2018-10-16 00:07:10 in reply to 4 [link] [source]

Hi,

There is a fossil branch "commonmark-markdown" which provides support for fenced code blocks. It can be compiled either on its own or merged with trunk fossil. Resulting binary will allow use fenced blocks with language hinting.

Making "trunk" version of commonmark capable fossil on linux is possible by similar steps:

#in repo root directory
fossil update trunk
fossil merge commonmark-markdown
#in src subdir:
tclsh makemake.tcl
#in repo root directory
./configure
make

Rendering results would be

####markdown snippet:

    require 'redcarpet'
    markdown = Redcarpet.new("Hello World!")
    puts markdown.to_html

####resulting html code: require 'redcarpet' markdown = Redcarpet.new("Hello World!") puts markdown.to_html

There are sure differences between current markdown implementation and commonmark one. I have noticed that current implementation is forgiving with heading where hash is followed immediately by heading text.

###Heading in current fossil markdown is not recognized without space before H in commonmark-markdown.

Sometimes fossil repo seems like real fossil. You never know what can find there.

Peter

(6) By yds on 2018-10-16 00:42:25 in reply to 5 [link] [source]

That's perfect! :D Can we have commonmark-markdown merged for the next fossil release? Please. :)

(8) By anonymous on 2018-10-16 15:07:49 in reply to 6 [link] [source]

+1. I think that if a commonmark-markdown is already there, in a branch, having it by default integrated in Fossil, would improve support more demanding task regarding documentation in the repositories, that current plain Markdown Fossil implementation doesn't bring (CommonMark is a effort to bring that support and unify a lot of variants of Markdown).

Meanwhile, I'm using Markdeep as a light way to bring improved documentation in my Fossil repositories, as you can see in these examples:

Cheers,

Offray

(29) By ckennedy on 2018-10-28 19:00:48 in reply to 6 [link] [source]

So, it looks like there was the thought that perhaps the commonmark-markdown branch would handle the original request, but I see most of the discussion being about the proposed patch and syntax highlighting/line numbering. Personally I would just like to be able to use the syntax for fenced code blocks without Fossil getting confused. Syntax highlighting would be pure sugar on top, and not needed as much as supporting what has become a very common markdown feature.

Thanks.

(7) By Stephan Beal (stephan) on 2018-10-16 01:03:45 in reply to 5 [link] [source]

Peter said: "There are sure differences between current markdown implementation and commonmark one. I have noticed that current implementation is forgiving with heading where hash is followed immediately by heading text."

Just entirely coincidentally, i ran into a use case today on a markdown-using social media site where that space-handling behaviour would actually be a bug: if the markdown parser recognizes #X (as opposed to (# X)) as a header then the site using the parser cannot support hashtags at the start of a line. It's common practice to place all hashtags for a post on a line of their own at the start or end of the post.

i.e. treating #X, with no space, as a header is a bug, IMO. i rather assume that that's why CommonMark requires a space for headers.