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

line numbers could use better visual distinction #26

Closed
2bndy5 opened this issue Nov 22, 2021 · 10 comments · Fixed by #45
Closed

line numbers could use better visual distinction #26

2bndy5 opened this issue Nov 22, 2021 · 10 comments · Fixed by #45

Comments

@2bndy5
Copy link
Collaborator

2bndy5 commented Nov 22, 2021

At first, this felt like a personal style choice, but I've been adding custom CSS to all my docs that use this theme.

.linenos {
  background-color: var(--md-default-bg-color--light);
  margin-right: 0.5rem;
}

which transforms
image
into something a little more "legible" like so
image


I was wondering if this CSS change should be considered for this theme. Additionally, I kinda like how line numbers look in the mkdocs-material upstream
image

@jbms
Copy link
Owner

jbms commented Nov 23, 2021

I agree that the current version is not good, and that the mkdocs-material styling is better.

I don't use line numbers in the tensorstore documentation so I didn't notice the issue.

Ideally we can avoid modifying the css at all, since mkdocs-material may already have some suitable rules, and just modify the HTML output from sphinx to work with it to produce the desired line number styling.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Nov 23, 2021

code-blocks in mkdocs-material are a stylized table in which the line numbers are a separate column and the literal code is another column. My first impression is that the code-block directive would have to overridden to produce this table structure. I don't think it would be simple matter of adding mkdocs-material classes to the html elements since the SCSS specifically looks for td and tr within an element using highlightable class attribute.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Feb 25, 2022

I just noticed the furo theme renders code blocks' line numbers like so
image
using

.highlight span.linenos {
  box-shadow: -.0625rem 0 var(--color-foreground-border) inset;
  display: inline-block;
  margin-right: .875rem;
}

This closely resembles mkdocs-material's code-blocks look, but the furo theme doesn't support changes in font-size (deeming such changes as "unstable").

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 1, 2022

Today, I had an idea to override the code-block directive with a theme specific implementation. That way we don't have to keep deviating from upstream CSS and simulate the HTML output that the CSS is expecting. Objections/thoughts?

@jbms
Copy link
Owner

jbms commented Mar 1, 2022

Yeah that could be reasonable. We are already overriding the HTML rendering of literal in inlinesyntaxhighlight.py --- we could similarly override the HTML rendering of literal_block docutils nodes, which is what the code-block directive generates (see the visit_literal_block method in sphinx/writers/html{,5}.py for the existing implementation in sphinx).

The only question would be whether there are other sphinx extensions that may be relevant that will break if we switch to a table layout, and if so, whether it is worth it.

You could see what code is used in mkdocs to render the code blocks with line numbers --- ultimately mkdocs uses the same pygments package to highlight code.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 1, 2022

whether it is worth it.

I forgot to mention that overriding the code-block directive could also offer users a way to implement adding code annotations. Although, I doubt a majority of sphinx users would take advantage of such an option. It would mostly be utilized by users migrating from mkdocs.

This is all speculative as I've clearly not researched the idea yet. (Thanks for the leads).

With all this in mind, I'm starting to lean toward an independent sphinx extension.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 15, 2022

I just noticed the "copy to clipboard" button copies the entire contents of the literal block including line numbers when apparent.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 15, 2022

Geez! This simple addition to conf.py fixed this issue entirely:

html_codeblock_linenos_style = "table"

This seems to use the upstream CSS for line numbers, and the "copy to clipboard" button only copies the literal block's content (not the line numbers).

Unfortunately, this config option has been deprecated as of sphinx v4.0 (according to the sphinx docs). I would propose that this theme automatically asserts this config option. When the option is completely removed from sphinx, we can look at overriding the visit_literal_block() to restore the functionality.

@jbms
Copy link
Owner

jbms commented Mar 15, 2022

I think it might be possible to have the theme set this option automatically by registering a handler for theconfig-inited event.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 15, 2022

Yeah, that's what I did to test my proposal, and it worked like a charm.

2bndy5 added a commit to 2bndy5/sphinx-immaterial that referenced this issue Mar 18, 2022
@jbms jbms closed this as completed in #45 Mar 20, 2022
jbms pushed a commit that referenced this issue Mar 20, 2022
* minor fixups: spelling, pylint, & pkg.lock

* fix #26 (till sphinx v6.0)

* adding directives

Docs require sphinxcontrib-details-directive, and use it instead of sphinx-design dropdown directive. This also reverts the currently erroneous fix to #15.

Add `md-tab-set` & `md-tab-item` directives. Update docs with new page on how to use these new directives.

* monkey patch details directive

* pleasing pylint

* ran black on details_patch.py

* [no ci] remove dev artifact from docs

* introducing mermaid implementation

- updates docs about the details directive
- adds page that demonstrates how to use the md-mermaid directive.

* avoid unknown nodes.raw error revert conf.py a bit

* fix mermaid directive for pseudo latex output

* pylance's auto-import is annoying for typos

* fix html/latex output for mermaid diagrams

* cleaning it up a bit

* auto add directives that are tied to our CSS

* docs proofreading

* some simple review changes

* fix doc typos

* visit_tab_set is the transformer

* remove unused import

* use better pseudo latex output for mermaid graphs

* append input elements literally; use len(children)

* remove setting useless `set_id` attribute

* use SkipNode after rendering a tab set's divs

* remove artifact class

* 3rd round review changes
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 a pull request may close this issue.

2 participants