-
-
Notifications
You must be signed in to change notification settings - Fork 806
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
Avoid over-linking #1293 #1294
Avoid over-linking #1293 #1294
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Ned!
5df7988
to
8fa2d3a
Compare
4ac3e45
to
ba8530b
Compare
Changes made, and I added another rule: no links in section titles. |
fac2e66
to
0a5704b
Compare
0a5704b
to
60426a1
Compare
documentation/markup.rst
Outdated
``:role:`~!hidden.visible``` makes more semantic sense, but it causes | ||
a warning as Sphinx tries to look up the reference ``!hidden.visible`` | ||
which does not exist. The shorter form ``:role:`!visible`` renders as | ||
desired and will build successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this should be included here. The quick reference lists commonly used markup, and what's being described here is not very common.
I would move this paragraph in the linked "roles" section, where the nuances of using !
and ~
are covered in more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took a few developers a while to work out what happened when trying to combine ~!
, so I think it will be helpful to leave here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm thinking is:
- the quick reference table is meant mostly for readers that already know about the existence and functioning of those constructs, but don't quite remember the syntax
- all further details and explanations shouldn't (and aren't) covered here, but in the sections linked in the third column
- if users look at the "roles" section (where
~
and!
are explained) for ways to combine~
/!
they won't find it there, unless you duplicate the note there - the wording of
roles w/o link, short
is not too clear, sinceshort
doesn't mirroronly last part
(understandably, due to lack of space) - a reader might be wondering how to combine
~
and!
together, but:role:`!visible`
doesn't seem an obvious answer to the question (unless they keep scrolling and read the note), especially since it doesn't have any apparent difference from:role:`!target`
- the consequences of combining
~
and!
are a Sphinx implementation detail that shouldn't matter to the reader, so I would omit them from the note
My suggestion is to add:
* Combining ``~`` and ``!`` (e.g. ``:meth:`~!Queue.Queue.get```) is not supported.
You can obtain the same result by simply using `!` and the last component of
the target (e.g. ``:meth:`!get```).
in the "roles" section, after the bullet points that already explain ~
and !
and use Queue.Queue.get
as an example.
The entry in the table could be removed altogether, since it's not a very common construct (trusting that the reader will follow the link looking for more information). If you prefer to keep it, the table should be updated by:
- widening the first column and using
roles w/o link, only last part
to mirror the text ofroles w/ only last part
- reordering the entries so that
roles w/o link
comes first,roles w/ only last part
next, androles w/o link, only last part
just after, so that the use of!visible
is more evident.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nedbat Let me take some time to read @ezio-melotti's comment. Ezio makes some good points especially considering the original intent of the chart. There needs to be a new doc contributor cheat sheet (that is useful to current contributors) to guide folks on many of these markup constructs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ezio-melotti I like your suggestion, and have made it in the latest commit.
Co-authored-by: Hugo van Kemenade <[email protected]>
Thanks! |
Thanks @nedbat |
Summarizing and codifying the advice discussed in python/docs-community#52
📚 Documentation preview 📚: https://cpython-devguide--1294.org.readthedocs.build/