Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #344653 +++ +++ This bug was initially created as a clone of Bug #344652 +++ +++ This bug was initially created as a clone of Bug #344031 +++ +++ This bug was initially created as a clone of Bug #117746 +++ Loading large files in an editor needs a lot of memory (up to 5 times the file size, see Bugzilla 75086), so the Workbench can run into an OutOfMemory error or freeze for some time while an editor is busy loading a file. Since users do not see in the Resource navigator how large a file is, it can easily happen that one tries to open a very large file by accident. We have auto-generated log files which range from 1 to 400 MB in size. The smaller ones we want to open in Eclipse Workbench, the larger ones we open in an external editor. Trying to opening a 400MB file in the Workbench by accident leads to an OutOfMemoryError and destabilizes the workbench or even makes it unusable for some time. It should be possible to define a maximum file size (in the Preferences, in plugin.xml or by computing available heap size) up to which an editor editor can open a file directly. If the user tries to open a file that exceeds the given size, the user should be shown a dialog which allows to * Open the file in spite of the warning * Open the file in an alternative (external) editor instead * Cancel the edit request Perhaps the best solution would be to extend the Editor API such that an editor is given the file path to be opened BEFORE it is actually opened. Thereby, each editor implementor could have policies for allowing or denying the open request. Policies for denying could be based on file size, virtual vs. real file system [See Bugzilla 106176], file content...) In case an edit request is denied, the Platform could show a dialog like "Editor FooBar has denied opening 'yourFile.xxx' with the following message: (Custom Message e.g. File is 50MB large) Do you want to * Open the file in (Dropdown of registered editors for xxx files) instead * Open the file in spite of the warning * Cancel "
Created attachment 194679 [details] Patch v01 Patch v01 for HEAD
At this point changes in 3.7 should be minimal, I suggest we go with the same patch as was done in 3.4.2+ (bug 344031 ). Note also reversal of the changes from Label to Text (bug 344031 comment 18 ) which seems to be missing from the proposed patch.
Created attachment 196027 [details] The "forward-ported" patch from 3.4.2
(In reply to comment #3) > Created attachment 196027 [details] > The "forward-ported" patch from 3.4.2 The patch looks okay to me and does the job with the least amount of code required. One thing I noticed was that it doesn't "work" if you change the value at runtime but I suppose we're just expecting people to use plugin_customization.ini to alter the values on startup and not for them to be randomly changing the value while the application is up. If one were to change this value, they would need to spawn new workbench windows (so that a new EditorManager would get instantiated) for the new value to kick in. I think it would be worth adding a @noreference tag to the "upgraded" STORE_ID_INTERNAL_EXTERNAL static field. While the text notes that it probably won't be referenced (as clients are not supposed to extend the class), it doesn't hurt to add the tag in my opinion though I admit to being unfamiliar with our API tagging policies. Lastly, I'm wondering if these changes will come up as API errors during the build? I know that SWT has to update the API exclusion list for bug 279461 but that is a case of making a public class @noextend while we are lifting the 'final' requirement and replacing it with a @noextend. Are similar rules going to kick in for our API tooling tags here?
Created attachment 196035 [details] The "forward-ported" patch from 3.4.2 updated (In reply to comment #4) > One thing I noticed was that it doesn't "work" if you change the value at > runtime but I suppose we're just expecting people to use > plugin_customization.ini to alter the values on startup and not for them to be > randomly changing the value while the application is up. That's correct. At present we do not expect the file size limit to change at runtime. > I think it would be worth adding a @noreference tag to the "upgraded" > STORE_ID_INTERNAL_EXTERNAL static field. Good point, I updated the patch. > Lastly, I'm wondering if these changes will come up as API errors during the > build? I checked with Olivier and he thinks that it should be fine for API tools. Of course there is always a chance that it won't :-).
Comment on attachment 196035 [details] The "forward-ported" patch from 3.4.2 updated Marking the attachment as a patch.
+1 for 3.7.RC2
Updated patch applied to CVS Head. Thank you!
Verified in I20110519-0800.
Oleg, it looks like this didn't make it into 4.x.
There is no EditorManager class in 4.2...
*** This bug has been marked as a duplicate of bug 117746 ***
Let's keep this as bug for the fix being in 3.7.x and 3.8.x.
