Skip to content

Use equal sign instead of colon in Symfony routing#3249

Closed
stephanvierkant wants to merge 7 commits intorectorphp:masterfrom
stephanvierkant:printArrayItemWithSeparator
Closed

Use equal sign instead of colon in Symfony routing#3249
stephanvierkant wants to merge 7 commits intorectorphp:masterfrom
stephanvierkant:printArrayItemWithSeparator

Conversation

@stephanvierkant
Copy link
Copy Markdown
Contributor

@TomasVotruba
Copy link
Copy Markdown
Member

I looked at a code and it seems there are multiple issues at once. I recommend sending 1 PR per issues.

I'd love to have the = first, as it influences the most users right now.

@stephanvierkant stephanvierkant force-pushed the printArrayItemWithSeparator branch from 78cc3f1 to 31f6dae Compare April 26, 2020 15:53
@stephanvierkant
Copy link
Copy Markdown
Contributor Author

Rector removes the " because it assumes the options in the annotation are in json format, while it isn't.

\Rector\BetterPhpDocParser\PhpDocNode\AbstractTagValueNode::printArrayItem creates a JSON string and hacking this (stop removing the ") doesn't feel right. WDYT?

@TomasVotruba
Copy link
Copy Markdown
Member

Not sure what is the question.

@stephanvierkant
Copy link
Copy Markdown
Contributor Author

stephanvierkant commented Apr 26, 2020

How do we stop Rector removing the quotes?

PrintArrayItem now returns a JSON string. But the @Routing options expect not a JSON format. So should this method be changed?

@TomasVotruba
Copy link
Copy Markdown
Member

I'm change for better 👍

@stephanvierkant
Copy link
Copy Markdown
Contributor Author

Master changed significant, so can't complete this PR.

@TomasVotruba
Copy link
Copy Markdown
Member

Feel free to delete fork, fork again and to re-open new one.
There are mostly test fixtures in the PR, should be doable.

@stephanvierkant stephanvierkant deleted the printArrayItemWithSeparator branch April 30, 2020 11:50
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.

Couldn't find constant expose:true

2 participants