Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Visually distinguish fixes from non-fixes in lightbulb menu #132109

Closed
DanTup opened this issue Sep 2, 2021 · 3 comments
Closed

Visually distinguish fixes from non-fixes in lightbulb menu #132109

DanTup opened this issue Sep 2, 2021 · 3 comments
Assignees
@mjbvz @justschen
Labels
editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone
August 2022

Comments

@DanTup
Copy link
Contributor

DanTup commented Sep 2, 2021

When invoking the lightbulb menu on an error, the menu can include a mix of both fixes and non-fixes (eg. refactors) mixed together:

Screenshot 2021-09-02 at 10 21 36

It would be good if there was some visual separation or indication of which of these are fixes for the diagnostic and which are not. For example in Android Studio I see the fixes have lightbulbs and non-fixes have pencil/edit icons:

Screenshot 2021-09-02 at 10 25 09
@mjbvz mjbvz added the editor-code-actions Editor inplace actions (Ctrl + .) label Sep 2, 2021
@mjbvz mjbvz added the feature-request Request for new features or functionality label Sep 15, 2021
@mjbvz mjbvz added this to the Backlog milestone Sep 15, 2021
@mjbvz mjbvz modified the milestones: Backlog, August 2022 Aug 4, 2022
@kieferrm kieferrm mentioned this issue Aug 7, 2022
Closed
@justschen justschen mentioned this issue Aug 9, 2022
Merged
ssigwart pushed a commit to ssigwart/vscode that referenced this issue Aug 12, 2022
@justschen
Added Icons and Header Separators to Code Action Widget (ref microsof…
8e456bd 
…t#132109)

* added disabled hover

* code cleanup on disabled option hovers

* removed comments

* widget enabled by default

* code cleanup and fix on build

* clean up on css removed unused importants

* small patch for css rules

* minor refactor on codeactionitems

* fix on disabled option click

* fix on disabled option click

* added some icons but just temp

* added iconws and modified widget look

* added beginning logic for menu groupings

* looks pretty good for a menu wooo

* added headers to menu + removed extra text from option labels

* minor code cleanup on group filtering

* Refactoring on code action kind

* changed styling based on feedback

* code cleanup

* First couple of fixes on PR for code action kinds

* modified icons and refactoring

* removed extra push

* removed parsing and added code action kind for surround
@lgrammel
Copy link

lgrammel commented Aug 18, 2022 •
edited
Loading

Hey I just tried out the new menu in insiders and it looks great!

I was wondering if you would consider adding 2 code action classes that are I think quite common:

  • refactor.inline (Inline)
  • refactor.move (Move)

This would be super valuable for the extension that I'm developing (P42 JavaScript Assistant), but I think also for other extensions. Afaik the TS language server also has refactor.move actions for example.

@justschen justschen mentioned this issue Aug 18, 2022
Closed
@justschen
Copy link
Contributor

@lgrammel Do you think you can provide examples of inline and move options and what those labels say?

ATM, I know there is a Move to new file code action, and I just have it lumped under More Actions..., but if inline and move are sufficiently different, I'd like to provide them with their own codicons as well!

@lgrammel
Copy link

lgrammel commented Aug 19, 2022 •
edited
Loading

@justschen Here are some examples from "Refactoring, 2nd Edition" (Fowler) that could be relevant for language services from different languages:

Inline:

  • Inline Class
  • Inline Function
  • Inline Variable

Move:

  • Move Field
  • Move Function
  • Move Statement into Function
  • Move Statements to Callers

I would also consider "Pull Up / Push Down" refactorings to be "move" refactorings, e.g. "Pull Up Field".

P42 has several inline refactorings:

  • Inline variable
  • Inline return
  • Inline into template

and many move refactorings, e.g.

  • Move statement
  • Move field initialization into constructor
  • Move switch case clause
  • Move default value into parameter

@mjbvz mjbvz added the verification-needed Verification of issue is requested label Aug 23, 2022
@lramos15 lramos15 added the verified Verification succeeded label Aug 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mjbvz mjbvz

@justschen justschen

Labels
editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Milestone
August 2022
Development

No branches or pull requests
5 participants
@lgrammel @DanTup @lramos15 @mjbvz @justschen