fix: remove content* methods from UrlPdfBuilder and UrlScreenshotBuilder#266
fix: remove content* methods from UrlPdfBuilder and UrlScreenshotBuilder#266marilenaRM wants to merge 3 commits into
Conversation
Jean-Beru
left a comment
Jean-Beru
left a comment
There was a problem hiding this comment.
Thanks you @marilenaRM !
It could be considered as a BC break but it's a bug fix... Maybe deprecating these methods will be more appropriated. WDYT @Neirda24 @StevenRenaux
PS: don't forget to update UPGRADE file to mention this change.
It would work I would say yes to the BC break but as it is not the case, not sure of the usefulness |
|
Shouldn't we split this in a dedicated trait ? As experienced before in this project we noticed that handling trait conflict while using reflection might just get things messy. Not in this specific case though. Don't mind the BC it is a bugfix to me as it doesn't produce anything I guess. |
marilenaRM
force-pushed
the
fix/remove-content-methods-from-url-builders
branch
from
d0458fd to
31a781b
Compare
|
Hello ! :) @Jean-Beru : I just added a commit for the update of the file UPGRADE :) @Neirda24 I pushed a separate commit for a way to introduce a separated Trait, WDYT? |
That's the reason why it can break current code. The following code works even if the $builder
->content(/*...*/)
->url(/* ... */)
👍
Then we could:
Later we could:
|
This solution LGTM 🤩 |
|
so, what's the plan? deprecate in 1.2 and BC Break in 2.0? |
LGTM |
header*, footer* methods move to the new PageMarginalTrait. The original methods remain in ContentTrait as deprecated no-ops (E_USER_DEPRECATED, will be removed in 2.0) for BC, allowing existing code to receive a deprecation warning rather than a runtime error. ChromiumPdfTrait is updated to prefer PageMarginalTrait over the deprecated ContentTrait stubs via insteadof. Screenshot builders keep using ContentTrait directly since Gotenberg does not support headers and footers for screenshots; the deprecated stubs are filtered out of the generated documentation. The deprecation message was corrected: the stubs in ContentTrait are only ever called by builders that do not use PageMarginalTrait (e.g. screenshot builders), so saying "Use PageMarginalTrait instead" would be misleading. The message now states that header/footer support is not available on the current builder.
Since the page body is fetched from the URL itself, content(), contentRaw() and contentFile() are meaningless on URL-based builders. They are now deprecated via DeprecatedBodyContentTrait (no-op + E_USER_DEPRECATED) and will be removed in 2.0.
Three bugs fixed in docs/generate.php: - @example merge: when a method appears in multiple traits (e.g. via insteadof resolution), the @example from the winning trait was silently discarded because the merge loop did not handle the example tag. The last seen @example now wins. - Description merge: a docblock containing only tags (@deprecated, @see, etc.) produces an empty description array [], which is distinct from [''] and incorrectly satisfied the overwrite condition, erasing a previously collected verbose description. The condition now guards against both empty forms. - Deprecated method filtering: public methods whose resolved docblock carries @deprecated are now excluded from the generated output. This prevents no-op stubs from appearing in the documentation without any useful content.
marilenaRM
force-pushed
the
fix/remove-content-methods-from-url-builders
branch
from
3f81f67 to
b84f329
Compare
4 participants
Description
For URL-based builders, the page body is provided by the URL itself. Exposing content(), contentRaw() and contentFile() was misleading since those methods have no effect on URL conversions.
Superseded by #274