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

Document the return value of AccessDialog() #1114

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Document the return value of AccessDialog() #1114

merged 1 commit into from
Oct 27, 2023

Conversation

WhyNotHugo
Copy link
Contributor

Closes: #1041

@@ -37,7 +37,8 @@
@subtitle: Subtitle for the dialog
@body: Body text, may be ""
@options: Vardict with optional further information
@response: Numeric response
@response: Numeric response. A value of 0 indicates that the user has
agreed to grant permission. Any other value is treated as a refusal.
Copy link
Contributor

Choose a reason for hiding this comment

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

(almost) all the other impl.portal just state Numeric response too - it'd be nice to update all those, and ideally to something like: "The value returned as the response argument in the #org.freedesktop.portal.Request::Response for the corresponding Request". This way you get the automatic link to where the documentation for the return values is.

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'm not sure I understand. What is "the #org.freedesktop.portal.Request::Response for the corresponding Request" in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, indeed, this is one (of the few?) that doesn't have an immediately corresponding request in the client API. I think in that case something like "The value range of the response matches the values allowed for #org.freedesktop.portal.Request::Response" for this one. Still would be nice to update the others though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@WhyNotHugo
Copy link
Contributor Author

It's not rendering correctly; I'm not entirely sure why.

@WhyNotHugo
Copy link
Contributor Author

Looks like the #org.freedesktop.portal.Request::Response doesn't actually add a link tho.

I'm not entirely sure what format this is, so I'm not sure where to look for its documentation. How do I add an actual link to the anchor?

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Oct 24, 2023

Oh, there's an example right above it 🤦

I tried this:

diff --git a/data/org.freedesktop.impl.portal.Access.xml b/data/org.freedesktop.impl.portal.Access.xml
index 29a9f70..5759e02 100644
--- a/data/org.freedesktop.impl.portal.Access.xml
+++ b/data/org.freedesktop.impl.portal.Access.xml
@@ -37,7 +37,7 @@
         @subtitle: Subtitle for the dialog
         @body: Body text, may be ""
         @options: Vardict with optional further information
-        @response: Numeric response. The values allowed match the values allowed for #org.freedesktop.portal.Request::Response signal.
+        @response: Numeric response. The values allowed match the values allowed for <link linkend="#gdbus-signal-org-freedesktop-portal-Request.Response">#org.freedesktop.portal.Request::Response</link> signal.
         @results: Vardict with the results of the call
 
         Presents a "deny/grant" question to the user.

But it fails to build:

ninja: job failed: /usr/bin/xmlto xhtml-nochunks -o doc -m ../doc/xmlto-config.xsl --searchpath /home/hugo/src/github.com/flatpak/xdg-desktop-portal/build/src doc/portal-docs.xml
xmlto: /home/hugo/src/github.com/flatpak/xdg-desktop-portal/build/doc/portal-docs.xml does not validate (status 3)
xmlto: Fix document syntax or use --skip-validation option
/home/hugo/src/github.com/flatpak/xdg-desktop-portal/build/doc/portal-docs.xml:133: element link: validity error : Syntax of value for attribute linkend of link is not valid
/home/hugo/src/github.com/flatpak/xdg-desktop-portal/build/doc/portal-docs.xml:133: element link: validity error : IDREF attribute linkend references an unknown ID "#gdbus-signal-org-freedesktop-portal-Request.Response"
Document /home/hugo/src/github.com/flatpak/xdg-desktop-portal/build/doc/portal-docs.xml does not validate
ninja: subcommand failed

I'm not sure how to work around this. Any insight is welcome.

@GeorgesStavracas
Copy link
Member

We're reworking the docs system on #1157 but this is good to have already

@GeorgesStavracas GeorgesStavracas added this pull request to the merge queue Oct 27, 2023
Merged via the queue into flatpak:main with commit d8fd8cc Oct 27, 2023
4 checks passed
@WhyNotHugo WhyNotHugo deleted the access-portal-docs branch October 27, 2023 20:55
@WhyNotHugo
Copy link
Contributor Author

Sphinx sound like a great choice 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Triaged
Status: Done
Development

Successfully merging this pull request may close these issues.

Return value for org.freedesktop.impl.portal.Access.AccessDialog
3 participants