Skip to content
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

Fix links in buffer format help widget and brackets in documentation #3121

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

w-pearson
Copy link
Contributor

Description

The first change fixes a set of typos in the buffer format documentation; there were missing brackets on [[fixed]] and [[single]].

The second change fixes the in-app link - before, all said "Exhaustive documentation can be found in the online docs" with something that renders as a link but wasn't clickable (even right-clicking it didn't give the link destination). That required changing the text interaction flags, converting the help message to a QTextBrowser, and setting the openExternalLinks property.

I verified that other uses of links work properly via checking all of them found via git grep href:

  • CaptureContext: Uses QMessageBox which defers to the style's SH_MessageBox_TextInteractionFlags which in practice has at least Qt::LinksAccessibleByMouse:
  • AboutDialog: sets openExternalLinks to true on the version and contact labels in the ui file
  • AnalyticsConfirmDialog and AnalyticsPromptDialog: uses on_label_linkActivated as an automatic slot (as it needs custom handling for opening the documented analytics report)
  • SettingsDialog: uses on_analyticsDescribeLabel_linkActivated as an automatic slot (and only has an internal link for opening the documented analytics report)
  • CrashDialog: several different ones:
    • captureFilename works via on_captureFilename_linkActivated as an automatic slot (and opens a file in explorer)
    • reportText has openExternalLinks in the ui file
    • on_send_clicked uses RDDialog::information with an email address, which uses QMessageBox (see CaptureContext)
    • finishedText has openExternalLinks in the ui file
  • ExtensionManager: sets openExternalLinks to true on the URL and author labels in the ui file
  • TipsDialog: sets openExternalLinks to true on the tipUrlLabel label in the ui file
  • nv_counter_enumerator.cpp: sets openExternalLinks to true on the counterDescription label in PerformanceCounterSelection.ui

The buffer format help can be accessed via first opening the raw
buffer viewer (e.g. from the actions section on the texture viewer),
and then clicking the "?" under saved formats. It includes a nice
summary of the format syntax, but says that "Exhaustive documentation
can be found in the online docs." That was supposed to include a link
to https://renderdoc.org/docs/how/how_buffer_format.html, and in fact
it renders as if it were link, but clicking on it did nothing.

This has been fixed by adding `Qt::LinksAccessibleByMouse` and
`Qt::LinksAccessibleByKeyboard` to `textInteractionFlags`, and also
setting `openExternalLinks` to true (which requires use of a
`QTextBrowser` instead of a `QTextEdit`). I left the full list of flags
as `Qt::TextBrowserInteraction` does not include the
`TextSelectableByKeyboard` flag (which seems useful here and was
included before).

I verified that other uses of links work properly via checking all of
them found via `git grep href`:
* CaptureContext: Uses `QMessageBox` which defers to the style's `SH_MessageBox_TextInteractionFlags` which in practice has at least `Qt::LinksAccessibleByMouse`:
  * https://github.com/qt/qtbase/blob/v5.15.11-lts-lgpl/src/plugins/styles/mac/qmacstyle_mac.mm#L2850-L2852
  * https://github.com/qt/qtbase/blob/v5.15.11-lts-lgpl/src/widgets/styles/qcommonstyle.cpp#L5303-L5305
  * https://github.com/qt/qtbase/blob/v5.15.11-lts-lgpl/src/widgets/styles/qfusionstyle.cpp#L3702-L3703
* AboutDialog: sets `openExternalLinks` to true on the `version` and `contact` labels in the ui file
* AnalyticsConfirmDialog and AnalyticsPromptDialog: uses `on_label_linkActivated` as an automatic slot (as it needs custom handling for opening the documented analytics report)
* SettingsDialog: uses `on_analyticsDescribeLabel_linkActivated` as an automatic slot (and only has an internal link for opening the documented analytics report)
* CrashDialog: several different ones:
  * captureFilename works via `on_captureFilename_linkActivated` as an automatic slot (and opens a file in explorer)
  * reportText has `openExternalLinks` in the ui file
  * on_send_clicked uses `RDDialog::information` with an email address, which uses `QMessageBox` (see CaptureContext)
  * finishedText has `openExternalLinks` in the ui file
* ExtensionManager: sets `openExternalLinks` to true on the `URL` and `author` labels in the ui file
* TipsDialog: sets `openExternalLinks` to true on the `tipUrlLabel` label in the ui file
* nv_counter_enumerator.cpp: sets `openExternalLinks` to true on the `counterDescription` label in PerformanceCounterSelection.ui
@@ -162,13 +162,13 @@ The buffer format supports annotations on declarations to specify special proper
Struct definitions support the following annotations:

* ``[[size(number)]]`` or ``[[byte_size(number)]]`` - Forces the struct to be padded up to a given size even if the contents don't require it.
* ``[[single]]`` or ``[fixed]]`` - Forces the struct to be considered as a fixed SoA definition, even if in context the buffer viewer may default to AoS. See the below section for more details. Structs with this annotation **may not** be declared as a variable, and should instead be the implicit final struct in a definition.
* ``[[single]]`` or ``[[fixed]]`` - Forces the struct to be considered as a fixed SoA definition, even if in context the buffer viewer may default to AoS. See the below section for more details. Structs with this annotation **may not** be declared as a variable, and should instead be the implicit final struct in a definition.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be nice to copy over the ":ref:the below section<aos-soa that is used in the Variable section to replace the unlinked "the below section"

Copy link
Owner

Choose a reason for hiding this comment

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

I'd agree that this would be nice but it's a pre-existing issue not related to your fix, so if you'd rather I merge this as is then let me know and I will.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't actually have a build environment set up for the documentation locally, so I'd rather not make that change myself just in case there's some nuance to the formatting I'm not aware of. But that does seem like a good idea.

@baldurk baldurk merged commit e03d7c9 into baldurk:v1.x Nov 9, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants