Swapping if/else does not work about js-assistant HOT 8 CLOSED

Yes, it used to. However, I found it hard to remember when to use left/right vs up/down, and there were conflicts with existing shortcuts (expand/shrink selection). Therefore I've changed most move actions to up/down and removed left/right. In cases where there is a conflict (here: move statement vs move if branch), the less common action (here: move if branch) is only accessible through the move context menu (or the refactor context menu). You can access it with Ctrl+Alt+m on Windows.

It seems you have custom shortcuts set up tho? Maybe there is a good way to provide code action kinds that would work for such custom setups 🤔

from js-assistant.

It seems you have custom shortcuts set up tho? Maybe there is a good way to provide code action kinds that would work for such custom setups 🤔

Just removed them.

Therefore I've changed most move actions to up/down and removed left/right.

I like that!

In cases where there is a conflict (here: move statement vs move if branch), the less common action (here: move if branch) is only accessible through the move context menu (or the refactor context menu). You can access it with Ctrl+Alt+m on Windows.

I would use anchors to distinguish between these cases.
E.g. when the cursor is no the else or if, it should swap branches. Otherwise the statement.

from js-assistant.

I would use anchors to distinguish between these cases. E.g. when the cursor is no the else or if, it should swap branches. Otherwise the statement.

I was thinking the same and first prototyped enabling move up/down from the else keyword. That leads to issues with the inverse action, so really the branch switch also needs to be activated on the if as well. However, this conflicts with move statement, because you can move all other statements at the beginning of the statement. I'm sure there are ways around it with different activation ranges (i.e. cursor positions / selections), but they are not necessarily obvious or easy to learn imo.

My current take is that learnability is one of the main challenges with the JavaScript Assistant, and I want to make it easier by having fairly simple systems for common cases (move up/down) and then context menus for different intents (e.g. move, extract, convert) with more specialized actions. While different activation ranges are part of it, I try not to make them too complex - otherwise it gets hard to find/activate the right code assist.

I'm open to changing this of course if I get more feedback or additional functionality like move in / move out requires it.

from js-assistant.

Fair points. What about using the if/else-if condition as anchor?

from js-assistant.

That could work - re-opening the issue. The one conflict I could see is with Move expression in homogenous condition (a && |b && c -> |b && a && c) (note: overlap/replacement of 'flip operator'), so I'll wait until that is available and then I'll experiment to see how activating on the condition would work.

from js-assistant.

Hmm, it is a bummer that this still does not work:

I would say reordering is the most valuable feature that p42 offers, but it often does not work :(

So I have to do the reordering of the if branches manually again...

from js-assistant.

Hey thanks for bringing this to my attention. The functionality is there, but it is currently hidden in the refactor menu. I'll look into a better key mapping this week, now that things have settled a bit around the shortcuts and the code action menu.

My current thinking on the anchors:

• use the content of the condition as anchor for moving if/else blocks -- other anchors are used form moving statements
• for moving parts of || conditions, require selection (can easily be smart selected) to enable move shortcuts (will come later)

from js-assistant.

Check out v1.157.1 - the move if-else up/down can now be triggered on the condition. Outside of the condition, a regular move statement is triggered (to enable both operations to be easily accessible).

from js-assistant.