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

Update doc generation #149

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

Neirda24
Copy link
Contributor

@Neirda24 Neirda24 commented Feb 20, 2025

Q A
Gotenberg API version ? 8.x
Bug fix ? no
New feature ? yes
BC break ? no
Issues Fix #...

Description

Better parsing of phpdoc in case of multlines param + parsing each tag like see to be added as a tip

  • Easier to maintain
  • Handles parent classes / interfaces / traits and overrides on children
  • handle sorting of classname / builder types / method names and @package
  • introduce @package to group some method in documentation automatically

@Neirda24
Copy link
Contributor Author

@Jean-Beru , @StevenRenaux : should we sorts methods by names ? At the moment it is sorted by decalaration order I think. Which in case of refactoring makes it much harder to detect in documentation what changed and what didn't. Forcing the order by name would make this much easier BUT it would also make it less coherent because some methods might be better grouped together. WDYT should be the trade off ?

@Jean-Beru
Copy link
Contributor

I updated (without commit) the script to sort by name when working on the refactoring. It's perfect to see differences between 2 versions.

But I think that it could only be done locally or in the CI (maybe by generating sorted n and n-1) to keep consistency in the documentation itself.

The goal of this script should be generating documentation.

@Neirda24
Copy link
Contributor Author

I agree @Jean-Beru . Still it would make sense to keep it consistent. Because of traits it may generate noise on doc generation for no reasons. Enforcing sorting might be worth it. Regarding the CI I don't think we need the n-1. What I meant regarding the upgrade is mostly for local purposes yes.

@Jean-Beru
Copy link
Contributor

Should we add an annotation to allow grouping?

@Neirda24
Copy link
Contributor Author

maybe... The native @package from php ?

@@ -12,6 +12,8 @@ trait CookieAwareTrait
/**
* Cookies to store in the Chromium cookie jar. (overrides any previous cookies).
*
* @package Behavior\\Chromium\\Cookie
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WDYT of this way ? the value of package is only used for aggregation and sorting at the moment. It is not printed anywhere. Also should the @package be placed here or at the bottom of the phpdoc ?

Copy link
Contributor

@Jean-Beru Jean-Beru left a comment

Choose a reason for hiding this comment

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

Quick review before an other one to understand how parsing works.

Comment on lines 11 to 13
/**
* @see https://google.com
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

It's like "find by yourself" ? :trollface:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

Comment on lines 143 to 151
$markdown = '';

if ('' === $line) {
continue;
}
if (\count($seeList) > 0) {
$markdown .= '> [!TIP]';
}

foreach ($seeList as $see) {
$markdown .= "\n> See: [{$see}]({$see})";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

            if ([] === $seeList) {
                return '';
            }
            
            $markdown = '> [!TIP]';
            foreach ($seeList as $see) {
                $markdown .= "\n> See: [{$see}]({$see})";
            }

@Neirda24 Neirda24 requested a review from Jean-Beru February 26, 2025 08:40
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.

2 participants