Community
Participate
Working Groups
From bug 358384 comment 2. > Note that for these quick assists to work the names of the properties file and > the accessor class have to be similar e.g. messages.properties and > Messages.java. Dissimilar names like abc.properties and XYZ.java will not > work. This is similar to the behavior when a properties file is selected and > 'Find broken externalized strings' action is invoked. This is because there is > no information in the properties file about the accessor class. (I can try to > do a search for accessor classes and then inspect it to find the correct match, > but this may affect the responsiveness of the quick assists.) Possible solution - when we open the editor we start a job that finds the corresponding message class in the background.
Unfortunately I was unable to finish work on this.
Created attachment 209900 [details] fix+test The patch should be good to go in terms of functionality. However, Dani can you please review the change in PropertiesFileEditor.doSetInput(IEditorInput), I am not 100% sure if this is the right place to invoke the new job.
(In reply to comment #2) > Created attachment 209900 [details] [diff] > fix+test I tested this patch with o.e.jdt.ui source, more specifically with /org.eclipse.jdt.ui/ui refactoring/org/eclipse/jdt/internal/ui/refactoring/refactoringui.properties (The corresponding accessor class is RefactoringMessages.java). In this case 54 compilation units, in the same package, are inspected to be a match for refactoringui.properties. The patch works without any issues. => It should be safe to release this for M5.
(In reply to comment #2) > Created attachment 209900 [details] [diff] > fix+test > > The patch should be good to go in terms of functionality. However, Dani can you > please review the change in PropertiesFileEditor.doSetInput(IEditorInput), I am > not 100% sure if this is the right place to invoke the new job. The location is good.
(In reply to comment #3) > (In reply to comment #2) > > Created attachment 209900 [details] [diff] [diff] > > fix+test > > I tested this patch with o.e.jdt.ui source, more specifically with > /org.eclipse.jdt.ui/ui > refactoring/org/eclipse/jdt/internal/ui/refactoring/refactoringui.properties > (The corresponding accessor class is RefactoringMessages.java). In this case 54 > compilation units, in the same package, are inspected to be a match for > refactoringui.properties. The patch works without any issues. > => It should be safe to release this for M5. I disagree ;-) 1. Create 'p.properties' inside 'src' ==> CCE. People also use the Properties File editor outside package and even for files outside a Java project. 2. Jobs don't run sequentially. If the editor is reused and the first job takes very long, then it might overwrite the result of the second job. 3. The job runs / does work even if the editor got closed before the job started 4. Pending jobs aren't stopped when the editor is closed or the input changed. Copyright dates also need an update.
(In reply to comment #5) Alright, M6 it is.
Created attachment 210112 [details] fix+test This patch should fix all the problems mentioned in comment 5.
(In reply to comment #7) > Created attachment 210112 [details] [diff] > fix+test > > This patch should fix all the problems mentioned in comment 5. Better, but still: - A job is not a cheap resource. We should not schedule it if we already know that it's not computing something because the input is not an IFile. - The job is now correctly cancelled but its implementation does not check the progress monitor for being cancelled: it simply computes everything as if nothing had happened. - The sequence problem got fixed but there is still a window where 'fAccessorType' can be wrong. This window can be relatively big if the second job is not scheduled fast enough.
(In reply to comment #8) Fixed in master - 3ebdf20161a85b3bd1ec911e75b24c1c9fbb7fa6 I think the fix should be good now. (I pushed the change to avoid the 'create patch apply patch' dance.)
Sorry Deepak, but your code is still not safe. Take another look. Ping me if you don't see the hole. Little issue: please fix the empty catch block.
38a9f9250543744fc04ae88b4dec86d9a2243b6b That should do it.
Looks good now.
