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

Inconsistencies on use of separating space before an [ or { in documentation and examples #10823

Closed
maettu-this opened this issue Apr 24, 2024 · 11 comments
Labels
bug documentation bug in the documentation

Comments

@maettu-this
Copy link

Describe the bug

Inconsistent:

Consistent:

Screenshots
n/a, see links.

To Reproduce
n/a, see links.

Expected behavior
Consistent documentation and examples, giving users consistent guidance on how Doxygen syntax is intended to be used.

Version
n/a, see links.

Stack trace
n/a, see links.

Additional context
n/a, see links.

Please let me know the Doxygen preference and I'll supply a PR for this.

@albert-github albert-github added bug documentation bug in the documentation labels Apr 24, 2024
@albert-github
Copy link
Collaborator

albert-github commented Apr 24, 2024

There are some different meanings regarding the [ parts:

  • the [...] indicates an optional part
  • the '['...']' indicates a part that has mandatory square brackets

Regarding the inconsistent behaviors:

  • \param here it is optional to have a blank space between the \param and the direction, both versions (i.e. with and without blank space) are possible. This is due to some historical reasons. So both description and example are right although adding a space in the example doesn't hurt.
  • \htmlonly / \htmlinclude This doesn't look 100% right as here we have the possibility \htmlonly[block] but \htmlonly [block] will give an error. So this might probably be better formulated as \htmlonly['[block]']

albert-github added a commit to albert-github/doxygen that referenced this issue Apr 24, 2024
… an [ or { in documentation and examples

Making `\param`, `\htmlonly` and `\htmlinclude`  bit more consistent
@albert-github
Copy link
Collaborator

I've just pushed a proposed patch, pull request #10824

@maettu-this
Copy link
Author

@albert-github, thanks for taking care of this. I fully understand the difference in [...] vs. '['...']'. I also understand the historic flexibility to have param support both with or without space.

However, looking at the proposed #10824 I question adding the separating space after param, because:

  • All '{'...'}' options don't have a separating space.
  • \htmlonly['[block]'] doesn't seem to allow a separating space.
  • Opinion Dimitri in yesterday's email that triggered filing this issue:

I think this space does indeed not need to be there. I'll see if I can fix this. If you want to do it yourself via a pull request even better ;-)

@albert-github
Copy link
Collaborator

I just made the documentation consistent with the current code. but it is possible to make the \param documentation without the space and just mention the space just as a note.
The space doesn't need to be there but I've seen that in the past the version with the space was the standard. I used this also in the past in a number of projects.

@albert-github
Copy link
Collaborator

I just checked the code base and the syntax with the possible space is present since release 1.3.7 (May 7, 2004), before the direction was not supported.

@doxygen
Copy link
Owner

doxygen commented Apr 25, 2024

@albert-github The space between @param and the direction is indeed optional, but as I understood, the confusion stems from the example in that section of the documentation, where the space was omitted (which I also consider the preferred notation), so in that sense it is better to make the header consistent with the original example and omit the spaces rather than add them.

@maettu-this
Copy link
Author

I have quickly performed a search accross Doxygen's code base. Here is what I have found:

with space: 2 occurrences

  • \src\doctokenizer.l { /* param [in,out] command */
  • \section cmdparam \\param '['dir']' <parameter-name> { parameter description }

without space: 167 occurrences

  • param[in]: 134 occurrences
  • param[in,out]: 17 occurrences
  • param[out]: 16 occurrences

It's pretty clear that Doxygen's standard is without a space, but the parser is flexible enough to also accept a space.

@albert-github
Copy link
Collaborator

I cannot agree with the conclusion:

It's pretty clear that Doxygen's standard is without a space, but the parser is flexible enough to also accept a space.

Doxygen has no problem with either the versions with and without spaces, also seen the rule:

PARAMIO   {CMD}param{BLANK}*"["{BLANK}*{INOUT}{BLANK}*"]"

as used in doctokenizer.l.

The /* param [in,out] command */ is just an internal comment to say that here the directions of the param command are handled.

From the "167 occurrences" it is to me only clear that doxygen prefers for its documentation the version without the space.

So probably better to remove the spape in the docu and make the fact that the space is possible just a note.

(I didn't read the comment from @doxygen, #10823 (comment) before writing this comment. I missed it)

albert-github added a commit to albert-github/doxygen that referenced this issue Apr 26, 2024
… an [ or { in documentation and examples

After review
doxygen added a commit that referenced this issue Apr 28, 2024
issue #10823 Inconsistencies on use of separating space before an [ or { in documentation and examples
@albert-github albert-github added the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Apr 28, 2024
@albert-github
Copy link
Collaborator

Code has been integrated in master on GitHub (please don't close the issue as this will be done at the moment of an official release).

@maettu-this
Copy link
Author

@albert-github, thanks!

@doxygen
Copy link
Owner

doxygen commented May 20, 2024

This issue was previously marked 'fixed but not released',
which means it should be fixed in doxygen version 1.11.0.
Please verify if this is indeed the case. Reopen the
issue if you think it is not fixed and please include any additional information
that you think can be relevant (preferably in the form of a self-contained example).

@doxygen doxygen removed the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label May 20, 2024
@doxygen doxygen closed this as completed May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation bug in the documentation
Projects
None yet
Development

No branches or pull requests

3 participants