Skip to content

doc: Update CSS for readable @retval tables#13047

Merged
aabadie merged 1 commit intoRIOT-OS:masterfrom
maribu:doc-css-retval
Jan 9, 2020
Merged

doc: Update CSS for readable @retval tables#13047
aabadie merged 1 commit intoRIOT-OS:masterfrom
maribu:doc-css-retval

Conversation

@maribu
Copy link
Member

@maribu maribu commented Jan 8, 2020

Contribution description

Currently now margin between the return value and its description are added in return value tables generated with the @retval command. This adds a 2em margin, which is consistent with the margin after parameter names in the parameter table.

Testing procedure

E.g. the documentation of the dht_init() function should be more readable with this.

Issues/PRs references

Noticed in #13046

Currently now margin between the return value and its description are added in
return value tables generated with the @RetVal command. This adds a 2em margin,
which is consistent with the margin after parameter names in the parameter
table.
@maribu maribu added Area: doc Area: Documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Jan 8, 2020
@maribu maribu requested a review from miri64 January 8, 2020 14:24
@miri64
Copy link
Member

miri64 commented Jan 9, 2020

Are you sure you mean dht_init()? It doesn't use @retval. However, dht_read() looks weird (and uses @retval). Pretty sure though, that this parsing error is not fixed with your proposal.

@miri64
Copy link
Member

miri64 commented Jan 9, 2020

Compare

my view of the link you provided

* @return 0 on success
* @return -1 on error

and

RIOT/drivers/include/dht.h

Lines 110 to 113 in 87d00ab

* @retval `DHT_OK` Success
* @retval `DHT_NOCSUM` Checksum error
* @retval `DHT_TIMEOUT` Reading data timed out (check wires!)
* @retval `DHT_NODEV` Unsupported device type specified

(my solution to "fix" this issue would be to remove the ` around the values, which (I believe) would also create references to their respective doc)

@maribu
Copy link
Member Author

maribu commented Jan 9, 2020

I meant indeed dht_init() and the missing space between e.g. "-1" and "on error".

The broken output for dht_read() (which by the way is a bug in @doxygen), is not intended to be fixed by this PR.

@miri64
Copy link
Member

miri64 commented Jan 9, 2020

I meant indeed dht_init() and the missing space between e.g. "-1" and "on error".

Then I'm still not sure what you are trying to fix here. The doc for the dht_init() return values (as with any @return-based doc) is based on a description list <dl> of class .return not as your fix implies on a table with <td>-class .retval.
image

@maribu
Copy link
Member Author

maribu commented Jan 9, 2020

I see. On current version of Doxygen it generates separate table fields for key and values. Can you build the doc locally and check again?

A quick look at the HTML generated by the historic version of Doxygen seems to indicate that this would not introduce an issue there, right? But it would fix one with the current version.

@maribu
Copy link
Member Author

maribu commented Jan 9, 2020

I reported the bug of incorrectly handling markdown in @retval upstream: doxygen/doxygen#7499

@maribu
Copy link
Member Author

maribu commented Jan 9, 2020

I see. On current version of Doxygen it generates separate table fields for key and values. Can you build the doc locally and check again?

No, it is not with the version. In dht_init() the table is generated from @return commands, not by @retval commands. The space is also missing for @retval commands in the historic version (as seen in the dht_read()). The bug of incorrectly generated code highlight there is just more ugly.

@maribu
Copy link
Member Author

maribu commented Jan 9, 2020

With #13063 applied it becomes more obvious that the margin between the table columns are currently missing.

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Ok, now I understand!

ACK, together with #13063 this results in the following—much nicer—output

image

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 9, 2020
@aabadie aabadie merged commit c310bfb into RIOT-OS:master Jan 9, 2020
@maribu maribu deleted the doc-css-retval branch January 9, 2020 21:28
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants