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

Use short form for type casts #102

Merged
merged 1 commit into from
Dec 26, 2017

Conversation

czosel
Copy link
Collaborator

@czosel czosel commented Dec 26, 2017

The type of types casts has been inconsistent: Sometimes, an alias/long form was used (e.g. boolean), other times the short form (e.g. int). This PR changes this by always using the short form.
Please also see prettier/plugin-php#5 (comment) for some more context.
Thanks for your feedback in advance!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.389% when pulling f1c5d7a on czosel:casts_with_shortnames into 6e6b9f2 on glayzzle:master.

@czosel
Copy link
Collaborator Author

czosel commented Dec 26, 2017

Hi @ichiriac, a little question on the side: I noticed you're not using ES6 features - is that intentional? I also saw #50 - maybe it would be a good idea to introduce eslint, prettier, and slowly move towards ES6?

@@ -272,7 +272,7 @@ module.exports = {
return this.node('cast')('int', this.next().read_expr());

case this.tok.T_DOUBLE_CAST:
return this.node('cast')('double', this.next().read_expr());
return this.node('cast')('float', this.next().read_expr());
Copy link
Member

Choose a reason for hiding this comment

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

not sure in PHP but in principle double and float have not the same precision, I'll check this

Copy link
Member

Choose a reason for hiding this comment

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

😄 got it from https://github.com/glayzzle/php-parser/blob/master/src/lexer.js#L111, actually there is only one token to these 3 casts types

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly - they are just aliases in PHP 😉

@@ -142,28 +142,35 @@ describe('Test expressions', function() {
});

it('test cast', function() {
var typesWithAlias = {
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@ichiriac ichiriac left a comment

Choose a reason for hiding this comment

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

everything is good

@ichiriac ichiriac merged commit 2631f4c into glayzzle:master Dec 26, 2017
@ichiriac
Copy link
Member

hi @czosel, I'll release it under 2.2.0 soon, I need to fix another little regression elsewhere, almost done

@czosel
Copy link
Collaborator Author

czosel commented Dec 26, 2017

Awesome! Thanks to your support I'm positive we'll have a working prettier implementation for PHP soon!

@czosel czosel deleted the casts_with_shortnames branch March 30, 2018 20:03
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