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

ROASTER-1: Java Statement Fluent Model #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sotty
Copy link
Contributor

@sotty sotty commented Sep 8, 2014

Update Copyright

@gastaldi
Copy link
Member

gastaldi commented Sep 8, 2014

The changes in MethodImpl are not updated to the last commit, could you fix that? Thanks.

@sotty
Copy link
Contributor Author

sotty commented Sep 8, 2014

oops something went wrong with the rebase.. should be fixed now

@gastaldi gastaldi changed the title Java Statement Fluent Model ROASTER-1: Java Statement Fluent Model Sep 11, 2014
@gastaldi
Copy link
Member

It looks like it is not using the Forge formatter. Could you apply the formatter and rebase with the latest master?

Thanks!

@gastaldi
Copy link
Member

@agoncal
Copy link

agoncal commented Apr 17, 2015

What about having a method MethodSource<O> setBody( Statement... blocks ); ? This would allow something like :

    method.setBody(
            newDeclare().setVariable(Integer.class, "y"),
            newDeclare().setVariable(String.class, "x")
    );

WDYT ?

@sotty
Copy link
Contributor Author

sotty commented Apr 18, 2015

Agreed. Actually it's already in the next version of this PR, which I haven't committed yet :)

@agoncal
Copy link

agoncal commented Apr 18, 2015

Cool. Commit it and I'll give it a try

@sotty sotty force-pushed the stats_exprs branch 4 times, most recently from 72450f4 to 247be3b Compare July 26, 2015 02:49
@@ -33,4 +34,5 @@
*/
public boolean isLocalClass();

public JavaClassSource asJavaClassSource();
Copy link
Member

Choose a reason for hiding this comment

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

Why is this method needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leftover, removed

On Sat, Jul 25, 2015 at 11:46 PM, George Gastaldi notifications@github.com
wrote:

In api/src/main/java/org/jboss/forge/roaster/model/JavaClass.java
#27 (comment):

@@ -33,4 +34,5 @@
*/
public boolean isLocalClass();

  • public JavaClassSource asJavaClassSource();

Why is this method needed?


Reply to this email directly or view it on GitHub
https://github.com/forge/roaster/pull/27/files#r35489315.

Update Copyright

Conflicts:
	api/src/main/java/org/jboss/forge/roaster/model/source/MethodSource.java
	impl/src/main/java/org/jboss/forge/roaster/model/impl/AnnotationImpl.java
	impl/src/main/java/org/jboss/forge/roaster/model/impl/MethodImpl.java

Conflicts:
	impl/src/main/java/org/jboss/forge/roaster/model/impl/MethodImpl.java
@sotty
Copy link
Contributor Author

sotty commented Jul 26, 2015

Next item is checking the consistency of the internal "origin"/"parent" tree

@@ -8,6 +8,7 @@
package org.jboss.forge.roaster.model;

import org.jboss.forge.roaster.Roaster;
import org.jboss.forge.roaster.model.source.JavaClassSource;
Copy link
Member

Choose a reason for hiding this comment

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

Unused import

@gastaldi
Copy link
Member

I like this, excellent job!

@gastaldi
Copy link
Member

How is parsing of statements work in this proposal? Suppose you want to change some statement in an existing class (change the for loop condition, add another statement), how will this be possible?

import org.jboss.forge.roaster.model.source.JavaSource;
import org.jboss.forge.roaster.model.statements.Statement;

public abstract class StatementImpl<O extends JavaSource<O>,
Copy link
Member

Choose a reason for hiding this comment

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

Can we have more meaningful names for these generics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but for uniformity that would need to happen across all the codebase

@sotty
Copy link
Contributor Author

sotty commented Jul 28, 2015

This is the new feature I (and Mario) started adding in the latest
iteration. It needs more testing and some more work, but see the test case
here:
mpl/src/test/java/org/jboss/forge/test/roaster/model/statements/ModifyMethodBodyTest.java

On Tue, Jul 28, 2015 at 3:21 PM, George Gastaldi notifications@github.com
wrote:

How is parsing of statements work in this proposal? Suppose you want to
change some statement in an existing class (change the for loop condition,
add another statement), how will this be possible?


Reply to this email directly or view it on GitHub
#27 (comment).

@sotty
Copy link
Contributor Author

sotty commented Jul 28, 2015

Oops broken paste :)
https://github.com/sotty/roaster/blob/master/impl/src/test/java/org/jboss/forge/test/roaster/model/statements/ModifyMethodBodyTest.java

On Tue, Jul 28, 2015 at 4:39 PM, Davide Sottara dsotty@gmail.com wrote:

This is the new feature I (and Mario) started adding in the latest
iteration. It needs more testing and some more work, but see the test case
here:

mpl/src/test/java/org/jboss/forge/test/roaster/model/statements/ModifyMethodBodyTest.java

On Tue, Jul 28, 2015 at 3:21 PM, George Gastaldi <notifications@github.com

wrote:

How is parsing of statements work in this proposal? Suppose you want to
change some statement in an existing class (change the for loop condition,
add another statement), how will this be possible?


Reply to this email directly or view it on GitHub
#27 (comment).

@jamestyrrell
Copy link

Any update on this?

@forge-bot
Copy link
Contributor

Can one of the admins verify this patch?

@gastaldi
Copy link
Member

gastaldi commented Jul 6, 2016

@sotty, are you still working on this PR?

@sotty
Copy link
Contributor Author

sotty commented Jul 6, 2016

I am, and I have just come back to development this week after a long
period working on other things.
So expect to hear back from me very soon

On Wed, Jul 6, 2016 at 10:16 AM, George Gastaldi notifications@github.com
wrote:

@sotty https://github.com/sotty, are you still working on this PR?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAf0Olz4-yQGhfH4n_jwRkNDcGFT18uFks5qS8bDgaJpZM4CfcnO
.

@gastaldi
Copy link
Member

gastaldi commented Jul 6, 2016

Awesome, thank you. Haven't fully checked yet, but it would be nice if it supported Lambdas too.

@jamestyrrell
Copy link

@sotty That is great to hear as I just started to get my hands dirty and hit a wall when VariableDeclarator.getInit() gives me back a String.

@gastaldi
Copy link
Member

gastaldi commented Jul 6, 2016

Perhaps this project may give some ideas: https://github.com/square/javapoet

@prksean
Copy link

prksean commented Feb 12, 2018

@sotty Hi is there any update on this project? Thanks

@kasium
Copy link
Contributor

kasium commented Jan 26, 2019

@sotty I really love this idea. Do you think you are able to finish it in the next time or is there any update on this?

@sotty
Copy link
Contributor Author

sotty commented Jan 28, 2019 via email

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

Successfully merging this pull request may close these issues.

7 participants