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

{platform}.Syntax modules #6849

Merged
merged 8 commits into from Feb 16, 2018

Conversation

Projects
None yet
3 participants
@RealyUniqueName
Copy link
Member

RealyUniqueName commented Feb 14, 2018

Unified some methods in modules:
php.Syntax,
js.Syntax,
python.Syntax

@nadako, please, review js and python related changes

@RealyUniqueName RealyUniqueName requested a review from nadako Feb 14, 2018

@skial skial referenced this pull request Feb 14, 2018

Closed

Haxe Roundup 419 #477

1 of 1 task complete
Generate `o[f]` expression
*/
static inline function field(o:Dynamic, f:String):Dynamic {
return code('{0}[{1}]', o, f);

This comment has been minimized.

@nadako

nadako Feb 14, 2018

Member

I would expect a method named Syntax.field generate o.f, not o[f].

This comment has been minimized.

@RealyUniqueName

RealyUniqueName Feb 14, 2018

Author Member

I failed to find a better name.
Also it mimics well known Reflect.field() method.

This comment has been minimized.

@kaikoga

kaikoga Feb 14, 2018

Contributor

Perhaps you meant o["f"] , in case of something like o["class"] becoming o.class?

This comment has been minimized.

@RealyUniqueName

RealyUniqueName Feb 14, 2018

Author Member

f is an identifier here, so i meant o[f] without quotes.
Example:

var fieldName:String = getFieldName();
Syntax.field(obj, fieldName); //generates obj[fieldName]
@@ -40,7 +40,7 @@ class HxOverrides {
switch( s.length ) {
case 8: // hh:mm:ss
var k = s.split(":");
var d = js.Syntax.new_(Date);
var d = js.Syntax.construct(Date);
(cast d)[cast "setTime"](0);

This comment has been minimized.

@nadako

nadako Feb 14, 2018

Member

I wonder why we do array access here instead of simply d.setTime...

This comment has been minimized.

@RealyUniqueName

RealyUniqueName Feb 14, 2018

Author Member

That's not in the scope of this PR :)

@RealyUniqueName

This comment has been minimized.

Copy link
Member Author

RealyUniqueName commented Feb 15, 2018

@nadako Can I merge this?

@nadako

This comment has been minimized.

Copy link
Member

nadako commented Feb 15, 2018

yeah, lgtm

@RealyUniqueName RealyUniqueName merged commit 6191e3c into HaxeFoundation:development Feb 16, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Gama11 added a commit to Gama11/haxe that referenced this pull request Feb 20, 2018

[js] fix __new__ deprecation
js.Syntax.new_() was renamed to construct() in HaxeFoundation#6849 without updating the the deprecation in genjs. This led to an error like this when __new__ is used:

Warning : __new__ is deprecated, use js.Syntax.new_ instead
Unknown js.Syntax method `new_` with 1 arguments

RealyUniqueName added a commit that referenced this pull request Feb 20, 2018

[js] fix __new__ deprecation (#6861)
js.Syntax.new_() was renamed to construct() in #6849 without updating the the deprecation in genjs. This led to an error like this when __new__ is used:

Warning : __new__ is deprecated, use js.Syntax.new_ instead
Unknown js.Syntax method `new_` with 1 arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.