Community
Participate
Working Groups
[1.7][quick fix] Need 'Surround with multi-try/catch'.
+1. I'd call it 'Surround with try/multi-catch'.
I was going to file the same request.
Created attachment 198959 [details] fix v0.9 The patch adds a new action to 'Surround With' menu - 'Try/multi-catch Block'. The existing 'Try/catch Block' action is untouched. Markus, please try out the patch. Is the behavior ok ? TODO: - Clean up javadoc - Prevent/Disable the action for compliance level < 1.7 - Add tests
Looks good so far. More TODO: - make the protected methods in the action classes package-private. - SurroundWithTryMultiCatchAction#fEditor is unused - 'Try/multi-catch Block' needs a different mnemonic - SurroundWithTryCatchRefactoring: - getCatchBody("Exception", ..) //TODO -> OK for me. I wouldn't go into the LUB business. Alternative would be to spell out the union type as 'Exception1 | Ex2 | Ex3'. - instead of: String type= fImportRewrite.addImport(exception, context); better do this: Type type= fImportRewrite.addImport(exception, getAST(), context); After playing with the example below, I think it would be good to have a quick fix 'Surround with try/multi-catch' that shows up in addition to the 'Surround with try/catch' quick fix (only for source >= 1.7 and only if there are multiple exceptions). void xxx() /*throws Exception*/ { getClass().getDeclaredMethod("xxx").invoke(this, (Object[]) null); }
Created attachment 198992 [details] fix + tests Finished all the TODOs and committed to BETA_JAVA7 (In reply to comment #4) > After playing with the example below, I think it would be good to have a quick > fix 'Surround with try/multi-catch' that shows up in addition to the 'Surround > with try/catch' quick fix (only for source >= 1.7 and only if there are > multiple exceptions). > > void xxx() /*throws Exception*/ { > getClass().getDeclaredMethod("xxx").invoke(this, (Object[]) null); > } Yeah, makes sense. I will add this.
Created attachment 198994 [details] fix + tests - II Added the quick fix as well.
.
I think the "Surround with try/multi-catch" quick fix should have higher priority (7), since this is probably used more often than the other one. The "Source > Surround With > Try/multi-catch Block" menu item should not be shown for source level < 1.7. It's OK to show the message for pre-1.7 projects (e.g. when invoked via key binding), but you could just as well disable the action in that case.
(In reply to comment #8) Done.
Created attachment 199133 [details] Fix III (enablement) Overriding isEnabled() in SurroundWithTryMultiCatchAction is not good. This could be a performance bottleneck (called too often), and the action is supposed to fire property change events when the enablement changes. Released the attached fix.
(In reply to comment #8) > I think the "Surround with try/multi-catch" quick fix should have higher > priority (7), since this is probably used more often than the other one. > "Surround with try/multi-catch" is being proposed after "Surround with try/catch". Am I missing something?
(In reply to comment #11) > (In reply to comment #8) > > I think the "Surround with try/multi-catch" quick fix should have higher > > priority (7), since this is probably used more often than the other one. > > > "Surround with try/multi-catch" is being proposed after "Surround with > try/catch". Am I missing something? Thanks Deepak for explaining me. I was looking at the "Surround with" context menu and the order is not important there.
Verified using patch 1.0.0.v20110714-1400
This does not work for me using 3.7 + Paste this: import java.nio.file.AccessDeniedException; import java.nio.file.FileAlreadyExistsException; public class Test { void f(int i) { if (i == 1) throw new FileAlreadyExistsException(""); else throw new AccessDeniedException(""); } } ==> I get errors but none of them offers a 'Surround with Try/multi-catch Block' quick fix.
Dani, please try with this ---------------------------------------------------------------------------- void xxx() /* throws Exception */{ getClass().getDeclaredMethod("xxx").invoke(this, (Object[]) null); } ---------------------------------------------------------------------------- (In reply to comment #14) > ==> I get errors but none of them offers a 'Surround with Try/multi-catch > Block' quick fix. There are 2 errors on 2 different statements and both errors are for a single uncaught exception. The quick fix is offered if there is more than one uncaught exception in a single statement. You could argue that the quick fix can be smarter and try to find if there are uncaught exceptions in the nearby statements. But I don't think this is a good idea for 2 reasons - We will also have to do this for 'Surround with Try/multi-catch'. But this quick fix has been around for some time, and people are used to the current behavior. - What if the developer wants the current behavior i.e. wants to surround only one statement with a try catch block. The correct solution in this case is for the developer to use 'Source > Surround with > Try/multi-catch block' :)
(In reply to comment #15) Point taken!
