Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 354823

Summary:	[rename] rename refactoring should save open documents
Product:	[Modeling] TMF	Reporter:	Knut Wannheden <knut.wannheden>
Component:	Xtext	Assignee:	Jan Koehnlein <jan>
Status:	CLOSED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	e.terekhov, jan, tmf.xtext-inbox, vpiskarov
Version:	2.0.1	Flags:	jan: indigo+
Target Milestone:	SR2
Hardware:	PC
OS:	Mac OS X - Carbon (unsup.)
Whiteboard:

Description Knut Wannheden

2011-08-16 09:18:55 EDT

Invoking a rename only persists changes to resources for which no open editor exists. Other resources OTOH are only updated in the editor which is left in a dirty state. As a result of this the following build (typically automatically triggered) may end up with linking errors.

I think the rename refactoring should instead save all touched resources, just like JDT does. Any other opinions on this?

Comment 1 Knut Wannheden

2011-08-16 11:07:58 EDT

Update: By default the JDT saves all files for which no *dirty* editor exists. In case of dirty editors only the document is updated. I think that's where the difference is: Xtext will *always* just update the the document if it finds an open editor for the given resource.

Comment 2 Vladimir Piskarev

2011-09-01 02:18:47 EDT

May I vote for this, too? As Knut have pointed out, JDT attempts to minimize the number of build errors resulting from inconsistencies between 'saved' and 'dirty' state after refactoring (these inconsistencies can't be fully avoided).

In addition, it seems that JDT policy for saving the document containing the element being renamed (as opposed to documents just referencing that element) is a function of the type of the element. Thus, when renaming a Java type it always saves the document containing the type declaration, but when renaming a method it avoids saving any documents that have already been 'dirty'.

To summarize, there appears to be one rule which JDT always obeys: never change 'clean' state to 'dirty' after refactoring (but not vice-versa).

Comment 3 Jan Koehnlein

2011-09-01 05:08:40 EDT

Having used refactoring a while now, I agree: We should try to mimick JDT behavior as close as possible.

Comment 4 Jan Koehnlein

2011-09-12 09:09:43 EDT

Pushed to MASTER.

We use a third kind of IRefactoringDocument now for documents that must be saved. For clean editors, the FileDocument is used, so we rely on the editor's resource listener to update the content. 

You can configure the refactoring behavior now in the preference pages. The same properties as in JDT are available.

Comment 5 Knut Wannheden

2011-09-12 13:38:37 EDT

That more or less corresponds to what we hacked together in our code. The downside of using FileDocument for clean editors is that it messes up the selection in the editor, as it isn't applied as a change to the document. So I think it would really be cleaner if an EditorDocument would be used and then saved at the end. What do you think?

Comment 6 Jan Koehnlein

2011-09-12 15:00:48 EDT

You are right, that's better.

Comment 7 Jan Koehnlein

2011-09-12 16:20:09 EDT

Fix and tests pushed to MASTER.

Comment 8 Jan Koehnlein

2011-09-16 07:27:29 EDT

Unfortunately, the current solution breaks the refactoring of Xtend2 classes. Seems like the document saving has a race condition with the resource rename change, causing a corrupt editor state.

Comment 9 Jan Koehnlein

2011-10-13 07:49:31 EDT

Fixed by Sven's commit 6c7f55abd2332df3975af37675c0d040193f7149