Bug 180671 - [refresh] It is not possible to refresh editor with double clicking on it when file has been changed on remote machine
Summary: [refresh] It is not possible to refresh editor with double clicking on it wh...
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 2.0   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: contributed
Depends on: 181145
Blocks: 190774 256644
  Show dependency tree
 
Reported: 2007-04-03 06:33 EDT by Ali Burak Kulakli CLA
Modified: 2011-05-25 10:23 EDT (History)
1 user (show)

See Also:
kmunir: review+


Attachments
Need to check if the file need to be re-downloaded even if file has been opened (1.61 KB, patch)
2007-05-27 00:31 EDT, Xuan Chen CLA
no flags Details | Diff
recreated patch since this file has been changed in between. (1.61 KB, patch)
2007-05-30 11:25 EDT, Xuan Chen CLA
no flags Details | Diff
Updated according to Kushal's comment to remove duplicated code. (2.12 KB, patch)
2007-05-30 11:53 EDT, Xuan Chen CLA
no flags Details | Diff
Update patch since file changed in between. (2.12 KB, patch)
2007-06-01 09:57 EDT, Xuan Chen CLA
mober.at+eclipse: iplog+
Details | Diff
Updated according to Martin's comment. (1.19 KB, patch)
2007-06-04 14:47 EDT, Xuan Chen CLA
mober.at+eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Burak Kulakli CLA 2007-04-03 06:33:40 EDT
Open a file from remote machine. In my example it is a file (filelist) that has a list of files the the directory. Editor is opened with no problem.

Then in remote shell, update the file using 'ls -l > filelist'. In the editor, it does not say file has been changed or it is not possible to reload it by double clicking the file. You need to close the editor than double click the file.  Or you can use open with menu to open the file.
-----------Enter bugs above this line-----------
TM 2.0M6 Testing
installation : eclipse-platform-3.3M6 CDT-4.0M6
RSE install  : update-site RSE-runtime-all
java.runtime : Sun 1.6.0-b105
os.name:     : Windows XP 5.1, Service Pack 2
------------------------------------------------
systemtype   : Unix-ssh
targetos     : Open SUSE LINUX 10.2 (i586)
targetuname  : Linux linux-sbko 2.6.18.2-34-default #1 SMP Mon Nov 27 11:46:27 UTC 2006 i686 i686 i386 GNU/Linux
targetvm     : Sun Java HotSpot(TM) Client VM (build 1.5.0_08-b03, mixed mode)
------------------------------------------------
Comment 1 Martin Oberhuber CLA 2007-05-21 15:48:16 EDT
Seen this again with 2.0M7. Now that bug #181145 is fixed, I think the dbl click operation should do the same as Right-click > Open with > Text editor i.e. check the file and reload it if it has changed.
Comment 2 Xuan Chen CLA 2007-05-26 23:36:22 EDT
The following is the code segment for SystemViewRemoteFileAdapter#handleDoubleClick(): 

SystemViewRemoteFileAdapter#handleDoubleClick()
{
         .
         .
         .
 	 // only handle double click if object is a file
	  ISystemEditableRemoteObject editable = getEditableRemoteObject(remoteFile);
	  if (editable != null)
	  {
	    try
	    {
	      if (editable.checkOpenInEditor() != ISystemEditableRemoteObject.OPEN_IN_SAME_PERSPECTIVE)
	      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	      {						
	          if (isFileCached(editable, remoteFile))
	          {
		           editable.openEditor();
	          }
	          else
	          {
	               DownloadJob oJob = new DownloadJob(editable, false);
	               oJob.schedule();
	           }
	      }
	      else
            ^^^^
	      {
		      editable.setLocalResourceProperties();
		      editable.openEditor();
	      }
	    }
	    catch (Exception e)
	    {
	    }
								
	  }
         .
         .
         .
}

As we can see, if a file already been opened in the same perspective, we there is no isFileCached() check.  So if a file had been changed, it will not be downloaded again.

We need to check isFileCache() even when a file has been opened.
If the file has been updated in the host, we need to download it again.
Comment 3 Xuan Chen CLA 2007-05-27 00:31:01 EDT
Created attachment 68866 [details]
Need to check if the file need to be re-downloaded even if file has been opened

Legal Message: I, Xuan Chen, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.
Comment 4 David McKnight CLA 2007-05-30 10:05:10 EDT
Kushal, could you please review this?
Comment 5 Xuan Chen CLA 2007-05-30 11:25:42 EDT
Created attachment 69311 [details]
recreated patch since this file has been changed in between.

Legal Message: I, Xuan Chen, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.
Comment 6 Kushal Munir CLA 2007-05-30 11:31:41 EDT
Xuan, your changes are fine, but we end up with some duplicate code in the if and else blocks.

How about having editable.setLocalResourceProperties() if we're in the same perspective, and moving the other code (which is the same for both if and else blocks) outside the if block?  
Comment 7 Xuan Chen CLA 2007-05-30 11:53:11 EDT
Created attachment 69322 [details]
Updated according to Kushal's comment to remove duplicated code.

Legal Message: I, Xuan Chen, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.
Comment 8 Kushal Munir CLA 2007-06-01 08:00:11 EDT
Changes look good! Approved. Thanks Xuan.
Comment 9 Xuan Chen CLA 2007-06-01 09:57:21 EDT
Created attachment 69691 [details]
Update patch since file changed in between.

Legal Message: I, Xuan Chen, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.
Comment 10 Martin Oberhuber CLA 2007-06-04 06:36:13 EDT
Patch looks basically good, I'm just wondering whether it is OK to
  editable.setLocalResourceProperties();
AFTER the file has already been opened through
  editable.openEditor()?

This is a change compared to the previous way it worked -- please review and check.

Also note that because the new code does isFileCached() on dblcklick, which resolves to subsystem.getRemoteFileObject() this performs a remote query IN THE UI THREAD. Which is dangerous, because if it hangs all Eclipse will hang. There should either be a watchdog to cancel the query after some time, or there should be some delegation to a background thread.

I created bug #190774 to deal with this potentially dangerous situation in 2.0.1
Comment 11 Xuan Chen CLA 2007-06-04 10:37:44 EDT
I will update the fix to make sure for the case where the file already been opened in the same perspective, call setLocalResourceProperties() first, then call openEditor().

Comment 12 Xuan Chen CLA 2007-06-04 14:47:01 EDT
Created attachment 70008 [details]
Updated according to Martin's comment.

Legal Message: I, Xuan Chen, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.
Comment 13 David McKnight CLA 2007-06-05 07:50:01 EDT
I've applied the update to cvs.
Comment 14 Martin Oberhuber CLA 2008-08-13 13:20:14 EDT
[target cleanup] 2.0 RC2 was the original target milestone for this bug