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

writers: Recognize extra syntax definitions #7241

Merged
merged 1 commit into from
Apr 25, 2021

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Apr 23, 2021

Since Skylighting does not support Nix syntax, we need to add custom syntax definition using --syntax-definition flag, otherwise files with nix extension will be recognized as bash scripts by the default syntax map. This means that code blocks with nix class will end up with language=bash when converted to DocBook and other formats.

Unfortunately, the writers always use the default syntax map so adding custom definition will not help.

This patch corrects that, allowing the writers to see all custom definitions.

The LaTeX reader suffers from the same issue but it has not been fixed there since it would require wider changes.

Test

$ printf '```nix\nfoo\n```' | cabal exec -- pandoc -f markdown -t docbook
<programlisting language="bash">
foo
</programlisting>
$ printf '```nix\nfoo\n```' | cabal exec -- pandoc -f markdown -t docbook --syntax-definition=<(curl -s 'https://gitlab.com/rycee/presentations/-/raw/da77a89576fe8eed2a8b4f73e29b31895d45c98f/2019-12-cph-nur/nix-syntax.xml')
<programlisting language="nix">
foo
</programlisting>

Since Skylighting does not support Nix syntax, we need to add custom syntax definition using --syntax-definition flag,
otherwise files with nix extension will be recognized as bash scripts by the default syntax map.
This means that code blocks with nix class will end up with language=bash when converted to DocBook and other formats.

Unfortunately, the writers always use the default syntax map so adding custom definition will not help.

This patch corrects that, allowing the writers to see all custom definitions.

The LaTeX reader suffers from the same issue but it has not been fixed there since it would require wider changes.
@jtojnar
Copy link
Contributor Author

jtojnar commented Apr 25, 2021

Fixed the -Wname-shadowing warning.

[T.toLower (sName s) | s <- syntaxesByExtension defaultSyntaxMap (T.unpack ext)]
languagesByExtension :: SyntaxMap -> T.Text -> [T.Text]
languagesByExtension syntaxmap ext =
[T.toLower (sName s) | s <- syntaxesByExtension syntaxmap (T.unpack ext)]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this would be a breaking API change. (Not a reason against, necessarily, but should be noted very clearly in the commit log.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add something like + Text.Pandoc.Highlighting: languages and languagesByExtension now take an additional SyntaxMap argument. Get it from writerOpts or use Skylighting’s defaultSyntaxMap. [API change].

Or I can create new functions and make the old ones call them with defaultSyntaxMap.

@jgm jgm merged commit c56d080 into jgm:master Apr 25, 2021
@jgm
Copy link
Owner

jgm commented Apr 25, 2021

It looks good otherwise. I just merged it, and (doh!) forgot to add the commit message noted above.

jgm added a commit that referenced this pull request Apr 25, 2021
Also taking this opportunity to note, for the record, that
the commit for #7241 should be marked [API change].
It changes the type of `languagesByExtension` in Highlighting,
adding a parameter for a `SyntaxMap`.
@jtojnar jtojnar deleted the writer-extra-syntaxes branch April 25, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants