Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add std/syntaxes to hint syntax highlighters about string literals #17722

Closed
wants to merge 9 commits into from

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 15, 2021

this PR allows highlighting hints

with the following additions to Syntaxes/Nim.YAML-tmLanguage (https://github.com/Varriount/NimLime, also used for github syntax highlight via github linguist):
https://gist.github.com/timotheecour/192fcbb8fda23b2012f8f420d88bf893

the std/syntaxes module renders as follows:

image

future work

  • make a PR against https://github.com/Varriount/NimLime with the above mentioned changes so that it works in sublime text
  • ditto with github linguist; hopefully the change to NimLime is enough
  • support asm with strings that are not literals, which would allow using asm lang"js ..."

@Clyybber
Copy link
Contributor

This should be under tools/ or tests/

@timotheecour
Copy link
Member Author

timotheecour commented Apr 15, 2021

This should be under tools/ or tests/

That would defeat the whole point; tools is mostly for standalone binaries, not for reusable library code; tests is not importable by users. The point is to allow users to syntax highlight string literals in nim files, using simple and explicit annotation that a syntax highlighter (sublime text, github) can exploit. This is common if you're doing ffi and need emit statements (c, cpp, js, asm) or even need to generate code in other languages from nim (eg python).

We can bikeshed the module name though, if you have a better name in mind. Other candidates:

  • std/syntaxhints

note

this will avoid bugs like this:
this is how lib/pure/hashes.nim renders in sublime text:

image

because in asm, ; means comment and sublime text's syntax grammars defers to source.asm to parse the string literal preceded by asm, causing it to treat closing """ as part of the comment. While it's easy to "fix" the code in hashes.nim to avoid generating a ; in the asm code (which is for nim js) or add a newline (as shown below), the problem remains that syntax highlighter can't guess whether a string literal should be js or asm (or c, c++ etc), in particular when the string literal is produced in a different place then where it's being used (eg: const someStringContainingJs = ...) or for nim code that generates code in some language (including nim!).

  when defined(js):
    var p: cstring
    asm """`p` = `Data`;"""
  else:
    var p = cast[cstring](data)

=>

  when defined(js):
    var p: cstring
    asm """`p` = `Data`;
"""
  else:
    var p = cast[cstring](data)

@timotheecour timotheecour marked this pull request as ready for review April 15, 2021 17:02
@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Apr 15, 2021
@Araq
Copy link
Member

Araq commented Apr 15, 2021

I don't understand. Can't the syntax highlighter simply stick to the spec and render multi line string literals as string literals?

@Araq
Copy link
Member

Araq commented Apr 15, 2021

About the name ... we need to distinguish between "std" which should be reserved for core libraries and "extensions" which are simply kinda there because we like the monorepo.

@timotheecour
Copy link
Member Author

timotheecour commented Apr 15, 2021

I don't understand. Can't the syntax highlighter simply stick to the spec and render multi line string literals as string literals?

after this PR, we can make syntax highlighters stick to the spec and render """ ... """ as multiline string literal, unless it's prefixed with a syntax hint (xxxLang"""...""")in which case it'll apply the corresponding syntax highlight. Before this PR it was forced to make an educated guess, with often bad results due to underlying ambiguity. (unless your question was: why do we need to allow syntax highlighting string literals, which for me sounds obvious when writing ffi etc).

Note that tooling beyond syntax highlighters can exploit this (eg linters etc).

About the name ... we need to distinguish between "std" which should be reserved for core libraries and "extensions" which are simply kinda there because we like the monorepo.

I don't have an objection with moving this file to lib/stdx/syntaxes. stdx is better than extensions and indicates stdlib origin, is short, self-explanatory, and has some precedent in other languages for such purposes (eg: see below). I'd rather move this file once though, so are you ok with lib/stdx/syntaxes? if so, followup PR's can be made to support stdx in kochdocs (as i had done in #14493)

Note that std and stdx should be able to mutually import each other, otherwise this again defeats purpose.

stdx in other langs

(these are not necessarily official, but it doesn't matter)

in stdx x would stand for extension, not experimental, ie the argument against std/experimental (moving out of experimental would break code) would not apply.

@Araq
Copy link
Member

Araq commented Apr 15, 2021

Ok, let's use "stdx" then. The module's name could be "language_annotations" or maybe "lang_prefixes". "syntaxes" is not clear enough.

@Clyybber
Copy link
Contributor

Clyybber commented Apr 15, 2021

This should be under tools/ or tests/

That would defeat the whole point; tools is mostly for standalone binaries, not for reusable library code; tests is not importable by users. The point is to allow users to syntax highlight string literals in nim files, using simple and explicit annotation that a syntax highlighter (sublime text, github) can exploit. This is common if you're doing ffi and need emit statements (c, cpp, js, asm) or even need to generate code in other languages from nim (eg python).

Oh, I thought all this was is a test file for highlighters :D

IMO this is not worth it, highlighters can just grow some heuristics instead or just display it as a plain string literal.

@Araq
Copy link
Member

Araq commented Apr 15, 2021

IMO this is not worth it, highlighters can just grow some heuristics instead.

Oh god no... These heuristics are always terrible.

@Clyybber
Copy link
Contributor

Oh god no... These heuristics are always terrible.

Agreed, but this isn't gonna improve anything realistically, since most users won't be aware that these hinting templates exist, and thus won't use them. This becomes even more true when we put it into some stdlib extension thing.

@Araq
Copy link
Member

Araq commented Apr 15, 2021

Agreed, but this isn't gonna improve anything realistically, since most users won't be aware that these hinting templates exist, and thus won't use them.

So what, we can use them ourselves and others will pick it up or not.

@Araq
Copy link
Member

Araq commented Apr 15, 2021

However, this must not turn into another distros.nim fiasco. There are so many PLs and markup languages around. We need to restrict it from the beginning to what .emit actually supports. Or maybe abandon the idea altogether, it's just a minor issue.

@Araq
Copy link
Member

Araq commented Apr 15, 2021

It should probably be std / emit_prefixes. And not stdx (yet?) as we don't need to introduce stdx just for that module.

@Clyybber
Copy link
Contributor

Clyybber commented Apr 15, 2021

We could also provide a highlighted proc (name up for debate :) which similarily to dedent would just strip
```someLang...``` from a string. Then one could write

  var code = highlighted"""```someLanguage
    ...
  ```"""

And we would only need highlighters to parse a highlighted""" string literal as markdown, would support all languages supported by the editors markdown highlighter, and would not need per-language support from the Nim side

@timotheecour
Copy link
Member Author

timotheecour commented Apr 15, 2021

piggybacking on @Clyybber's idea, here's my latest proposal:

  • a single highlight proc that doesn't require any imports (hence usable in low level code)
proc highlightImpl(a: string): string =
  for i in 0..<a.len:
    if a[i] == '\n':
      return a[i+1..a.len-1]

template highlight*(a: string{lit}): string =
  const b = highlightImpl(a)
  b

runnableExamples:
  let a = highlight"""js
if (!Math.trunc) {
  Math.trunc = function(v) {
    v = +v;
    if (!isFinite(v)) return v;
    return (v - v % 1) || (v < 0 ? -0 : v === 0 ? v : 0);
  };
}"""
  assert a == """
if (!Math.trunc) {
  Math.trunc = function(v) {
    v = +v;
    if (!isFinite(v)) return v;
    return (v - v % 1) || (v < 0 ? -0 : v === 0 ? v : 0);
  };
}"""

now all we need is to "recommend" a few custom languages but we don't need symbols for it, so users can use

highlight"""go
...
"""

if they want, nim doesn't need to know about this.

(it's simpler than

  var code = highlighted"""```someLanguage
    ...
  ```"""

)

I will update this PR soon

other candidate names after this change

  • stdx/highlights
  • stdx/strlits

Regarding stdx, well, we have to start somewhere... see stdx/readme.md in this PR; even if this module belongs in std/ rather than stdx

@Clyybber
Copy link
Contributor

Clyybber commented Apr 15, 2021

piggybacking on @Clyybber's idea, here's my latest proposal:

Awesome! (only remaining question is if it's as easy to make it support a few languages as with the markdown embedding, but I guess the respective markdown highlighter has to do that too, so should be possible)

other candidate names after this change

  • stdx/highlights
  • stdx/strlits

@Araq and I had a discussion on IRC, resulting in: we can put it in system.nim, as it would also be used there and that way users don't need to import anything to use it, increasing it's usage hopefully :)

@Araq
Copy link
Member

Araq commented Apr 16, 2021

@Araq and I had a discussion on IRC, resulting in: we can put it in system.nim, as it would also be used there and that way users don't need to import anything to use it, increasing it's usage hopefully :)

Er ... I didn't agree. Don't put it in system.nim. System.nim code can import other modules anyway these days.

@stale
Copy link

stale bot commented Apr 16, 2022

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Apr 16, 2022
@stale stale bot closed this May 31, 2022
@metagn metagn removed the TODO: followup needed remove tag once fixed or tracked elsewhere label Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Staled PR/issues; remove the label after fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants