Community
Participate
Working Groups
When the document is Save-As'ed by IStructuredModel#save(IFile), this file seems to be saved by java.io.File so that the saved file doesn't appear in the naviagator view. This seems to be because even if IFile points to the inside of the workspace, ModelManager uses JavaTextFileBufffer, not a ResourceTextFileBuffer, and because JavaTextFileBuffer uses java.io.File, not org.eclipse.core.internal.resources.File, this problem seems to happen.
I believe that "save" will go away, and we definitly should not be use Java file.io! But, will leave open until we document the right way (or API) to save a model. Thanks for reminding me this is critical for you.
Changing priority so won't be confused with "P1" meaning "do not declare milestone". They are just as important and high priority as they were.
Will give focus for M9.
I'm changing milestone to acknowledge I did not get this done for M9, but do still believe its important, and will attempt to find a safe fix to put in before R1.0. Thanks very much.
I've opened up TextFileBufferManager#createFileBuffer issue. Please see https://bugs.eclipse.org/bugs/show_bug.cgi?id=118199
By WTP convention, I'm marking this as P4, indicating we will not fix for Release 1.0, but we do agree its very important. Matsumoto-san has indicated to me that they have found a ("hacky") work around for immediate future (basically, I think calling refresh after a save). This will be much easier for us to fix if/when bug 118199 is fixed, but even if it is not, we can greatly improve/fix the situration next release. Thanks all.
As discussed on status call, we'll plan on this to be improved for 1.5, and retarget accordingly.
By the following bugzilla : https://bugs.eclipse.org/bugs/show_bug.cgi?id=107316 https://bugs.eclipse.org/bugs/show_bug.cgi?id=107320 the 'full path' like /workspace/WebContent/test.html is always used as the 'location' for TextBuffers including JavaTextFileBuffer. The side effect for these changes is that JavaTextFileBuffer still assumes that the location is the absolute path, like C:/workspace/ WebContent/test.html so that it can create java.io.File class to access the file. But SSE sets full path, not the absolute path, for JavaTextFileBuffer, it can't access the right file. So we will enable another workaround code ( to use TextFileDocumentProvider to save the document as TextEditor does. This way is a 'magic' ), but the real solution is not to use JavaTextFileBuffer to read/write the model from the workspace. .
no change in importance, just trying to "line up" with right WTP conventions on srubbing bugs. Since I'm not * positive * this will be in first 1.5 milestone, will leave blank for now.
no change in importance, just trying to "line up" with right WTP conventions on srubbing bugs.
David, please followup and determine if this is still required for 1.0.1. Thx.
As described in the history and related bugs, this problem is suitable for 1.5, not 1.0.1, and I simply forgot to re-assign the target.
Also, Matsumoto-san, since you've found a work around, and "Critical" means "rashes, loss of data, severe memory leak", I think this might better be categorized as 'major'. If you disagree, please explain here and we'll assign it back.
03/09 status meeting - No progress this week.
Per the 04/20 status call, we are removing this from the hot list. Scott, if you feel this is still a hot bug for you, please explain why this is an hot issue. Thanks.
since you've worked around this, I'd prefer not to bother this relase, but will look at next release when we made a formatl API. Sound acceptable?
As Matsumoto-san mentioned before, we have the workaround code, however, it can not completely avoid this problem. Our workaround code works fine if a file does not exist. But once the file is saved, after that, JavaTextFileBuffer is used to save it until the model is released. So the file will be in the out-of-sync state. And we have not found the workaround about this issue yet.
Thanks for the info. I'll still (try) and take a look for this release, to see if a safe fix is possible.
This used to be a hot bug, but was deferred due to time. I'm putting back on the hotbug list, and we will investigate. (but, still not positive it can be fixed, but, we will try).
I believe one way to look at this bug is that there is nothing in SSE for us to do. We are simply waiting for bug 118199 to be fixed? Or, did you have another fix in mind for us? I ask because comment #17 mentions JavaTextFileBuffer but we do not use that class in SSE. Am I missing the point?
I'm marking as dependent on 118119, since I don't think there's much we in SSE can do about this, except to provide our own implementation of a TextFileBufferManager which we would not want to do in a maintenance release, even if we wanted to have our own implemenation (which, we'd prefer not to). Also, I apologize if this should be obvious from your description, but, how can we reproduce this in WTP by itself? We dont' seem to see a "ghost file" after creating a new file. Thanks.
Here are the updates. I couldn't reproduce this on SaveAs scenario within WTP. Now we can see the problem on using SSE API. I thought that following sequence reproduced the problem where 'file' is unexisting IFile: IStructuredModel model = StructuredModelManager.getModelManager().getNewModelForEdit(file, false); model.getStructuredDocument().set("content"); model.save(); But there is another problem (I think it is already reported, but I cannot find it now..) that this model#save() creates a file to wrong location (when the 'file' is yet to exist). And it prevents us from reproducing the 'ghost'. So the following is the alternative snippet to reproduce the 'out-of-synch' situation. On running the code, the IResourceChangeListener#resourceChanged() should be called and ".resourceChanged()" should be printed out. (Please make sure that auto-building is turned off.) final IResourceChangeListener rc = new IResourceChangeListener() { public void resourceChanged(IResourceChangeEvent event) { IResourceDelta d = event.getDelta().findMember(file.getFullPath()); if (d != null) { System.out.println(".resourceChanged()"); } } }; try { IStructuredModel model = StructuredModelManager.getModelManager().getNewModelForEdit(file, false); model.getStructuredDocument().set("content"); file.create(new ByteArrayInputStream(new byte[0]), true, new NullProgressMonitor()); // workaround for the file to be saved in the workspace try { Thread.sleep(1000); } catch (Exception e) {} Platform.getJobManager().join(ResourcesPlugin.FAMILY_AUTO_BUILD, null); ResourcesPlugin.getWorkspace().addResourceChangeListener(rc); model.save(); } catch (Exception e) { } finally { // ResourcesPlugin.getWorkspace().removeResourceChangeListener(rc); // leak this for testing } But it doesn't. When you type 'F5' to refresh resources on Navigator view, the ".resourceChanged()" comes. --- Anyway, I cannot see why you use IFile#getFullPath() instead of IFile#getLocation() in FileBufferModelManager#getFile(IFile) and in FileBufferModelManager#calcurateId(IFile). I seem it's the key to this. Could I have some more detailed info?
I'm re-opening, since some information was supplied, though, I haven't digested it all yet. But, to answer the last question, I believe we changed to getFullPath at your request :) thought I can't find that bug at the moment. If we don't use getFullPath, then there's some ambiguity due to bug 118199 and files are saved/created in the "wrong" place, and, if I recall, there's some danger of creating two models for what is essentially the same resource.
Ah, I found the related bugs: bug 107316 and bug 107320. These were "fixed" by the fullPath change.
Thank you for the answer, David. I see why getFullPath() :) and it is a sort of bug 118199.
Ok, I'll take it off the hotlist to acknowledge we can not fix it in this maintenance release, but, it bugs me too, so will leave open. Mabye we need to look closer at defining our own TextFileBufferManager (there one other thing is does we don't like ... notifies and gives the documnet to others when the document is created before the requester gets it! and, they also do an "automatic refresh" which can cause an auto-build to start, and, hence is not job safe :( As a work around for your case, is your code in a place you could programatically call "refresh" after the save?
We should still investigate this for a fix in WTP 2.0, as I think the base platform as improved their API's to make this fix possible on an Eclipse 3.3 base.
Now some base platform APIs are extended for this purpose and some APIs are marked as deprecated, that are still used in FileBufferModelManager. I guess this could be handled by fixing the use of deprecated APIs in FileBufferModelManager. Could you please take a look? Thank you,
Created attachment 68226 [details] adds usages of org.eclipse.core.filebuffers.LocationKind to calls to text file buffer manager Attaching a simple patch to make use of the connect/get methods that take a location kind and to supply them. The callers of connect/get already know whether they're operating on java.ui.Files or IFiles so the determination is simple, there. The internal info object has been altered to remember the location kind when those calls are made so it can also be used to correctly disconnect the buffer.
CCing Amy for a quick sanity check of the patch
code looks good, some additional suggestions: -since you're cleaning up deprecated calls, it looks like in detectContentType() FileBuffers.getSystemFileAtLocation(location); should be FileBuffers.getFileStoreAtLocation(location); -In getModel(IFile), there is a check for info != null before its fields are set. The same should probably also be done in getModel(File) -make sure you update the copyright
Created attachment 68403 [details] updated patch (In reply to comment #31) Most of this sounds good, but getting rid of the deprecated FileBuffers.getSystemFileAtLocation() call the right way would mean adding a requirement on org.eclipse.core.filesystem and making use of the IFileStore APIs, a change I feel is better left for 3.0. The other deprecated cleanups are specifically to fix the issue in this bug.
okay, updated patch looks good
Patch released for RC1.
