Skip to content

Allow renderers to override _any_color_fmt. #2490

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

Merged
merged 19 commits into from
May 5, 2025

Conversation

asinghvi17
Copy link
Collaborator

@asinghvi17 asinghvi17 commented Apr 3, 2024

This function is used to decide whether to emit ANSI colored output to the output fields of @example blocks.

This has uses for DocumenterMarkdown as well as DocumenterVitepress, and will allow us to get rid of the current @ansi block formulation.

The PR simply changes the check in _any_color_fmt to calling a function writer_supports_ansicolor instead of a hardcoded type check; that function can then be extended.

This has uses for DocumenterMarkdown as well as DocumenterVitepress, and will allow us to get rid of the current `@ansi` block formulation.
@asinghvi17 asinghvi17 changed the title Make _any_color_fmt a method which can be overridden. Allow renderers to override _any_color_fmt. Apr 3, 2024
Copy link
Member

@goerz goerz left a comment

Choose a reason for hiding this comment

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

Seems okay to me. Of course, there is still the fact that this is 100% internal. If we had an official stable plugin API it would be worth thinking about whether a function like this would be part of that plugin API. If it was, the function would have to have a non-underscore name (format_supports_color?) and a docstring that describes exactly what that a writer promises when it claims format_supports_color as true.

@mortenpi
Copy link
Member

mortenpi commented Apr 4, 2024

I agree with Michael. I'm fine with this change as well, but in the current form, it would not have any guarantees of a public API. But I feel this could be turned into a public API as well, if we can articulate a precise docstring for it.

@asinghvi17
Copy link
Collaborator Author

Thanks for the feedback!

I would love to make this a public API - if so, it would probably go in Documenter.Writers, and perhaps the name supports_ansicolor would work? That would also remove the need for all writers to have an ansicolor field if they do support it.

Perhaps a docstring like:

"""
	supports_ansicolor(writer::Documenter.Writer)::Bool

Returns `true` if the writer `writer` accepts ANSI colored input from Documenter, or `false` if it does not. 

Defaults to `false`.

This is mainly used to color the output of `@example` and `@repl` blocks.
"""

@mortenpi
Copy link
Member

it would probably go in Documenter.Writers

We're moving away from the submodules. Documenter.writer_supports_ansicolor would probably be a good name.

Re the docstring: generally, looks fine. Though I think the main difference will be that the show and other IO methods will be called with an color=true IOContext?

@fingolfin
Copy link
Collaborator

@asinghvi17 are you still interested in working on this?

@asinghvi17 asinghvi17 requested review from mortenpi and goerz April 30, 2025 20:13
@asinghvi17
Copy link
Collaborator Author

Things should be working after this commit.

@fingolfin fingolfin closed this Apr 30, 2025
@fingolfin fingolfin reopened this Apr 30, 2025
Copy link
Collaborator Author

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Let's see if this works instead

Copy link
Collaborator Author

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Let's see if this works instead

@asinghvi17
Copy link
Collaborator Author

Maybe using HTML is messing something up here? But I have no clue what.

Copy link
Member

@goerz goerz left a comment

Choose a reason for hiding this comment

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

LGTM, apart from the trailing whitespace, which should be trivial to fix ;-)

@asinghvi17
Copy link
Collaborator Author

@lazarusA after this PR is released, we can deprecate the @ansi block in Vitepress

@asinghvi17 asinghvi17 requested a review from goerz May 2, 2025 15:33
Copy link
Member

@goerz goerz left a comment

Choose a reason for hiding this comment

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

Okay, this should be good now :-)

@mortenpi mortenpi enabled auto-merge (squash) May 5, 2025 03:41
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.

LGTM as well, thank you! I took the liberty of putting the new function into the manual as well, under the public API section.

@mortenpi mortenpi merged commit ced42e5 into JuliaDocs:master May 5, 2025
23 of 24 checks passed
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.

4 participants