Skip to content

Add a prompt for Extract local variable #4749

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.

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

Closed
lsaudon opened this issue Sep 20, 2023 · 10 comments
Closed

Add a prompt for Extract local variable #4749

lsaudon opened this issue Sep 20, 2023 · 10 comments

Comments

@lsaudon
Copy link

lsaudon commented Sep 20, 2023

Is your feature request related to a problem? Please describe.
I'd like to see a prompt for Extract local variable as well as for Extract method.

Describe the solution you'd like
Add a prompt after select Extract local variable.

Describe alternatives you've considered
After extract, trigger Rename symbol vscode.

@DanTup DanTup modified the milestones: Backlog, v3.74.0 Sep 21, 2023
@DanTup DanTup added the in editor Relates to code editing or language features label Sep 21, 2023
@DanTup DanTup closed this as completed in 56b30ec Sep 26, 2023
@DanTup
Copy link
Member

DanTup commented Oct 11, 2023

The way this was implemented (prompting for a name like Extract Method) has resulted in losing the server-generated name and I think that's made this much worse. I'm going to revert this for now, and try to find a better way to rename (such as triggering the rename automatically if it's possible).

Issue tracking the revert is #4786, and I'll re-open this to track a new solution.

@DanTup DanTup reopened this Oct 11, 2023
@DanTup DanTup modified the milestones: v3.74.0, v3.76.0 Oct 11, 2023
@lsaudon
Copy link
Author

lsaudon commented Oct 11, 2023

Also, why when I do this:

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(),
      ),
    );
  }

I can't extract AppBar() with name appBar ? I have this message The name 'appBar' is already used in the scope..
Likewise, I can rename with appBar after extract.

@DanTup
Copy link
Member

DanTup commented Oct 11, 2023

@lsaudon I'm not sure - could you file a new issue about that with exact instructions of what you're selecting and invoking? Thanks!

@DanTup DanTup modified the milestones: v3.76.0, v3.78.0 Oct 30, 2023
@DanTup DanTup modified the milestones: v3.78.0, v3.80.0 Nov 27, 2023
@DanTup DanTup modified the milestones: v3.80.0, v3.82.0 Dec 21, 2023
@DanTup DanTup modified the milestones: v3.82.0, On Deck Jan 25, 2024
@lsaudon lsaudon closed this as completed Jun 13, 2024
@DanTup
Copy link
Member

DanTup commented Jun 13, 2024

I believe this is still outstanding and worthwhile, is there a reason you closed it @lsaudon ?

@DanTup DanTup reopened this Jun 13, 2024
@lsaudon
Copy link
Author

lsaudon commented Jun 13, 2024

I've closed it because I can no longer reproduce the problem I had.
Before, when I extracted a value with the same name as the parameter.
I used to get this result.

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(),
      ),
    );
  }
// to 
  @override
  Widget build(BuildContext context) {
    final appBar2 = AppBar();
    return MaterialApp(
      home: Scaffold(
        appBar: appBar2,
      ),
    );
  }

I can no longer reproduce it.

@DanTup
Copy link
Member

DanTup commented Jun 13, 2024

@lsaudon what do you see now?

@lsaudon
Copy link
Author

lsaudon commented Jun 13, 2024

  @override
  Widget build(BuildContext context) {
    final appBar = AppBar();
    return MaterialApp(
      home: Scaffold(
        appBar: appBar,
      ),
    );
  }

@DanTup
Copy link
Member

DanTup commented Jun 17, 2024

Oh, was your complaint that it was generating a bad variable name, rather than not prompting for it in advance?

I would like to automatically trigger rename after generating, but there's no great way to do this right now within standard LSP.

@lsaudon
Copy link
Author

lsaudon commented Jun 18, 2024

Yes

@DanTup
Copy link
Member

DanTup commented Jun 18, 2024

Gotcha, thanks. In that case I'll re-close this, and we can hope to fix it by triggering the rename functionality directly after the refactor if VS Code/LSP get that capability.

@DanTup DanTup closed this as completed Jun 18, 2024
@DanTup DanTup removed this from the On Deck milestone Jun 18, 2024
@DanTup DanTup removed is enhancement in editor Relates to code editing or language features labels Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants