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

[Validator] Missing information about payload in callback #18791

Merged

Conversation

alamirault
Copy link
Contributor

Fix #18787

and the :class:`Symfony\\Component\\Validator\\Context\\ExecutionContextInterface`
instance as the second argument.
, the :class:`Symfony\\Component\\Validator\\Context\\ExecutionContextInterface`
instance as the second argument and the :ref:`payload option <payload>` as the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the :ref: I used. When I build locally it's broken: href is Sequentially.html#payload

What syntax should I use?


It's in .. include:: /reference/constraints/_payload-option.rst.inc

image

Copy link
Member

Choose a reason for hiding this comment

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

This is tricky:

  • RST creates the anchors based on the section titles
  • We're including a RST doc with a section title called Payload
  • So, the payload cross reference exists and it's valid

But, I don't like this automatic references much because if you change the title, then the reference breaks. To avoid this, while merging I've added a explicit reference name in the included file and used it in the links. See 61fcb40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the payload cross reference exists and it's valid

This behavior is very strange, and can confuse user because it redirect to another page. https://symfony.com/doc/current/reference/constraints/Ip.html#reference-constraints-payload for example

Do you know if we can improve this, in which repository ?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's fixable in symfony-tools repository or in the underlying RST parser 😐

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created issue doctrine/rst-parser#275 with a reproducer and fix proposal (I hope 😄 )

@carsonbot carsonbot changed the title Missing information about payload in callback [Validator] Missing information about payload in callback Aug 28, 2023
@javiereguiluz javiereguiluz merged commit 111e861 into symfony:5.4 Aug 28, 2023
3 checks passed
@javiereguiluz
Copy link
Member

Antoine, thanks for fixing this bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants