Provide Quick Fix for Inline Variable/Function about abracadabra HOT 9 CLOSED

I don't really have an opinion, I usually press F1 to use a refactoring (I need mouse for quick fixes, and I'm lazy ^^ (and yes, I probably just missed the quickfix shortcut))

To be less opinated, we could add this in the config, so every refactoring can be a quickfix or not. So the user can have the wanted noise that he want.

from abracadabra.

Enabled by default seams reasonable 👌 I will talk about this with @nicoespeon tomorrow 😉

from abracadabra.

To be honest, current UX was inspired from Webstorm. The rationale is that any variable could be inlined, so that would add a lot of "noise". Instead, it's a first-class refactoring that has a dedicated keyboard shortcut.

I understand the need for discovery though. But I wonder if that's something that wouldn't be too noisy.

What do you think @fabien0102 ? @visusnet ?

from abracadabra.

I understand this reasoning. AndroidStudio doesn't offer it as a Quick Fix (or whatever it's called) either, contrary to what I remember. On the other hand, the split declaration and initialization refactoring is already provided at each variable declaration, so you'll get a "Show fixes" button anyway.

There's also the Refactoring menu (CTRL + Shift + R) which appears to have slightly different entries. Perhaps it would be more appropriate there. Currently it's not included in there either. I feel that this refactoring should be at least available in the refactoring menu, but I don't know how much control we have about that.

from abracadabra.

@nicoespeon I‘m kinda biased because I have been using IntelliJ for years and I am used to the special binding. I personally would not need it in the quick action menu, however discoverability is a good reason to have it. I generally think that decreasing cognitive load is important for good UX. This includes the number of shortcuts that are necessary in your memory. I would vote for having both, the quick action (mostly for people who don‘t have an IDEA bias) and the special keyboard shortcut (for people who are already used to it from other IDEs).

from abracadabra.

@fabien0102 What would the default be? I guess everything should be enabled by default and opt-out for more experienced people.

from abracadabra.

I was thinking about providing a config to disable it too. I also think that will help discoverability indeed.

As Fabien said, we'll discuss that tomorrow. Thanks for your inputs, that's helpful 😃

from abracadabra.

Hey @automatensalat 👋

So we dig into this with @fabien0102 and we've decided that we won't do it, but I'm open to PRs to implement that.

The rationale:

• we don't have a strong motivation to do it
  • we think it's ok to expect people who install the extension to read the docs and learn the shortcuts (unlike native editor features)
• we understand the benefits it could bring (discoverability)
  • it would be OK to propose these as Quick Fixes if we give a boolean option to deactivate that for users who prefer to use the shortcut only
• to make it work, we should first refator the code and it's not a trivial task
  • the refactorings should expose an Action Provider, using the createVisitor() pattern like the others
  • there's a bit of added complexity since 2 refactorings are exposed as one ("Inline Variable or Function")
  • it's totally do-able, just non-trivial
• we want to focus on other features like
  • improving "Move Statements" UX
  • finishing the "Extract Generic Type" refactoring
  • implement Hocus Pocus' inference

Therefore, it doesn't worth the effort for us, but I will review and accept a PR that implements it 👍

In the meantime, I'm closing the issue as I don't expect it to be addressed. If someone wants to tackle it, you can open a draft PR to discuss the details.

from abracadabra.

That's alright. It's just a minor issue and I can see your points. Thanks for thinking about this though 👍

from abracadabra.