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 support for ANSI colors #1441

Merged
merged 1 commit into from
Jul 6, 2021
Merged

Conversation

kimikage
Copy link
Contributor

@kimikage kimikage commented Oct 19, 2020

This starts from a Discourse topic: https://discourse.julialang.org/t/how-do-i-enable-ansi-colored-text-output-within-documenter-jl/48398

This changes:

  • add a new field / argument ansicolor to User / makedocs()
    • for global setting
  • add support for a keyword argument ansicolor in the @example block
    • for per-block setting to overide the global setting
  • use the ansicolor option for the :color I/O context property
  • translate "text/plain" output with ANSI escape codes in HTMLWriter
  • add styles for ANSI colors to the default themes

These changes allow you to colorize the lazy print objects in the @example output blocks.
ansi2

However, there are some issues to be resolved.

I've put an ANSI escape code translator for HTML in a new package AnsiColoredPrinters.jl to make this feature work. The package is experimental and under development, and there are things left to be discussed before it is registered, including the name of the package. I won't refuse to contribute it as part of Documenter.jl, but I prefer to modularize the functionality into minimal packages.

The other is that currently there is no way to specify the :color I/O context property for output to stdout / stderr. (Edit: solved by JuliaDocs/IOCapture.jl#1)
This means that even if the @example or @repl code potentially supports ANSI colors, we cannot get the colored result.

Also, although ansicolor is false by default and is the responsibility of the user, we need to strip the ANSI escape codes if the output format does not support ANSI colors. (This will be implemented soon.)

When the solutions for the above are found, we will need to discuss the compatibility with highlight.js, the theme for system 16 colors, etc.

@kimikage kimikage force-pushed the ansicolor branch 6 times, most recently from 7975d02 to f2ea031 Compare October 19, 2020 10:00
@fredrikekre
Copy link
Member

Nice! Is there ever a reason not to have this by default? Perhaps we can just look at the command line flag instead.

@kimikage
Copy link
Contributor Author

One problem with IOCapture is that redirect_{stdout|stderr}(::IOContext) is only available for Julia v1.6 (or later). (cf. JuliaLang/julia#36688)
Possible measures include:

  • giving up the color display of stdout/stderr on v1.5 or earlier
  • emulating PipeEndpoint as a subclass of LibuvStream
  • pirating get(::PipeEndpoint, key, default)

Do you have any other good ideas?

@fredrikekre
Copy link
Member

giving up the color display of stdout/stderr on v1.5 or earlier

I think this is fine.

@kimikage
Copy link
Contributor Author

giving up the color display of stdout/stderr on v1.5 or earlier

I think this is fine.

I also think it's the safer of the three. Even if we don't offer the colored stdout/stderr as a feature of these packages, perhaps defining get(::PipeEndpoint, key, default) in the user code (i.e. "docs/make.jl"), would not be too much of a problem.

@kimikage
Copy link
Contributor Author

There is still a lot of work to be done, but the stdout/stderr and @repl blocks are now colored in HTML.
Also, although I haven't fully tested it yet, the interference with highlight.js (v10.3.1) doesn't seem to be a problem. I'm looking forward to the improved highlighting. 😍

stdout
repl

As suggested in #1441 (comment), instead of adding the ansicolor keyword argument to makedocs, the :color property of stdout, which is determined by the --color command line flag, is now used as the default value.

@kimikage
Copy link
Contributor Author

Currently, I added the ansicolor keyword argument to the @example/@repl block, but it might be better to generalize it to IOContext. (cf. issue #1450)
However, we will face the problem of PipeEndpoint again. 😕

@kimikage kimikage force-pushed the ansicolor branch 5 times, most recently from 028d64e to 56a1e81 Compare October 30, 2020 14:46
@kimikage
Copy link
Contributor Author

kimikage commented Oct 30, 2020

The system 16 colors are now generated from variables such as $red by default.

So, I changed the default color scheme while maintaining as much of the current look as possible. (I'm going to separate the color scheme changes into another PR.)
Edit: The color change was purged for now.

16colors

Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

I've put an ANSI escape code translator for HTML in a new package AnsiColoredPrinters.jl to make this feature work. The package is experimental and under development, and there are things left to be discussed before it is registered, including the name of the package. I won't refuse to contribute it as part of Documenter.jl, but I prefer to modularize the functionality into minimal packages.

I think the functionality is quite well-defined and general, so it make sense to have it as a separate package. Would you mind having that under the JuliaDocs umbrella though?

I would love to have you as its primary maintainer, but it would be good if there is a wider set of people who would can, if need be, release quick fixes, since the Julia master branch will be depending on this code.

assets/html/scss/documenter/_ansicolors.scss Outdated Show resolved Hide resolved
src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
@mortenpi mortenpi added Format: HTML Related to the default HTML output Type: Feature labels Oct 31, 2020
@kimikage
Copy link
Contributor Author

kimikage commented Nov 1, 2020

Would you mind having that under the JuliaDocs umbrella though?

After making some miscellaneous changes (I think they will take about a week), I'd like to transfer the package to JuliaDocs. 😃

However, before we do that, we need to decide on the package name. Even if it is based on the current name, I'd like to change "Ansi" to "ANSI".

Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

However, before we do that, we need to decide on the package name. Even if it is based on the current name, I'd like to change "Ansi" to "ANSI".

I also think that the capitalized name would look better.

@kimikage
Copy link
Contributor Author

kimikage commented Nov 10, 2020

ANSIColoredPrinters' documentation is still unfriendly, but its code coverage is now over 90%. So I will apply for the transfer.
https://github.com/kimikage/ANSIColoredPrinters.jl/issues/10

@kimikage
Copy link
Contributor Author

kimikage commented Jul 2, 2021

Sorry I haven't been able to update this PR. I will try to rebase it in a few days.

@kimikage kimikage force-pushed the ansicolor branch 2 times, most recently from b9341bf to cc0f26a Compare July 3, 2021 14:27
@fredrikekre
Copy link
Member

Thanks for the rebase. Something seems wrong with the dark theme:
Screenshot from 2021-07-04 00-13-47

@fredrikekre
Copy link
Member

It also looks like hljs overrules things like

<code class="language-julia-repl ansi">julia&gt; printstyled("hello, world\n"; color=:blue)
<span class="sgr34">hello, world</span></code>

turning it into

<code class="language-julia-repl ansi hljs"><span class="hljs-meta">julia&gt;</span><span class="language-julia"> printstyled(<span class="hljs-string">"hello, world\n"</span>; color=:blue)
</span>hello, world</code>

@fredrikekre
Copy link
Member

For

```@repl
println("hello")
```

perhaps we need to emit something like

<pre>
<code "language-julia-repl">
println("hello")
</code>
<code "nohighlight">
hello
</code>
</pre>

i.e. split into two blocks where only the first is used with hljs

@fredrikekre
Copy link
Member

fredrikekre commented Jul 4, 2021

I have some fixes for my last two comments which I can push after. I think that this PR can be merged once the dark theme css is fixed and I will make a follow up PR:

@fredrikekre
Copy link
Member

OMG in the test package I tried this on I had a custom css file overriding the documenter one.....

@kimikage
Copy link
Contributor Author

kimikage commented Jul 4, 2021

I was considering the approach of writing a highlight.js plugin, but the plugin API does not work well with DOM manipulation.
For this reason, I think the approach proposed above of separating the REPL input and output is more elegant.

In this PR, I will postpone the ansi color support for @repl (comment out some code) and write documentation and tests.

@fredrikekre
Copy link
Member

Sounds good

src/Expanders.jl Outdated Show resolved Hide resolved
Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

.

src/Expanders.jl Outdated Show resolved Hide resolved
@mortenpi mortenpi modified the milestones: 0.28.0, 0.27.4 Jul 6, 2021
@mortenpi
Copy link
Member

mortenpi commented Jul 6, 2021

Thank you guys for working on this! Fredrik and I briefly discussed this on Slack how to proceed with this in terms of versions:

  • We'll make this opt-in 0.27.4, to get this out to the users faster (to be toggled by a new ansicolor keyword to HTML).
  • We'll enable this by default in 0.28.0. So we'll have a follow-up PR that changes the default value (either now, or closer to the release). This is to avoid a large behavior change in a patch release.

Otherwise, if Fredrik is happy with this, then from my side this just needs a CHANGELOG entry.

@fredrikekre fredrikekre marked this pull request as ready for review July 6, 2021 01:56
@fredrikekre
Copy link
Member

I pushed those changes and added docs + changelog.

- Adds `ansicolor::Bool=false` option to `Documenter.HTML` for
  enabling/disabling mapping of ANSI escape codes to HTML.
- Adds option to locally disable/enable color using the `ansicolor` keyword
  argument in the `@example`/`@repl` block.
- Add styles for ANSI colors to the default themes

Co-authored-by: kimikage <kimikage.ceo@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Format: HTML Related to the default HTML output Type: Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants