[FalconJX] MXMLFlexJSEmitter refactored, patch attached

[Tigran Najaryan <ti...@gmail.com>]

> org.apache.flex.compiler.internal.codegen.mxml.flexjs.MXMLFlexJSEmitter
> .java
> is a good place to start :-)

OK, I did some very simple refactoring. Let me know if it makes any sense. 

I verified that the refactored code emits exactly the same output when
compiling the FlexJSTest_again sample. 

Patch file attached (as output by git diff -- MXMLFlexJSEmitter.java, is
this the correct way to create a patch file?).

Comments and criticism are welcome.


One question. 

The old code was emitting an indentation based on a condition (!isMainFile
|| propertySpecifierNodes == null) however there was no matching indentPop()
for the earlier indentPush() call. Can you please explain what does this do?
I probably do not understand it but it looks like some kind of incomplete
code to me.

My refactored version is currently functionally equivalent to the old code,
it passes an 'indent' parameter to emitClassDeclStart().

Tigran.

Re: [FalconJX] MXMLFlexJSEmitter refactored, patch attached

[Erik de Bruin <er...@ixsoftware.nl>]

The list doesn't allow for attachements. I suggest you create a JIRA
ticket with a brief description of your changes and attach the patch
to that. I'll pick it up as soon as possible.

If you're feeling brave, you might want to have a look at the "var
self = this" issue. As Alex noted earlier, the "self" reference is
only needed when a reference to the class is passed as an argument of
a function call. In all other cases, the "self = this" is not needed.
"This" will suffice in all other cases. Attacking this will cause you
to have to change the asserts of some tests. Please be very careful
when doing that and verify that the actual output (FlexJSTest_again)
keeps functioning with the new code...

EdB



On Wed, Apr 10, 2013 at 3:40 PM, Tigran Najaryan <ti...@gmail.com> wrote:
>> org.apache.flex.compiler.internal.codegen.mxml.flexjs.MXMLFlexJSEmitter
>> .java
>> is a good place to start :-)
>
> OK, I did some very simple refactoring. Let me know if it makes any sense.
>
> I verified that the refactored code emits exactly the same output when
> compiling the FlexJSTest_again sample.
>
> Patch file attached (as output by git diff -- MXMLFlexJSEmitter.java, is
> this the correct way to create a patch file?).
>
> Comments and criticism are welcome.
>
>
> One question.
>
> The old code was emitting an indentation based on a condition (!isMainFile
> || propertySpecifierNodes == null) however there was no matching indentPop()
> for the earlier indentPush() call. Can you please explain what does this do?
> I probably do not understand it but it looks like some kind of incomplete
> code to me.
>
> My refactored version is currently functionally equivalent to the old code,
> it passes an 'indent' parameter to emitClassDeclStart().
>
> Tigran.



-- 
Ix Multimedia Software

Jan Luykenstraat 27
3521 VB Utrecht

T. 06-51952295
I. www.ixsoftware.nl

Re: [FalconJX] MXMLFlexJSEmitter refactored, patch attached

[Alex Harui <ah...@adobe.com>]

Please open a JIRA issue with your patch.


On 4/10/13 6:40 AM, "Tigran Najaryan" <ti...@gmail.com> wrote:

>> org.apache.flex.compiler.internal.codegen.mxml.flexjs.MXMLFlexJSEmitter
>> .java
>> is a good place to start :-)
> 
> OK, I did some very simple refactoring. Let me know if it makes any sense.
> 
> I verified that the refactored code emits exactly the same output when
> compiling the FlexJSTest_again sample.
> 
> Patch file attached (as output by git diff -- MXMLFlexJSEmitter.java, is
> this the correct way to create a patch file?).
> 
> Comments and criticism are welcome.
> 
> 
> One question. 
> 
> The old code was emitting an indentation based on a condition (!isMainFile
> || propertySpecifierNodes == null) however there was no matching indentPop()
> for the earlier indentPush() call. Can you please explain what does this do?
> I probably do not understand it but it looks like some kind of incomplete
> code to me.
> 
> My refactored version is currently functionally equivalent to the old code,
> it passes an 'indent' parameter to emitClassDeclStart().
> 
> Tigran.

-- 
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui