This is an archive of the discontinued LLVM Phabricator instance.

[clang-format][docs] Add ability to link to specific config options
ClosedPublic

Authored by rymiel on Nov 21 2022, 9:28 AM.

Details

Summary

This allows for the creation of permalinks to specific clang-format
options, for better sharing of a specific option and its options.

(I'm adding the usual clang-format reviewers on this patch because
I don't know any other reviewers that well, perhaps someone with
docs experience should be added instead...)

Note that I wanted to make minimal changes to make this happen and thus
landed on an unideal setup, but to me, it seems like the best out of
worse ones.

I could have made every style option a subheading, which would add
automatically the logic for permalinks and the little paragraph icon for
sharing.

However, this meant that the links themselves would be suboptimal, as
they'd include the whole text of the heading, including the type and
versionbadge, which is needless noise and could change, breaking the
concept of a "permalink". The format of the page could be changed to
put the option names on their own in a heading, and the other info below
it in a paragraph.

As Sphinx seems unwilling to fix https://github.com/sphinx-doc/sphinx/issues/1961,
there isn't a succinct way to change the "id" html field used for
sections

I could have used an add-on (https://github.com/GeeTransit/sphinx-better-subsection),
or made one myself, but I wanted to avoid extra dependencies for no
reason. (plus, I don't know how to make one myself.)

I could have used raw HTML for each heading, but that would immensely
pollute the rst file, which, while it is generated, is currently still
human-readable and it'd be nice for it to stay that way.

Also note that sphinx treats references as case-insensitive, which means
that they will all be lowercased in the resulting HTML. I envisioned
the ability to simply add #OptionName after the URL to get placed right
at the desired config option, which isn't possible without things such
as inline raw HTML.

To reconcile that, I added the ¶ paragraph buttons that can be used to
generate the link to the desired section, but since headings are not
actually used, they are faked and literally just a link following each
option, which means they stylistically don't match all other headings.

Also note that this sort-of assumes HTML output. I know Sphinx can
output other formats but I do not know if they are used. A non-html
output could embed unusable ¶ signs everywhere.

I'm okay with this patch being rejected in its current solution, or if
any of the above listed alternatives are better, they could be pursued
instead. In case the downsides of this solution are too much, I will
just create a feature request issue for this and maybe let someone more
experienced with Sphinx handle it, since this is still a feature I would
like to have. (and I do not want to deal with Sphinx at all after
battling with it for a whole day to produce a mediocre result.)

Diff Detail

Event Timeline

rymiel created this revision.Nov 21 2022, 9:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 9:28 AM
rymiel requested review of this revision.Nov 21 2022, 9:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 9:28 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Full disclaimer: I have no idea what sphinx really is or how any of this stuff works, I still generate HTML from doxygen.

But I think the solution looks nice in code.

This revision is now accepted and ready to land.Nov 21 2022, 12:07 PM

Another point, when looking at that code: All options have the version badge, and any new option should have it too. Maybe the script should just fail if self.version is not true?

I consulted a little bit with a friend who has a little bit of experience with sphinx and they suggested making a simple custom extension for this. But given that there is no custom sphinx logic right now at all, I don't feel great adding something that novel to the whole of clang docs just for this page of clang-format

This comment was removed by rymiel.
owenpan accepted this revision.Dec 29 2022, 8:33 PM
aaron.ballman accepted this revision.Jan 12 2023, 6:14 AM
aaron.ballman added a subscriber: aaron.ballman.

I consulted a little bit with a friend who has a little bit of experience with sphinx and they suggested making a simple custom extension for this. But given that there is no custom sphinx logic right now at all, I don't feel great adding something that novel to the whole of clang docs just for this page of clang-format

Unless the custom extension is something that is committed to version control, fetched alongside the rest of the repo, and "just works" for folks without setup, I think it's best to avoid it. Otherwise we'll have to update the sphinx build bots, downstreams will have to react to it, and users will have a harder time building the docs locally. It's not that we can't do it if it's the right solution; it's that it's better to avoid something requiring manual intervention if we can.

As best I know, the way you've done things in this patch is the way Sphinx is expected to work for arbitrary anchors (despite being pretty janky IMO), so this LGTM!

MyDeveloperDay accepted this revision.Jan 12 2023, 7:03 AM

So if I understand completely its so its easier to pick up an option and share as a hyperlink i.e. <.....>/llvm-project/clang/html/ClangFormatStyleOptions.html#allowallconstructorinitializersonnextline

LGTM

So if I understand completely its so its easier to pick up an option and share as a hyperlink i.e. <.....>/llvm-project/clang/html/ClangFormatStyleOptions.html#allowallconstructorinitializersonnextline

LGTM

Yup, that's how I read this -- easier to share links to specific options (which is really neat, I wish the command line reference for Clang did the same thing!).