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

[WIP v1.2] Add full compatibility with Symfony\Yaml 4.4 #21

Closed
wants to merge 5 commits into from

Conversation

mjauvin
Copy link
Member

@mjauvin mjauvin commented Mar 28, 2021

No description provided.

@mjauvin mjauvin changed the title Do not quote class names as keys Use single quotes instead of double quotes Mar 28, 2021
@mjauvin
Copy link
Member Author

mjauvin commented Mar 28, 2021

@LukeTowers can you merge this to fix the yaml files parsing?

Thanks.

@LukeTowers
Copy link
Member

Can you add tests for what happens if single and double quotes appear in the unquoted values?

@mjauvin
Copy link
Member Author

mjauvin commented Mar 28, 2021

@LukeTowers You mean if a value in the original file (before it gets pre-parsed) contains single or double quotes?

testQuoting:
complex1: 'this is "very" complex quoting'
complex2: "this is also 'very' complex quoting"
middle: this string is "quoted" in the middle
Copy link
Member

Choose a reason for hiding this comment

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

How about the above but also just a single quote? Complex2 would pass without being modified because it's a quoted value in it's entirety, and middle would pass because the value would be wrapped in single quotes, but I'm wondering about middeSingle: this string has some single quote's, and maybe a 'air quote', anything to make your life diff'cult

@LukeTowers LukeTowers changed the base branch from develop to wip/1.2 March 29, 2021 02:45
@LukeTowers LukeTowers changed the title Use single quotes instead of double quotes [WIP] Add full compatibility with Symfony\Yaml 4.4 Mar 29, 2021
@LukeTowers LukeTowers added this to the v1.2.0 milestone Mar 29, 2021
@LukeTowers LukeTowers changed the title [WIP] Add full compatibility with Symfony\Yaml 4.4 [WIP v1.2] Add full compatibility with Symfony\Yaml 4.4 Mar 29, 2021
@tschallacka tschallacka mentioned this pull request Apr 25, 2021
@LukeTowers
Copy link
Member

@mjauvin what's the status on this?

@mjauvin
Copy link
Member Author

mjauvin commented Aug 9, 2021

@mjauvin what's the status on this?

@LukeTowers since we reverted @bennothommo 's work on this, it's currently irrelevant as far as I remember.

@LukeTowers
Copy link
Member

@bennothommo what's the status of this?

@bennothommo
Copy link
Member

@LukeTowers you'd have to check with @mjauvin of what the purpose of this PR was? I'm not sure if it's needed, but I also can't recall the reason we had to roll back my processor originally - there was some sort of issue with it, I imagine?

@mjauvin
Copy link
Member Author

mjauvin commented Dec 15, 2021

@LukeTowers it was needed at the time (see the tests that were added). Not sure if it's still relevant.

@mjauvin mjauvin deleted the class-backslash branch February 20, 2022 19:33
@mjauvin mjauvin removed this from the v1.2.0 milestone Feb 27, 2022
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