Skip to content

Link rcParams role to docs #11448

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 1 commit into from
Jun 21, 2018
Merged

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jun 16, 2018

PR Summary

Follow up to #10226. This turns the text generated by the :rc: role into a link to the rc documentation.

Note: I'm no expert in docutils and it's really difficult to find good documentation. There generation of the refuri feels a bit clumsy, because I have to build the final url. Naturally, one would like to have some function label_to_uri('matplotlib-rcparams') but I couldn't find something like that. The downside is that we rely on a hard coded file structure, but I can live with that (We're not likely to change that often).

@anntzer
Copy link
Contributor

anntzer commented Jun 17, 2018

Would setting refname instead of refuri (set it to whatever you'd use in rst to generate the link, minus the final underscore) work? (I think that may be handled at the transform stage, http://docutils.sourceforge.net/docs/dev/hacking.html#transforming-the-document)

@anntzer
Copy link
Contributor

anntzer commented Jun 17, 2018

Minor point, possibly just an issue to open with sphinx/docutils (although we may get away with applying our own styling, but I'd rather get the issue fixed upstream): https://10444-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/api/_as_gen/matplotlib.axes.Axes.errorbar.html#matplotlib.axes.Axes.errorbar (see errorbar.capsize rc) uses curly quotes for the literal+link, whereas the previous version at https://matplotlib.org/devdocs/api/_as_gen/matplotlib.axes.Axes.errorbar.html#matplotlib.axes.Axes.errorbar uses straight quotes; the latter seems better.
Or perhaps this is just a matter of putting the literal in the reference rather than the other way round?

@timhoffm
Copy link
Member Author

timhoffm commented Jun 17, 2018

Would setting refname instead of refuri work?

Only partially. It works for files in api/_as_gen. For files directly below api I get WARNING: Unknown target name: "matplotlib-rcparams" and the role appears literally in the html as a non-working link:

grafik

@timhoffm
Copy link
Member Author

Curly quotes:

They are replaced by nodes.reference no matter of the nesting order with nodes.literal. Also, if you put a literal node into a reference node, the iteral formatting is not applied.

For now, the present solution is as good as I can get this (an alternative would only be a raw node).

@@ -23,6 +23,14 @@ a {
text-decoration: none;
}

a.rcparams {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is is really worth it using a different styling (or even a different class)?
I think the normal orange would probably actually look better.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we want to use styling, a class is the cleanest way.

To me, the original orange looked a bit too distracting. Can add a screenshot later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for comparison. I still prefer the darker links for the rcParams. But I'm not dogmatic about this if the majority has another opinion.

Same color as regular links

grafik

Darker links

grafik

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's see what others think, then.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 opinions welcome

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why it should be distinct. Every other link in the docs, including normal url links, are the same orange.

'tutorials/introductory/customizing.html#matplotlib-rcparams')

ref = nodes.reference(rawtext, rendered, refuri=refuri,
classes=['rcparams'])
Copy link
Contributor

Choose a reason for hiding this comment

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

would perhaps not bother with the separate class

Copy link
Member Author

@timhoffm timhoffm Jun 18, 2018

Choose a reason for hiding this comment

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

Doesn't harm. And makes it easier to adapt the style of the link if desired now or later (e.g. it would also be possible not to have a literal node nested and instead style the font-family of the link).

Copy link
Contributor

Choose a reason for hiding this comment

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

(that comment basically just applies if we decide not to style rcs differently from normal links)

rel_source = inliner.document.attributes['source'].split('/doc/', 1)[1]
levels = rel_source.count('/')
refuri = ('../' * levels +
'tutorials/introductory/customizing.html#matplotlib-rcparams')
Copy link
Contributor

Choose a reason for hiding this comment

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

use an absolute link? ("/tutorials/...")

Copy link
Member Author

Choose a reason for hiding this comment

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

when trying this locally, sphinx links to file:///tutorials/introductory/customizing.html#matplotlib-rcparams

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps set refid instead of refuri? (just throwing some ideas around -- but somewhere docutils does have the ability to do this, so we just need to find the right way to trigger it...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. refid='matplotlib-rcparams' just links to the local anchor #matplotlib-rcparams.

I don't have the time to scan through the docutils docs and try out various stuff. I've posted the question to stackoverflow. If there is no hint from there, I'd leave it at that.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I don't think this needs to be resolved to merge this PR -- it's quite unimportant. The only remaining point for me is about the styling.)

@timhoffm
Copy link
Member Author

Removed the css class and special highlighting for the link. It now appears as a regular link.

@timhoffm
Copy link
Member Author

Concerning curly quotes: This is controlled via the smartquotes option.

Maybe, nodes.reference() applies them, because it does not know that it will be wrapped in nodes.literal(). I don't know if there is a way to locally deactivate this.

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

Some possible further improvements, but that's clearly a step in the right direction and can be merged as is.

@jklymak jklymak merged commit 0baeca9 into matplotlib:master Jun 21, 2018
@timhoffm timhoffm deleted the rcparams-link branch June 22, 2018 00:26
@QuLogic QuLogic added this to the v3.0.0 milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants