Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 964
Avoid over-linking #1293#1294
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nedbat commented Mar 20, 2024 • edited by github-actions bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by github-actions bot
Uh oh!
There was an error while loading. Please reload this page.
willingc left a comment
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!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
5df7988 to 8fa2d3aCompare4ac3e45 to ba8530bComparenedbat commented Mar 20, 2024
Changes made, and I added another rule: no links in section titles. |
fac2e66 to 0a5704bCompare0a5704b to 60426a1CompareUh oh!
There was an error while loading. Please reload this page.
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, shortis not too clear, sinceshortdoesn'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 partto mirror the text ofroles w/ only last part - reordering the entries so that
roles w/o linkcomes first,roles w/ only last partnext, androles w/o link, only last partjust after, so that the use of!visibleis 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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
hugovk commented Mar 27, 2024
Thanks! |
willingc commented Mar 27, 2024
Thanks @nedbat |
Summarizing and codifying the advice discussed in python/docs-community#52
📚 Documentation preview 📚: https://cpython-devguide--1294.org.readthedocs.build/