Skip to content

[Validator] Allow ConstraintViolation::__toString() to expose codes that are not null or emtpy strings #29375

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

Merged
merged 1 commit into from
Dec 1, 2018

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Nov 29, 2018

Q A
Branch 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Allow to expose 0 or "0" validation codes.

@@ -79,13 +79,13 @@ public function __toString()
}

$propertyPath = (string) $this->propertyPath;
$code = $this->code;
$code = trim($this->code);
Copy link
Contributor

Choose a reason for hiding this comment

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

trimming is a new behavior, which IMHO is not needed.

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 understand your point, and I agree. My concern was originated by the absence of delimiters around the printed code, so I thought that readability could be improved trimming any undesired blankspace around the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

the delimiter is (code and ) :)

undesired blankspace

we don't know that

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 mean cases like

(code    )

I think it is not clear which space comes from the dumped message and which one comes from the actual code, but I'm pretty sure this is an edge case that we can ignore.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(for 3.4 as 2.8 is EOLed now)

@phansys phansys changed the base branch from 2.8 to 3.4 November 30, 2018 22:17
@phansys
Copy link
Contributor Author

phansys commented Nov 30, 2018

Updated base to target 3.4.

@nicolas-grekas nicolas-grekas force-pushed the constraint_violation_tostring branch from 1cbe5dc to 7bb0fb5 Compare December 1, 2018 08:21
@nicolas-grekas
Copy link
Member

Thank you @phansys.

@nicolas-grekas nicolas-grekas merged commit 7bb0fb5 into symfony:3.4 Dec 1, 2018
nicolas-grekas added a commit that referenced this pull request Dec 1, 2018
…xpose codes that are not null or emtpy strings (phansys)

This PR was squashed before being merged into the 3.4 branch (closes #29375).

Discussion
----------

[Validator] Allow `ConstraintViolation::__toString()` to expose codes that are not null or emtpy strings

|Q            |A  |
|---          |---|
|Branch       |2.8|
|Bug fix?     |yes|
|New feature? |no |
|BC breaks?   |no |
|Deprecations?|no |
|Tests pass?  |yes|
|Fixed tickets|n/a|
|License      |MIT|
|Doc PR       |n/a|

Allow to expose `0` or `"0"` validation codes.

Commits
-------

7bb0fb5 [Validator] Allow `ConstraintViolation::__toString()` to expose codes that are not null or emtpy strings
@phansys phansys deleted the constraint_violation_tostring branch December 1, 2018 10:20
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.

5 participants