Add a Sphinx role to link to GitHub files

[ezio-melotti]

This PR adds a role to link to GitHub files (in the python/cpython repo) and uses them in internals/parser.rst.

• Fixes Add links to files in internals/parser.rst #960

[ezio-melotti]

Using gh-file will make it more symmetric with gh-label, but since the two are not used together, I thought using cpy-file would make the fact that they are linking to the cpython repo more explicit.

[ezio-melotti]

After creating the link, I (manually) found out that it was supposed to be lowercase.

Upon further testing, I found out that these are already integrated with linkcheck:

(internals/parser: line  520) broken    https://github.com/python/cpython/blob/main/Grammar/Python.gram - 
404 Client Error: Not Found for url: https://github.com/python/cpython/blob/main/Grammar/Python.gram

[hugovk]

Looks good!

One idea, filenames are often written with a fixed-width font (e.g. in this style guide), shall we have the role do that too?

And document this role at https://cpython-devguide--961.org.readthedocs.build/documentation/markup.html#roles ?

(We should add gh-label there too.)

[CAM-Gerlach]

This is very helpful; we could add this to the PEPs too.

I do agree with @hugovk that this really should be literal-formatted like the original :file: role, though I'm not entirely sure how to achieve that without just re-implementing the link manually.

[ezio-melotti]

One idea, filenames are often written with a fixed-width font (e.g. in this style guide), shall we have the role do that too?

It took me a bit but eventually (and with the help of @AA-Turner and @CAM-Gerlach) I figured out how to make it a literal, in a way that is simple and concise enough.

While I was at it, I also added support for ! so that we can do e.g. :cpy-file:`!Makefile` without creating a link (since Makefile is generated, and not in the actual repo).

I also thought about adding support for variables parts like :cpy-file:`somedir/python3.{x}/somefile.py`, but this seems more involved and IMHO not worth it. If there is a variable part, the file can't be linked anyway and in that case a regular :file: can be used (even if it's slightly less semantic in the source, but otherwise unnoticeable since they render the same).

Support for ~ (to strip the dirs and leave the filename) seems YAGNI to me, but it's not too difficult to add if we really wanted to.

This is how the output of the current PR looks like:

And document this role at https://cpython-devguide--961.org.readthedocs.build/documentation/markup.html#roles ?

Those roles are just for the cpython repo so I don't think this should be documented. If we end up adding it to python/cpython (and python/peps too), then it can be documented. (Doing that would also mean duplicating (or triplicating) the code, so we might want to come up with a better solution.)

Note that cpython already defines a :source: role (which is also not documented).

Click to see some related findings

Sphinx defines a bunch of extra nodes, including a literal_emphasis, but it doesn't support variables parts and just makes everything italic. I haven't seen a node that already does what we need:
https://github.com/sphinx-doc/sphinx/blob/6ed4bbba396eb410c4d00e5f9881aa621cec749a/sphinx/addnodes.py#L512

A new node type that combines literal and reference could be created and used as well:
https://github.com/docutils/docutils/blob/c5a6ec93fe81fc44851f65aff37bef8a4cd2d173/docutils/docutils/nodes.py#L1893-L1894

Sphinx also defines a EmphasizedLiteral role (which is what is used for the :file: role):
https://github.com/sphinx-doc/sphinx/blob/6ed4bbba396eb410c4d00e5f9881aa621cec749a/sphinx/roles.py#L272

If we wanted to handle this at the role level (rather than the node level) we could subclassed that and enhance it to support linking. This shouldn't be too difficult, but it's not entirely obvious to me how to do it and I'm happy with the current solution.

[CAM-Gerlach]

Ah, I didn't realize autolink was a custom role defined for this repo (I'd seen it mentioned being used on the NumPy repo and elsewhere); my assumptions and a few comments I'd seen when googling had led me to believe otherwise, and we'd have to re-implement it ourselves (in which case I went with the class approach), but don't know why I didn't double check.

In any case, yeah, its simple to add that given it's already defined here, especially for a first implementation. As for

If we end up adding it to python/cpython (and python/peps too), then it can be documented. (Doing that would also mean duplicating (or triplicating) the code, so we might want to come up with a better solution.)

I've drafted (but not yet tested, so it likely needs some debugging) a common base class that provides a superset of the existing functionality, including everything here plus customizable branches/tags/commits, ~ support, custom link titles (which are automatically not rendered as a literal), emulating the CSS classes of the file role, and some other things, while being fully extensible to files on any GitHub project, or indeed any arbitrary link.

It needs some more refactoring (to move the default org/repo to config settings rather than a custom subclass, and provide a factory function to generate new ones), a couple more features (support for fragments and validation so it works with PEPs/RFCs, support for GitHub issues/PRs), and a few tweaks (using @ instead of : for branches/tags/commits, adding URL escaping). However, it could form a common base class (and subclasses) implemented in a Sphinx extension to provide many of the similar, currently-custom roles used/desired across our repos (GitHub issue/PR, BPO issue, PEPs, and RFCs, alongside source files, which could all specify other repos or even orgs if needed), with a superset of the existing functionality, to replace the patchwork of repo-bespoke code. Going forward, building off this would likely make more sense than copying things around piecemeal between repos, and would be easily reusable for other projects as well.

If we wanted to handle this at the role level (rather than the node level) we could subclassed that and enhance it to support linking. This shouldn't be too difficult, but it's not entirely obvious to me how to do it and I'm happy with the current solution.

That's quite possible, but would require basically re-implementing the role, which is really just the same as :samp: with an different CSS class, as it is basically one big monolithic function and almost all the logic is devoted to parsing the sample bits, which aren't useful for a concrete specific file (that this role represents) as opposed to an abstract file that the file role corresponds to. Your approach here is far more reasonable, especially as a first implementation.

Longer-term, the above proposal features a common base class with many small, overridable methods, which can be easily subclassed used for other types of links and custom behavior, which avoids having to reimplement everything.

[CAM-Gerlach]

A few minor comments; otherwise LGTM for now 👍

[hugovk]

Ah, I didn't realize autolink was a custom role defined for this repo (I'd seen it mentioned being used on the NumPy repo and elsewhere);

Yeah, it's one of those handy bits of sample code that gets copied and pasted around a lot!

[AA-Turner]

Docutils things:

A

[CAM-Gerlach]

Seems to work well overall now. While more features can be added in the future, or it can be replaced with a more powerful, flexible and extensible alternative, IMO the main limitation here that could be worth addressing now is that there is currently no support for explicit titles/overriding the default title; i.e. :cpy-file:Python's PEG generator <Tools/peg_generator/>` displays:

Python's PEG generator <Tools/peg_generator/>

But I'm not sure how easy that would be to add aside from manual parsing logic (my longer-term approach relies on the ReferenceRole superclass to handle that), so I'm not sure if its worth it now. @AA-Turner any easy way to add this?

[ezio-melotti]

I'm going to merge this and follow up with another PR to use it with other files. We can then improve on it with other PRs and possibly port it to cpython and the peps repos.

[CAM-Gerlach]

SGTM, thanks @ezio-melotti

[ezio-melotti]

I created a new PR to use the new role throughout the Devguide:

• Use :cpy-file: throughout the Devguide #984