enable syntax highlighting for inline code #17585

a-mr · 2021-03-30T19:39:38Z

Add highlighting of text in single backticks as Nim code, just like it works for code-blocks.

An example from Nim Manual:

An artificial example:

the corresponding RST text:

Use `import std/os` statement to import this module.
More examples: `proc f()` and `var x: int` and `result = 5`.
So `"abc".match(re"(\w)").get.captures[0] == "a"`.

And 6 other languages that ``highlite.nim`` supports too:

* Python: `class X(object): pass`:python:
* C: `typedef unsigned char BYTE;`:c:
* YAML: `- {name: John Smith, age: 33}`:yaml:
* Java: `double x = Math.sin(1);`:java:
* C#: `listOfFoo.Where(delegate(Foo x) { return x.size > 10; });`:csharp:
* C++: `throw std::runtime_error("error");`:cpp:

Literals without highlighting still can be input: `import`:literal: and ``import``.

It also highlights blindly non-code like this:

To avoid this spurious highlighting the following rule is added to contributing.rst for using single and double backticks:

use single backticks for fragments of code in Nim and other
programming languages, including identifiers
prefer double backticks otherwise:
- for file names: ``os.nim``
- for fragments of strings not enclosed by " and " and not
  related to code, e.g. text of compiler messages
- for command line options: ``--docInternal``
- also when code ends with a standalone \ (otherwise a combination of
  \ and a final ` would get escaped)

Ref. nim-lang/RFCs#355 for discussion on syntax.

Later it will be possible to switch default role (programming language or just literal), ref bug #17340.
cc @narimiran @timotheecour

timotheecour · 2021-03-30T20:14:30Z

@a-mr

how about disable the automatic syntax highlighting as nim for single backtick that doesn't contain an explicit :nim: role? I'm assuming this should be a small change in your PR.

IMO that's the only controversial thing in this PR due to the other implications you listed. In subsequent PR we can revisit that point, so that it doesn't block the rest of this PR

in future work, it would be nice to support string litterals with syntax highlighting, which emit could then use, either explicitly or via compiler logic, eg:

{.emit: """
#include <stdio.h>
""".lang(cpp).}

const s = """
import os
""".lang(py)
writeFile("foo.py", s)

# or even reusing apostrophe syntax, eg: 
const s = """
import os
"""'py

where lang(cpp) would not affect codegen but would affect rendering

a-mr · 2021-03-30T20:29:41Z

@timotheecour
I grepped for both single and double backticks and estimated roughly that it's actually code in more than 95% cases in *.nim files (and it's almost always Nim code) and more than 50% in *.rst in this repo. You can grep and check yourself.

So it makes sense to make Nim code the default option and change to double backticks only when needed, at least for *.nim.
Also different *.rst files have different proportions of those 2 cases, so it makes sense to change .. default-role:: literal only for some of them.

default-role can be implemented in this PR or in a follow-up.

timotheecour · 2021-03-30T21:19:11Z

then how about this for single backtick without explicit role:

if in nim source file, use implicit nim role and syntax highlight as nim
if in rst source file, don't use implicit nim role; at least for now (can be revisited later, maybe)

that fits well with your 95% vs 50% stats, as well as with rst spec (ie honors default-role:code which is now present in most/all rst files); in subsequent PR we can think about whether to change those rst files to default-role:nim or similar)

a-mr · 2021-03-30T21:49:44Z

I think we will have another spell in every rst file instead:

.. role:: nim(code)
   :language: nim
.. default-role:: nim

Github does not highlight it but it's still rendered as code: https://github.com/a-mr/Nim/blob/test-branch/doc/tut2.rst

(of course, currently Nim would say "Error: invalid directive: 'role'")

timotheecour · 2021-03-30T22:52:02Z

I think we will have another spell in every rst file instead:

can these lines be replace by a single include instead?
something like:

.. include:: rstcommon.rst

and then we can add to rstcommon.rst whatever's in common, eg:

.. role:: nim(code)
   :language: nim
.. default-role:: nim

(and perhaps more, eg related to a common index, or C language highlighting, etc)
(maybe #4864 needs to be fixed first?)

a-mr · 2021-03-30T23:22:03Z

Tried it. include works for rst2html.py but does not for Github: https://github.com/a-mr/Nim/blob/test-branch/doc/tut1.rst — it displayed as a normal text.

(#4864 seems unrelated)

timotheecour · 2021-03-30T23:49:00Z

ok how about:

.. default-role:: code
.. include:: rstcommon.rst

which should at least display as code in github, and then still allows factoring all common definitions in 1 place (including overriding .. default-role:: code if needed)

implementation

a-mr · 2021-03-31T23:23:23Z

Agreed.

Added missing parts for default-role and some minimal support for role directive.

doc/contributing.rst

timotheecour · 2021-04-02T02:04:36Z

lib/packages/docutils/rst.nim

@@ -514,10 +525,17 @@ proc defaultFindFile*(filename: string): string =
  if fileExists(filename): result = filename
  else: result = ""

+proc defaultRoleKind(options: RstParseOptions): RstNodeKind =


instead of adding defaultRoleKind, you could infer it from currRole, eg:

p.s.currRoleKind = defaultRoleKind(p.s.options) p.s.currRole = defaultRole(p.s.options) => p.s.currRole = defaultRole(p.s.options) p.s.currRoleKind = whichRole(p, p.s.currRole)

(a bit more DRY)

timotheecour · 2021-04-02T02:07:16Z

lib/packages/docutils/rst.nim

+proc defaultRoleKind(options: RstParseOptions): RstNodeKind =
+  if roNimFile in options: rnInlineCode else: rnInlineLiteral
+proc defaultRole(options: RstParseOptions): string =
+  if roNimFile in options: "nim" else: "literal"


add

type BuiltinRoles = enum brUnknown = "unknown" brNim = "nim" brLiteral = "literal" brCode = "code" ... # doesn't need to be a complete list

and then use it everytime you're using one of those builtin roles, it's more typesafe eg:

if roNimFile in options: $brNim else: $brLiteral

too little profit for a new type. Most roles are met only once and immediately converted to RstNodeKind. The only exception is "nim" and "literal", which occur twice, but both occurrences are within a few lines.

timotheecour · 2021-04-02T02:09:56Z

lib/packages/docutils/rst.nim

@@ -1018,15 +1036,43 @@ proc fixupEmbeddedRef(n, a, b: PRstNode) =
  for i in countup(0, sep - incr): a.add(n.sons[i])
  for i in countup(sep + 1, n.len - 2): b.add(n.sons[i])

-proc whichRole(sym: string): RstNodeKind =
-  case sym
+const supportedLanguages = ["nim", "yaml", "python", "java", "c",


is that supposed to stay in sync with

const sourceLanguageToStr*: array[SourceLanguage, string] = ["none", "Nim", "C++", "C#", "C", "Java", "Yaml", "Python"]

from lib/packages/docutils/highlite.nim? if so, refactor (DRY)

ideally, yes, but mirror should be maintained anyway, because c# role would not be allowed according to RST spec:

A role name is a single word consisting of alphanumerics plus isolated internal hyphens, underscores, plus signs, colons, and periods; no whitespace or other characters are allowed.

timotheecour · 2021-04-02T02:34:42Z

lib/packages/docutils/rst.nim

+  result = newRstNode(rnInlineCode)
+  var args = newRstNode(rnDirArg)
+  var lang = language
+  if language == "cpp": lang = "c++"


something is inconsistent:

.. code-block:: cpp #include <stdio.h> .. code-block:: c++ #include <stdio.h> `#include <stdio.h>`:cpp: `#include <stdio.h>`:c++:

gives

=> depending on block vs inline, the syntax that works is c++ vs cpp

we should have one that works in all contexts, and recommend it (including in example from eariler).

The fact that :c++: is not parsed is another bug in our parser. Added to the list of minor bugs.

The proper fix (with code reusing) is not trivial, it's better to handle in another PR.

lib/packages/docutils/rst.nim

timotheecour · 2021-04-02T02:38:16Z

lib/packages/docutils/rst.nim

@@ -1052,14 +1098,23 @@ proc parsePostfix(p: var RstParser, n: PRstNode): PRstNode =
    result = newRstNode(newKind, newSons)
  elif match(p, p.idx, ":w:"):
    # a role:
-    newKind = whichRole(nextTok(p).symbol)
+    let roleName = nextTok(p).symbol
+    newKind = whichRole(p, roleName)
    if newKind == rnGeneralRole:


case newKind
of ...
of ...
else: ...

lib/packages/docutils/rst.nim

timotheecour · 2021-04-02T02:46:07Z

lib/packages/docutils/rstgen.nim

-  dispA(d.target, result, blockStart,
-        "\\begin{rstpre}\n" & n.anchor.idS & "\n", [])
+    else:  # rnInlineCode
+      blockStart = "<tt class=\"docutils literal\"><span class=\"pre\">"


prefer """ ... """ to make it easier to copy paste

timotheecour · 2021-04-02T02:54:01Z

tests/stdlib/trstgen.nim

@@ -201,14 +206,14 @@ not in table"""
        `|` outside a table cell should render as `\|`
    consistently with markdown, see https://stackoverflow.com/a/66557930/1426932
    ]#
-    doAssert output1 == """
+    check(output1 == """


how about import std/strformat and using &"". {...}..""" to avoid breaking the string?

ditto in other examples below

IMO, resulting code is less noisy, easier to read/edit

timotheecour · 2021-04-02T03:11:54Z

tests/stdlib/trstgen.nim

-    check """`foo\`bar`""".toHtml == """<tt class="docutils literal"><span class="pre">foo`bar</span></tt>"""
-    check """`\`bar`""".toHtml == """<tt class="docutils literal"><span class="pre">`bar</span></tt>"""
-    check """`a\b\x\\ar`""".toHtml == """<tt class="docutils literal"><span class="pre">a\b\x\\ar</span></tt>"""
+    check("""`foo.bar`""".toHtml ==


these get more and more complicated to read/write/maintain, we should really look into timotheecour#676 at some point

timotheecour

LGTM, remaining comments can be addressed preferably before merging but in followup PR is ok too

Co-authored-by: Timothee Cour <[email protected]>

lib/packages/docutils/rst.nim

… enable-inline-highlighting

timotheecour

still LGTM , and previous LGTM comment still applies: remaining comments can be addressed either in this PR or in followup PR

enable syntax highlighting for inline code

3250644

finish '.. default-role' and preliminary '.. role'

32ea770

implementation

more compact check in dirRole

c9c7268

timotheecour reviewed Apr 1, 2021

View reviewed changes

doc/contributing.rst Show resolved Hide resolved

timotheecour reviewed Apr 1, 2021

View reviewed changes

doc/contributing.rst Show resolved Hide resolved

timotheecour reviewed Apr 1, 2021

View reviewed changes

doc/contributing.rst Show resolved Hide resolved

set :literal: as default role for *.rst

39c601d

timotheecour reviewed Apr 2, 2021

View reviewed changes

lib/packages/docutils/rst.nim Outdated Show resolved Hide resolved

timotheecour reviewed Apr 2, 2021

View reviewed changes

lib/packages/docutils/rst.nim Outdated Show resolved Hide resolved

timotheecour reviewed Apr 2, 2021

View reviewed changes

timotheecour approved these changes Apr 2, 2021

View reviewed changes

a-mr and others added 3 commits April 2, 2021 19:29

Update lib/packages/docutils/rst.nim

baa7115

Co-authored-by: Timothee Cour <[email protected]>

use whichRole for setting currRoleKind

cd462c1

Update lib/packages/docutils/rst.nim

17b7cdd

Co-authored-by: Timothee Cour <[email protected]>

timotheecour reviewed Apr 2, 2021

View reviewed changes

lib/packages/docutils/rst.nim Outdated Show resolved Hide resolved

a-mr mentioned this pull request Apr 2, 2021

RST minor bugs #17340

Open

28 tasks

a-mr added 2 commits April 2, 2021 21:17

rename rnGeneralRole -> rnUnknownRole

32541fe

Merge branch 'enable-inline-highlighting' of github.com:a-mr/Nim into…

7838863

… enable-inline-highlighting

timotheecour approved these changes Apr 2, 2021

View reviewed changes

Araq merged commit e35946f into nim-lang:devel Apr 2, 2021

timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Apr 2, 2021

a-mr mentioned this pull request Apr 10, 2021

turn on syntax highlighting in Manual & Tutorial #17692

Merged

a-mr mentioned this pull request Apr 21, 2021

add RST highlighting for command line / shells (also fixes #16858) #17789

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable syntax highlighting for inline code #17585

enable syntax highlighting for inline code #17585

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021 •

edited

Loading

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021 •

edited

Loading

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021

a-mr commented Mar 30, 2021

timotheecour commented Mar 30, 2021 •

edited

Loading

a-mr commented Mar 31, 2021

timotheecour Apr 2, 2021

a-mr Apr 2, 2021

timotheecour Apr 2, 2021 •

edited

Loading

a-mr Apr 2, 2021

timotheecour Apr 2, 2021 •

edited

Loading

a-mr Apr 2, 2021

timotheecour Apr 2, 2021

a-mr Apr 2, 2021

timotheecour Apr 2, 2021

timotheecour Apr 2, 2021

timotheecour Apr 2, 2021 •

edited

Loading

timotheecour Apr 2, 2021

timotheecour left a comment

timotheecour left a comment

enable syntax highlighting for inline code #17585

enable syntax highlighting for inline code #17585

Conversation

a-mr commented Mar 30, 2021 • edited Loading

timotheecour commented Mar 30, 2021 • edited Loading

a-mr commented Mar 30, 2021 • edited Loading

timotheecour commented Mar 30, 2021 • edited Loading

a-mr commented Mar 30, 2021 • edited Loading

timotheecour commented Mar 30, 2021

a-mr commented Mar 30, 2021

timotheecour commented Mar 30, 2021 • edited Loading

a-mr commented Mar 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour left a comment

Choose a reason for hiding this comment

timotheecour left a comment

Choose a reason for hiding this comment

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021 •

edited

Loading

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021 •

edited

Loading

a-mr commented Mar 30, 2021 •

edited

Loading

timotheecour commented Mar 30, 2021 •

edited

Loading

timotheecour Apr 2, 2021 •

edited

Loading

timotheecour Apr 2, 2021 •

edited

Loading

timotheecour Apr 2, 2021 •

edited

Loading