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

Don't have curly for propertylookup in offset #128

Closed
alexander-akait opened this issue Mar 6, 2018 · 8 comments
Closed

Don't have curly for propertylookup in offset #128

alexander-akait opened this issue Mar 6, 2018 · 8 comments
Assignees
Labels

Comments

@alexander-akait
Copy link
Collaborator

Example:

$this->{quantity};

Ref: prettier/plugin-php#112

@ichiriac ichiriac self-assigned this Mar 6, 2018
@ichiriac ichiriac added this to the 3.0.0 milestone Mar 6, 2018
@czosel
Copy link
Collaborator

czosel commented Mar 6, 2018

This might be a duplicate of #101

@ichiriac
Copy link
Member

ichiriac commented Mar 6, 2018

Not really, here the problem is about unclear AST. In fact it's not really a bug as the output can be resolved as following :

image

If the name is an identifier node, use '{' + name.name + '}' or else use directly name.

I find it a bit ugly, so I'll use this version in order to introduce changes on AST and make it more intuitive, related on #62

@ichiriac ichiriac added enhancement and removed bug labels Mar 6, 2018
@czosel
Copy link
Collaborator

czosel commented Mar 6, 2018

Oh, of course - and it's not even about encapsed. I guess i should take a break 😉

@czosel
Copy link
Collaborator

czosel commented Mar 9, 2018

@ichiriac the workaround in our prettier plugin is actually not too bad: prettier/plugin-php#183

@alexander-akait
Copy link
Collaborator Author

/cc @ichiriac friendly ping, after some delay let's solve this issue, any ideas how better solve this?

@ichiriac
Copy link
Member

ichiriac commented Jan 3, 2019

Hi @evilebottnawi,

I'm sorry that my reply took so long, I'm almost free & back now. I'll work this saturday all the day, I still plan to release it on a stable version soon.

I wish you an happy new year plenty of joy and cool projects 😄

@ichiriac ichiriac added the high-pri High Priority bug label Jan 7, 2019
@ichiriac
Copy link
Member

ichiriac commented Jan 7, 2019

Hi @evilebottnawi,

Could you check remaining issues and add the flag high-pri on every issue you need to be fixed for the stable version ? Like this I'll know on what to focus.

@ichiriac ichiriac modified the milestones: 3.0.0, 3.0.0-prerelease.9 Jan 7, 2019
ichiriac pushed a commit that referenced this issue Jan 22, 2019
@ichiriac
Copy link
Member

hi, it's implemented, take a look at the comment #248 (comment) in order to see the implementation rules

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

No branches or pull requests

3 participants