Community
Participate
Working Groups
In order to use "Refactor -> Extract local variable", I have to place the cursor at the end of a statement or select it. Additionally, I would like this functionality to be also available when the cursor is placed within a statement. Thus, working mainly with the keyboard would be more efficient. For instance: -------------------- List<String> names = Arrays.asList("Harry", "Mary", "Lars"); System.out.println(names.size()); -------------------- The statement "names.size()" should be extracted when I place the cursor anywhere within "size" and press the shortcut for "Refactor -> Extract local variable". Extended version: An extended version of this could also suggest (in e.g. a popup) all parts of a combined statement which are available for extraction before extracting the part. For instance: -------------------- System.out.println(names.get(0).length()); -------------------- Placing the cursor within "get" should suggest "names.get(0)" and "names.get(0).length()".
You might want to give the 'Expand Selection To' actions a try. I usually use Alt+Shift+Up to select the desired range before invoking the refactoring. I find this less intrusive than a popup where I have to choose from a list.
Cool, the hard part is already done! All that's needed is to iteratively try each expanded selection before giving up, restoring the original selection and showing an error message, and doing that for all refactorings. And probably need to add special case treatment for semicolons if there's nothing but whitespace right of the caret.
Using Alt+Shift+Up to expand the selection is a good idea. Thanks for the suggestion. However, I still think that using Alt+Shift+Up (with Up multiple times) plus Ctrl+1 for Quick Assist (which shows a popup menu from which I have to choose using Up or Down) plus the enter key are a lot of keys to press. Additionally, I have to move my hands to different positions on the keyboard and twist them quite much when using the default shortcuts. It is a bit better when using the "Refactor -> Extract local variable" (Alt+Shift+L) since two keys are identical but still it does not feel good for me. I have to look too often at the keyboard to find the correct keys and move and twist my hands very much. When pressing just one shortcut and afterwards using only the keys Up and Down or Enter, I have to look only once (for the shortcut) on the keyboard and move and twist my hands only slightly. (I can find and press the keys Up, Down and Enter without looking at the keyboard.) Using a popup menu to suggest the available valid statements (that is without statements returning void) might be a bit more intrusive but I do not think it would disturb most users. For instance, Quick Assist uses a popup menu but I doubt that users complain about that. Additionally, I would limit the cases when the popup menu appears. For example: -------------------- List<String> names = Arrays.asList("Harry", "Mary", "Lars"); // line 1 System.out.println(names.size()); // line 2 System.out.println(names.get(0).isEmpty()); // line 3 -------------------- When placing the cursor within 1. "names" of line 2, I would suggest "names" and "names.size()" in the popup menu. 2. "size" of line 2, I would not show a popup menu but extract the statement "names.size()" to a variable. 3. "names" of line 3, I would suggest "names", "names.get(0)" and "names.get(0).isEmpty()" in the popup menu. 4. "get" of line 3, I would suggest "names.get(0)" and "names.get(0).isEmpty()" in the popup menu. 5. "isEmpty" of line 3, I would not show a popup menu but extract the statement "names.get(0).isEmpty()" to a variable. Since most statements are quite short like line 2 (especially when applying clean code principles), the popup would hardly ever show up. If you do not like the idea of the popup menu at all, would it at least be possible to extract the statement where the cursor is placed? That is: When placing the cursor within 1. "names" of line 2, "names" is extracted to a local variable. (not sensible but consistent) 2. "size" of line 2, "names.size()" is extracted to a local variable. 3. "names" of line 3, "names" is extracted to a local variable. 4. "get" of line 3, "names.get(0)" is extracted to a local variable. 5. "isEmpty" of line 3, "names.get(0).isEmpty()" is extracted to a local variable. This variant would just be an additional feature and not disrupt your current kind of writing code but might help others like me a lot.
Changing commands which can popup a UI is difficult, because the check for a valid selection happens deep in UI helper classes far removed in space and time from where the selection was decided. Plus, it would need more UI or it wouldn't feel right. However, changing Quick Assist versions which don't have UI is a lot simpler and amounts to trying several AST nodes in a loop instead of only one. I have a rough implementation which I need to acid test a bit to make sure it isn't missing any essential parts. One thing I already found and added was with Quick Assist - Inline local variable, the counterpart to Quick Assist - Extract local variable (replace all occurrences). It now needed to work from anywhere within a local variable declaration, not just from a variable's name. In the meantime, binding Quick Assist's extract and inline commands to Ctrl+ the 2 keys right of the L-key (; and ' in US layout, ö and ä in Finnish) feels quite comfortable.
Pushed to Gerrit here https://git.eclipse.org/r/24326 This patch changes: - Quick Assist processing to be aware of which command should be executed and using this information to try bigger enclosing AST nodes until a matching proposal is found or further expansion is not possible. - Inline local variable works from more AST nodes to match the increased flexibility of extracting local variables. - Introduces keybindings for: Ctrl-Alt-Shift-Left Arrow does Extract local variable (this occurrance) Ctrl-Alt-Left Arrow does Extract local variable (all occurrances) Ctrl-Alt-Right Arrow does Inline local variable (all occurrances) (There is no corresponding action for Ctrl-Alt-Shift-Right Arrow) I suggested other key bindings earlier but these new key bindings complement the existing Ctrl-Alt-Up/Down Arrow bindings quite nicely and most importantly allow a lot of refactorings without having to move your hands at all. Here's some example code to get a feel of this new functionality. Please note its perhaps surprising accuracy, for example in the last method when invoked next to '+' and next to 'h'. This is definitely a power user feature: class LetsSeeItWorking { static void methods() { int i = go(4), j = go(5), h = 6; int m = here(i, j, h) + here(go(1), go(2), go(3)); here(m, go(2), go(3)); } static int go(int i) { return i; } static int here(int i, int j, int h) { return i + j + h; } }
Can I take this? Would that speed things along?
I've made a little GIF animation about this feature: https://github.com/Overruler/eclipse.jdt.ui#refactoring-on-steroids
I have taken this, which actually did speed things along. The changes fixing this bug are included in Evening-IDE, which is available here: http://overruler.github.io/Evening-IDE/
(In reply to Timo Kinnunen from comment #8) > I have taken this, which actually did speed things along. The changes fixing > this bug are included in Evening-IDE, which is available here: > http://overruler.github.io/Evening-IDE/ Can you upload a Gerrit review for the change? I don't think JDT developers will search for the code at Github.
(In reply to comment #9) > Can you upload a Gerrit review for the change? I don't think JDT developers will > search for the code at Github. Please see comment 5 :)
(In reply to Timo Kinnunen from comment #5) > Pushed to Gerrit here https://git.eclipse.org/r/24326 Thanks, but can you please provide a patch that just fixes this bug? If you want to change/fix other parts, a new bug should be opened.
