Bug 327410

Comment 1 Navid Mehregani 2010-10-14 18:30:32 EDT

Chris, 

This should be working from the diagrams.  What you need to do is SINGLE click on the name of an artifact in a diagram.  This will invoke the "rename refactoring" for the corresponding artifact.

I'm still thinking that a 'Rename' context menu option might not be a bad idea as the above step may not be obvious to some users.  Let me know your thoughts.

Comment 2 Chris Hartley 2010-10-14 18:46:49 EDT

This may be realted to or a duplicate of https://bugs.eclipse.org/bugs/show_bug.cgi?id=327000

Comment 3 Navid Mehregani 2010-10-14 19:18:51 EDT

(In reply to comment #2)
> This may be realted to or a duplicate of
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=327000

I don't think this defect is related to 327000 (this is a deadlock issue).  As discussed in my e-mail, I think you were running into a different issue. 

Let's keep this enhancement open for the purpose of providing a Rename context menu action when right clicking on artifacts in diagram files.

Comment 4 Daniel Johnson 2010-11-02 18:55:11 EDT

Created attachment 182264 [details]
patch_327410.txt

Adds a 'Rename...' option to the right-click context menu for class diagrams.

Comment 5 Navid Mehregani 2010-11-04 14:46:58 EDT

(In reply to comment #4)
> Created an attachment (id=182264) [details]
> patch_327410.txt
> 
> Adds a 'Rename...' option to the right-click context menu for class diagrams.

Thanks Dan, but your patch doesn't work for Associations, Dependencies, and Extends relationships.

It likely has to do with the following code in RefactorRenameAction:
 private EObject findElement() {
        EditPart targetPart = findTargetPart();
        if (targetPart instanceof ClassDiagramShapeNodeEditPart ) 
            return ((Node) targetPart.getModel()).getElement();
        	
        return null;
    }

Also, why are you including the following in your action contribution?

 <selection                 class="org.eclipse.tigerstripe.workbench.ui.visualeditor.diagram.Object1">
  </selection>

The referenced class doesn't even exist and using <selection> element is not even needed in this case.

Comment 6 Daniel Johnson 2010-11-04 15:24:03 EDT

Created attachment 182416 [details]
patch_327410.txt

That selection tag was leftover from me experimenting, i have removed it.
Added ability to rename Associations, Association Classes, Dependencies. 
Extends makes no sense to be able to rename :-)

Does anyone know how to make the option not visible on Instance Diagrams?

Comment 7 Navid Mehregani 2010-11-04 15:59:09 EDT

Sorry, I wasn't thinking straight when I mentioned Extends relationship.
I verified the new patch and it looks good.  Thanks!
Patch Submitted.

Comment 8 Navid Mehregani 2010-11-04 16:08:19 EDT

BTW, wrt your question about removing this action from instance diagrams....ideally, we would want this action to also work from instance diagrams so instead of removing it, I'd vote for supporting it in instance diagrams.

Comment 9 Daniel Johnson 2010-11-04 16:50:51 EDT

If I understand it correctly the instance diagrams don't actually link to real artifacts, so a rename doesn't need to fire the refactor business logic. I think we should have the ability, but I think the logic should be separate. That is my initial thoughts anyway. Currently using this "Rename..." on an association in an instance diagram is allowed, but clicking it does not do anything.

Comment 10 Chris Hartley 2010-11-04 18:10:47 EDT

I don't think we should allow renames at all from instance diagrams.
I'd prefer the option removed, but I'm happy if it doesn't do anything.
Instance diagrams are used to document the model by providing examples.

I'd suggest asking Richard about how to disable the rename option on instance diagrams.

Comment 11 Daniel Johnson 2010-11-04 18:55:03 EDT

Currently once you create an instance artifact on an instance diagram there is no way to change the name, not even in the properties view. Because of this I agree with Chris that the option should be removed from instance diagrams. A Rename from an instance diagram that affects the whole model seems misleading, and since we don't allow renaming of the actual instance artifact there is no need for it.

Comment 12 Chris Hartley 2010-11-04 18:59:43 EDT

If rename was going to do anything it would be to rename the instance (not the class).

Currently we do that by double clicking the instance and changing the instance name in the dialog box.

Comment 13 Navid Mehregani 2010-11-04 22:42:23 EDT

(In reply to comment #12)
> If rename was going to do anything it would be to rename the instance (not the
> class).
> 
> Currently we do that by double clicking the instance and changing the instance
> name in the dialog box.

Exactly, we do allow for the instances to be renamed.  See attached snapshot.  This is activated by double clicking on the instance name.  I was suggesting that the 'Rename' action would allow the renaming of *instances*.  Instead of user double clicking on the name (which is kind of hidden), it would be exposed through 'Rename' context menu action.  There is no good reason why this shouldn't be done and I'm not convinced about hiding 'Rename' action in instance diagrams.

Comment 14 Navid Mehregani 2010-11-04 22:44:04 EDT

Created attachment 182446 [details]
Rename dialog in instance diagrams.