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

Add API context page #30115

Closed
wants to merge 4 commits into from
Closed

Add API context page #30115

wants to merge 4 commits into from

Conversation

utzig
Copy link
Member

@utzig utzig commented Nov 18, 2020

Updated breathe for xrefitem page support, add @allowed_from macro and update API functions to use it instead of @note to describe context requirements, and add new page to render the "Allowed From" contents.

Initial support for suggestions on #21061

@github-actions github-actions bot added area: API Changes to public APIs area: Documentation labels Nov 18, 2020
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Can you remove the changes you made to kernel.h? Let's begin with irq.h and see what it looks like, can you also post the rendered version?

@carlescufi
Copy link
Member

@utzig if you could, could you paste screenshots of what this looks like, on both the API doc and the new context page?

@pabigot
Copy link
Collaborator

pabigot commented Nov 30, 2020

I think this is missing the mark. A key driver of #18970 (which is what we're trying to solve) is to precisely describe what behavior can be expected when using a specific API. "Allowed contexts" doesn't cover the range of behaviors at all, and free-form text is verbose and potentially inconsistent.

What do we want the source code and the generated documentation to look like? As a first-attempt I'd like to see the doxygen markup look something like:

/** @brief summary
 * text
 * @param whatever
 * @retval whatever
 *
 * @apiattr isr-ok no-wait sleep
 */

where the behavior is defined using standard tags.

An objection to @apiattr has been something like "people won't understand it". I think that's on "people", and a goal of tooling support is to transform those those bare symbols to something that links to the tag definitions to help them understand what it means without massive boilerplate in the source.

If we could have the generated detailed documentation for each function contain those identifiers replaced by links to the terminology that might work. Also generating a separate page that produces a table with rows for each API function like #18970 (comment) would be very helpful too.

@utzig
Copy link
Member Author

utzig commented Nov 30, 2020

@utzig if you could, could you paste screenshots of what this looks like, on both the API doc and the new context page?

Sure, I'll do as a preview. Building today has one link direction broken and I am trying to find what broke it.

This is what an API documentation section looks like:

main_irq_lock

Mind that the "Allowed from:" title should be a link to the context page, which is what broke since the PR was first openened.

This is the context page (and link in the nav):

context_page

This was generated with "kernel.h" also included while I am debugging the issue mentioned before. Also there could be pages for the standard commands like "\deprecated", "\test", "\todo" and "\bug", if people decide to use those in source code. I have not added them here. This behavior pretty much only mimics what Doxygen already does on its html "pages" generation.

@utzig
Copy link
Member Author

utzig commented Nov 30, 2020

An objection to @apiattr has been something like "people won't understand it". I think that's on "people", and a goal of tooling support is to transform those those bare symbols to something that links to the tag definitions to help them understand what it means without massive boilerplate in the source.

If we could have the generated detailed documentation for each function contain those identifiers replaced by links to the terminology that might work. Also generating a separate page that produces a table with rows for each API function like #18970 (comment) would be very helpful too.

Yeah, I like the idea as well, I really lacked the creativity to come up with something that works well in both the Sphinx based generation and the Doxygen html generation, which was one of @nashif requirements initially. If it was Sphinx only, it would be fine to do it this way.

@pabigot
Copy link
Collaborator

pabigot commented Nov 30, 2020

Yeah, I like the idea as well, I really lacked the creativity to come up with something that works well in both the Sphinx based generation and the Doxygen html generation

I don't know the internals, but I don't think it's a big deal if this doxygen documentation were to use unlinked tag identifiers, as long as this documentation could have them replaced with links. Are there hooks that would allow that to be done?

@carlescufi
Copy link
Member

carlescufi commented Nov 30, 2020

@pabigot just to clarify, when you mention apiattr you are referring to what we describe in previous conversations as funcprops right? See #21061 (comment). Of course if we have a line per property we could rename it funcprop.

@pabigot
Copy link
Collaborator

pabigot commented Nov 30, 2020

@pabigot just to clarify, when you mention apiattr you are referring to what we describe in previous conversations as funcprops right?

I'd forgotten that branch of the discussion existed; something like that is fine. I just want something relatively terse using well-defined terms.

@pabigot
Copy link
Collaborator

pabigot commented Nov 30, 2020

Also, to be forward-looking: Even if we have a list of function properties, it's likely there will be cases where some additional text is required to explain specific behavior. E.g. no-wait may have to be explained to describe exactly what condition indicates that the no-wait path should be taken. That gets harder to do in doxygen as it really requires a multi-level markup: a funcprops description consists of a list of tags plus optional text that would ideally be full markup itself (potentially including lists and links and whatever), either describing relations between the tags or providing finer detail for a specific tag.

I don't know how that could be done, but it's something that has value.

This release adds support for xrefitem based pages.

Signed-off-by: Fabio Utzig <[email protected]>
@allowed_from should be used to document where APIs can be called from.

Signed-off-by: Fabio Utzig <[email protected]>
Change current @note commands with API context information to use
@allowed_from.

Signed-off-by: Fabio Utzig <[email protected]>
Add a page for rendering the contents of @allowed_from documentation.

Signed-off-by: Fabio Utzig <[email protected]>
@utzig
Copy link
Member Author

utzig commented Dec 1, 2020

@pabigot I am doing a bit of experimenting with Doxygen macros and trying to come up with a solution that is still based on the initial idea but that also implements your suggestions. One example would be adding aliases like this:

ALIASES += "apiattr=\par \"API attributes\" "
ALIASES += "reschedule=\htmlonly reschedule \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_reschedule`</verbatim> \endxmlonly"
ALIASES += "sleep=\htmlonly sleep \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_sleep`</verbatim> \endxmlonly"
ALIASES += "isr_ok=\htmlonly isr-ok \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_isr-ok`</verbatim> \endxmlonly"
ALIASES += "no_wait=\htmlonly no-wait \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_no-wait`</verbatim> \endxmlonly"
ALIASES += "pre_kernel_ok=\htmlonly pre-kernel-ok \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_pre-kernel-ok`</verbatim> \endxmlonly"
ALIASES += "async=\htmlonly async \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_async`</verbatim> \endxmlonly"
ALIASES += "supervisor=\htmlonly supervisor \endhtmlonly \xmlonly <verbatim>embed:rst:inline :ref:`api_term_supervisor`</verbatim> \endxmlonly"

This will force all those API terminology into "reserved words", meaning that the API attributes are not "free text", so they must be typed correctly and each attribute links back to its documentation in the API terminology page on the Sphinx pages, does not link on Doxygen yet. A rendering looks like this:

apiattr

It's still a PoC locally, have not updated commits, but I hope this moves the PR closer to what you have in mind.

@pabigot
Copy link
Collaborator

pabigot commented Dec 1, 2020

Would that in the header look like:

* @apiattr @isr_ok @reschedule

? I think that could be tolerable. If it's the bare word reschedule (or sleep) that gets replaced by a link that wouldn't work.

@carlescufi let's look at this in API today too.

@utzig
Copy link
Member Author

utzig commented Dec 1, 2020

Would that in the header look like:

* @apiattr @isr_ok @reschedule

Yes, exactly that.

@carlescufi
Copy link
Member

carlescufi commented Dec 1, 2020

API meeting 1st December 2020:

  • Remove the current allowed_from tag/section completely, only @funcprops will be used
  • Each function will have this structure in its body:
/** @brief summary
 * text
 * @funcprops \isr-ok \no-wait \sleep (note: in a consistent order)
 *
 * @param whatever
 * @retval whatever
 */

Each of the function properties is a link to the corresponding section in terminology page. That page in its turn will link to a list of functions (cross-reference) that contain the same property. Aside from this we need to generate a table described in this comment.

@utzig when you prototype this, please paste the result of rendering a sample function in both Breathe and Doxygen.

@carlescufi carlescufi requested a review from ru-fu January 12, 2021 11:24
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Mar 22, 2021
@github-actions github-actions bot closed this Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants