Skip to content

Changed option guessing to maxlength #7273

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Changed option guessing to maxlength #7273

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f4bc71
Changed option guessing to maxlength
caillioux Dec 18, 2016
b0113fc
Replaced "option" word with "html attribute"
caillioux Dec 18, 2016
52abda1
Changed section title
caillioux Dec 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions forms.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,11 @@ the guessed field.
.. index::
single: Forms; Field type guessing

Field Type Options Guessing
Field Attributes Guessing
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you did it but this change is not needed since guessers are about to change type's options, actually required and maxlength are define thanks to the attr option in the form factory but the registered doctrine and validation guessers are setting some options too, maybe it's worth documenting them though. I don't know what others think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think we should just remove the maxlength part and keep required?

Copy link
Contributor

Choose a reason for hiding this comment

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

No I don't :) I was suggesting to add undocumented guessed options, not to remove these ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

This change also needs to be reverted.

~~~~~~~~~~~~~~~~~~~~~~~~~~~

In addition to guessing the "type" for a field, Symfony can also try to guess
the correct values of a number of field options.
the correct values of a number of field attributes.

.. tip::

Expand All @@ -557,13 +557,13 @@ the correct values of a number of field options.
field type options can then be guessed from that information.

``required``
The ``required`` option can be guessed based on the validation rules (i.e. is
The ``required`` html attribute can be guessed based on the validation rules (i.e. is
the field ``NotBlank`` or ``NotNull``) or the Doctrine metadata (i.e. is the
field ``nullable``). This is very useful, as your client-side validation will
automatically match your validation rules.

``max_length``
If the field is some sort of text field, then the ``max_length`` option can be
``maxlength``
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for opening this PR! This change is legit, but I think you should also replace option by html attribute for both this one and required above. since this does not use the options at all, but the attr option, btw max_length option has been deprecated in 2.5.

If the field is some sort of text field, then the ``maxlength`` html attribute can be
guessed from the validation constraints (if ``Length`` or ``Range`` is used) or
from the Doctrine metadata (via the field's length).

Expand Down