These are chat archives for JnRouvignac/AutoRefactor

14th
Nov 2016
Luis Cruz
@luiscruz
Nov 14 2016 12:45
Hi
I’m having problems with a comment that gets removed after refactoring
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:46
hello
yes tell me
or show me the culprit code :)
Luis Cruz
@luiscruz
Nov 14 2016 12:46
Are comments part of other statements?
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:46
I bet it is calling ASTBuilder.copy()
nope they live in a strange world
a bit there, a bit not there
Luis Cruz
@luiscruz
Nov 14 2016 12:47
ok, let me check it
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:47
they are a bit like ghosts
:)
Luis Cruz
@luiscruz
Nov 14 2016 12:47
:sweat_smile:
yea I’m using copy()
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:50
you would need to use move() or other techniques in order to keep the source as close to the original as possible
Luis Cruz
@luiscruz
Nov 14 2016 12:50
humm actually
do you think this also removes it r.remove(visitor.viewAssignmentStatement); ?
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:51
maybe
code! I need code!
I cannot tell just like this
Luis Cruz
@luiscruz
Nov 14 2016 12:51
because the thing is I am creating a different statement based on this one, and then I get rid of it
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:52
but definitely, you should not both move and delete the same node
it was ok when you copied
but copy always means you lose the comments
you should use move() by default
Luis Cruz
@luiscruz
Nov 14 2016 12:53
I am copying the inflater.inflate expression
and in the end I remove the original statement
so even if I “move” I wouldn’t get the comments, since it is only a subtree
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:55
I see no comments here?
so no comments to lose?
Luis Cruz
@luiscruz
Nov 14 2016 12:56
I’ll do it again
            //this should not be refactored
            rootView = inflater.inflate(android.R.layout.simple_list_item_1, parent, false);
            //this should not be refactored
            if (convertView == null) {
                convertView = inflater.inflate(android.R.layout.simple_list_item_1, parent, false);
            }
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:57
so you are losing this comment?
Luis Cruz
@luiscruz
Nov 14 2016 12:57
yes!
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:57
by default comments are attached to the nodes following them
this is a property of the ASTParser
Luis Cruz
@luiscruz
Nov 14 2016 12:58
ok, so if I want to keep them I have to make sure I copy it before removing the node
is that right?
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:59
I do not think that will work
I recall something and I think it was more complicated than this
Luis Cruz
@luiscruz
Nov 14 2016 12:59
:\
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 12:59
comments are somewhat attached to an ASTNode, but they are not directly part of the AST
or directly attached to an ASTNode
hence why I say there are there, but not there
Luis Cruz
@luiscruz
Nov 14 2016 13:00
ya, I get it now
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:01
sorry it was a bit mystical, but very hard to explain without a concrete example :)
Actually I think it is a property of the ASTRewrite:
See setTargetSourceRangeComputer()
eclipse internal API has a NoCommentSourceRangeComputer
Luis Cruz
@luiscruz
Nov 14 2016 13:05
What do you think of checking getAlternateRoot() in every Comment
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:05
what do you want to do with it?
Luis Cruz
@luiscruz
Nov 14 2016 13:06
I would get the comment for the node I want to remove and then reassign a different node using setAlternateRoor()
not sure if I understood these Comment methods
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:07
I would be surprised if that work: that would work very differently from the rest of the JDT APIs
the best advice i can give you is to give it a shot
or else duck and ignore this problem (as I cowardly did)
Luis Cruz
@luiscruz
Nov 14 2016 13:08
I will create a comment and assign it using setAlternateRoot
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:08
actually not totally I went as far as understanding using the move() method is important :)
ok, try and see if that helps
Luis Cruz
@luiscruz
Nov 14 2016 13:09
I dont believe it willl work as well, but it shouldn’t take too much time
:)
does’t work
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:13
:-|
Luis Cruz
@luiscruz
Nov 14 2016 13:19
anyway, I’ll go over this after lunch
thanks ;)
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 13:20
np sorry I could not give you the ultimate answer
Luis Cruz
@luiscruz
Nov 14 2016 13:29
no worries ;)
Luis Cruz
@luiscruz
Nov 14 2016 17:35
do you know how can I get text from a comment?
Luis Cruz
@luiscruz
Nov 14 2016 17:45
well, never mind. I give up for now. It’s not a priority for now.
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 18:05
See CommentsRefactoring
Luis Cruz
@luiscruz
Nov 14 2016 18:10
changing the topic, why does getAncestor throws an exception instead of returning null?
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 18:12
Because when using this API, it is expected to find the required type
I never had a problem with it
Is it causing you grief ?
Luis Cruz
@luiscruz
Nov 14 2016 18:14
:sweat_smile: it’s just that in my code sometimes there are cases in which I’m not expecting it to find the ancestor
so a simple `ìf/else``turns into nested try/catch statements
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 18:16
Ah this is ugly
Maybe can we add getAncestorOrNull?
Luis Cruz
@luiscruz
Nov 14 2016 18:18
yea
shall I do it?
Jean-Noël Rouvignac
@JnRouvignac
Nov 14 2016 18:18
Yes
Please