Move Up/Down Code Actions about js-assistant HOT 49 OPEN

Move statements is available in v1.132.0:

Let me know what you think about the current keyboard shortcut setup or if you run into any bugs.

from js-assistant.

In general, I would expect move commands to be non-safe, so safety information is not very helpful for me for this code action.

showing the menu only when there are > 1 options (could still lead to friction)

I would always select the element closer to the cursor. Maybe there could be a second command that shows the context menu. Or even a setting.

from js-assistant.

Nice work!!

Top level declarations would also be super nice, as I often reorder classes to clean-up code.

Thanks! Top-level declarations should be moveable in v1.136.1 - are there any cases where it does not work?

from js-assistant.

Nice, it works perfectly!

from js-assistant.

Btw. you should reveal the location for the moved source. For large classes the moved text just disappears.

Thanks for letting me know - I just shipped v1.137.0, which reveals the (moved) text selection / cursor and adds "move switch case":

from js-assistant.

v1.139.0 supports moving array elements in destructuring expressions:

from js-assistant.

An animation would be cool - can VS Code do that?
Yes, the edit ranges and the selection ranges are used in the code action already.

For now, you could use a decoration to highlight the target location for a short period of time.

I'll hold off on the animation for now - while it might look cool at first glance and could make for good demos, I found that animations in areas of direct user interaction tend to get into the way in practice more often than not (e.g. by slowing the user down). When there is some built-in VSCode API that I can leverage I might look into prototyping / revisiting this.

How are you envisioning the decoration? Would it be shown after the edit is applied, or before (and delay the edit)?

Update: I've started experimenting a bit with this idea and I think I can come up with something that could work.

from js-assistant.

I would only show the decoration for the location where the code moved to, for about 300ms. Maybe you can even do some tricks with backgroundColor: `red; transition: all 0.5s ease-out, to get a nice fade out, but I don't know.

I don't think this animation will be annoying, it should be very subtle. But it will help to see where the text moved to.

from js-assistant.

Great idea, thanks for the writeup!

Regarding one command: I'd much rather leverage grouping through code action kind prefixes and have individual code actions - this gives users more flexibility configuring their keyboard shortcuts without losing anything ( see https://p42.ai/documentation/p42-for-vscode/editor-integration#keyboard-shortcuts for some examples).

Code action kind prefixes could be:

• "refactor.move.up.*"
• "refactor.move.down.*"
• (TBD) "refactor.move.swap.*"

This would allow grouping all move refactorings (incl. to different files in the future), grouping by up/down/swap, and mapping individual code actions.

One challenge is that sometimes several move up/down refactorings may be available for the same position (could be solved by configuring the shortcut to show a menu when there is > 1 action for a position).

from js-assistant.

have individual code actions

Maybe it makes sense to distinguish them, but I think discoverability is much more important than precision for refactorings.

ReSharper has one unified move down/up system, and it works flawlessly.

Having one unified system where always only one thing is applicable makes it much more predictable.
In my opinion, one universal tool provides much more value than many smaller (more precise) tools.

from js-assistant.

I don't understand how this would impact discoverability. There would be a single shortcut for "move up" and a single shortcut for "move down". The additional flexibility is just in case a user wants it, and there have to be separate implementations anyways because there are different matchers, transformations, safety evaluations, etc.

from js-assistant.

There would be a single shortcut for "move up" and a single shortcut for "move down"

Then you would need to use many context keys to compute upfront which of those "move up" commands wins effectively. Alternatively you would need a single dispatching move-up and move-down command (which would also enable changing the keybinding for all move commands at the same time).

from js-assistant.

There would be a single shortcut for "move up" and a single shortcut for "move down"

Then you would need to use many context keys to compute upfront which of those "move up" commands wins effectively. Alternatively you would need a single dispatching move-up and move-down command (which would also enable changing the keybinding for all move commands at the same time).

Maybe, but it could also depend on the activation ranges etc. I'll probably stick with the current system for now, since I have significant tooling around, and will see how far I get. I can always implement some prioritization algorithm when there is a need for it.

from js-assistant.

That is very cool! When invoking the shortcut, I wouldn't expect the context menu though.

from js-assistant.

That is very cool! When invoking the shortcut, I wouldn't expect the context menu though.

Yeah - I agree. I'm still trying to figure out how to best deal with the following 2 constraints:

• sometimes there are overlapping move refactorings (e.g. move stmt & move if-else-branch) for the same activation range. Could be solved by
  • prioritization algorithm - but e.g. it is unclear to me how move down branch from an if-statement would work in this case
  • showing the menu only when there are > 1 options (could still lead to friction)
• currently there is no good way of showing safety information without the context menu. Possible solutions:
  • show confirmation dialog if the refactoring would result in an error
  • show notification when a refactoring is directly invoked and there is a info/warning/error safety information

Would love to hear your thoughts, in particular if there are other options that I'm not considering.

from js-assistant.

I think some conflicts will be unavoidable if there is only up/down (e.g. move if branches conflicts with move statement). I'll introduce move left/right in addition to move up/down.

Move up/down

• Move Statements / Declarations
• Move Class Members
• Move JSX elements
• Move Case-Branches in Switch Statements
• TypeScript: move interface members, move type members, re-order union types, etc.

Move left/right

• Move Declarations inside multi-variable declaration
• Move Array Items (taking care of commas)
• Move Object Properties (taking care of commas)
• Move If/Else Branches #28
  #41
• Move expression in homogenous condition (a && |b && c -> |b && a && c) (note: overlap/replacement of 'flip operator')
• Move argument in function invocation (f(1, |2) -> f(|2, 1))
• Move Parameters (fixing all call-sites) (note: technically hard - requires multi-file, dataflow, etc.; more limited version possible)
• Move JSX attributes

This should reduce conflicts and still be fairly intuitive. I'll also remove the menu.

from js-assistant.

The keyboard shortcuts directly execute (when there is only 1 option for now) starting with v1.132.2:

from js-assistant.

Hi! how can I disable it? ctrl+alt+up/down is a very usefull hotkey to move code lines in any file type but now I'm getting this message when I press these commbination in files different to *.ts *.js

from js-assistant.

@serchserch you can disable it in the keyboard shortcuts panel when you record the "ctrl+alt+up+ keybinding:

Command Bar: >Preferences: Open Keyboard Shortcuts

Thanks for the report, I'll look into limiting the shortcut to the js/ts/vue suffixes.

from js-assistant.

@serchserch I've released v1.132.3, which limits keyboard shortcuts to supported files. Please let me know in case the JavaScript assistant still provides keyboard shortcuts for unsupported files.

from js-assistant.

Hi! how can I disable it? ctrl+alt+up/down is a very usefull hotkey to move code lines in any file type but now I'm getting this message when I press these commbination in files different to *.ts *.js

This is a good point - can you just move lines up/down in case there is no applicable refactoring?

from js-assistant.

Hi! how can I disable it? ctrl+alt+up/down is a very usefull hotkey to move code lines in any file type but now I'm getting this message when I press these commbination in files different to *.ts *.js

This is a good point - can you just move lines up/down in case there is no applicable refactoring?

Interesting idea - I'll think about it. My concern is that is might be confusing for the users and that they would often have another shortcut for this. I'll wait until the whole move system is further along to see if it would fit in naturally.

from js-assistant.

v1.133.0 supports moving JSX attributes:

from js-assistant.

I think for Windows, shift+alt+{up, down, left, right} might be better.
Ctrl+Alt+... is already used for multi-cursor and is quite important.

from js-assistant.

I think for Windows, shift+alt+{up, down, left, right} might be better. Ctrl+Alt+... is already used for multi-cursor and is quite important.

Thanks for letting me know - definitely need to change this for windows. Unfortunately "shift + alt + {direction}" is already taken as well on windows, and "ctrl + shift" is a standard for extending selections afaik.

Would "win + alt + {direction}" work for you? (I did not find that shortcut in the list)

from js-assistant.

shift+alt+{up, down, left, right}

I think duplicating lines up and down is probably not used by many, so I think it should be okay to overwrite that.

from js-assistant.

shift+alt+{up, down, left, right}

I think duplicating lines up and down is probably not used by many, so I think it should be okay to overwrite that.

Is there any downside to the win + alt + {direction} option that I'm missing? I'd rather not mess with any built-in shortcuts (even tho I agree not many people are likely to use this one).

from js-assistant.

win + alt + {direction} is already taken by the OS.
I think the "win" modified should be avoided for shortcuts.

from js-assistant.

win + alt + {direction} is already taken by the OS. I think the "win" modified should be avoided for shortcuts.

Hm that makes things more difficult. I wanted to use shift as a modifier for move into/out of operations, e.g. in the following situation:

|statement1();
{
  statement2();
}

Shortcut w/o shift: move statement down

{
  statement2();
}
|statement1();

Shortcut w/ shift: move into block:

{
  |statement1();
  statement2();
}

That way correct "barriers" would exist, but can easily be overcome by adding an additional modifier key.

Current alternatives I'm considering:

• shift+alt+{direction} (for move) and shift+alt+pgdown/up (for move into/out of)
  • disadvantage: not as easy to do move into/out of
  • disadvantage: conflicts with expand/shrink selection & copy lines
• move to a different area of the keyboard instead of the arrows, e.g. ijkl
  • disadvantage: hard to find on keyboard, other overlapping shortcuts

Since the conflict for shift+alt is also on expand/shrink selection, I'm torn between the 2 alternatives.

from js-assistant.

v1.134.0 supports moving array elements:

from js-assistant.

v1.135.0 supports moving object properties:

from js-assistant.

v1.136 supports moving class members:

from js-assistant.

Nice work!!

Top level declarations would also be super nice, as I often reorder classes to clean-up code.

from js-assistant.

Would you be able to track the text ranges before/after the move of the moved element?
Maybe we can do a cool animation

from js-assistant.

Would you be able to track the text ranges before/after the move of the moved element? Maybe we can do a cool animation

An animation would be cool - can VS Code do that?

Yes, the edit ranges and the selection ranges are used in the code action already.

from js-assistant.

can VS Code do that?

Not yet, but this would be a great use-case to expose that functionality to extension authors.
There is something internal that could be enhanced a bit to make it work.

from js-assistant.

can VS Code do that?

Not yet, but this would be a great use-case to expose that functionality to extension authors. There is something internal that could be enhanced a bit to make it work.

Would love to try it, for move refactorings this could be very useful to help the user understand what's going on.

Probably the animations would need to be able to turned off for power users (animations can easily slow down the user interactions, I almost always disable them when I can).

from js-assistant.

Btw. you should reveal the location for the moved source. For large classes the moved text just disappears.

from js-assistant.

v1.138.0 supports moving variable declarations:

from js-assistant.

An animation would be cool - can VS Code do that?
Yes, the edit ranges and the selection ranges are used in the code action already.

For now, you could use a decoration to highlight the target location for a short period of time.

from js-assistant.

I would only show the decoration for the location where the code moved to, for about 300ms. Maybe you can even do some tricks with backgroundColor: `red; transition: all 0.5s ease-out, to get a nice fade out, but I don't know.

I don't think this animation will be annoying, it should be very subtle. But it will help to see where the text moved to.

I agree, that is along the lines of what I'm implementing. If the CSS in the background color does not support animation (docs say only rgba is allowed), I might code the animation in JS.

from js-assistant.

Quick preview:

This looks nice, will clean it up and include it in the next release.

from js-assistant.

v1.139.1:

Thanks for the suggestion! Animations might have potential for other refactorings such as extract or inline as well.

from js-assistant.

Move object property in destructuring (v1.140.0):

from js-assistant.

Very nice!

from js-assistant.

I think for Windows, shift+alt+{up, down, left, right} might be better. Ctrl+Alt+... is already used for multi-cursor and is quite important.

v1.141 switches to "shift+alt+direction" on Windows/Linux.

from js-assistant.

v1.142 simplifies the shortcuts:

The activation depends on the cursor position. Less frequently used move operations that would conflict with others are now availalbe in the move menu (e.g. move if-else branches).

from js-assistant.

Move nested if statements in v1.143:

from js-assistant.

A quick update on why progress has been slow in the last few months: in order to get this right, I need to handle whitespace and comments differently, which requires some larger changes to the refactoring engine. I've put this on a lower priority for now, since many of the core parts already work, but I hope to get back to it in Q1/23.

from js-assistant.