Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Add command and code action to extract into function and local variable #2139

Merged
merged 13 commits into from
Mar 4, 2019

Conversation

aswinmprabhu
Copy link
Contributor

No description provided.

@msftclas
Copy link

msftclas commented Nov 22, 2018

CLA assistant check
All CLA requirements met.

@Nerzal
Copy link

Nerzal commented Jan 2, 2019

Thanks for the PR! This is one of my most wanted features :D

Any Updates on this?
As far as i see, only the OSX Build for go 1.11 failed. That shouldn't be a big issue?
and also a merge conflict needs to be fixed :)

@mdonnellyli
Copy link

Wondering what the status of this is?

@aswinmprabhu
Copy link
Contributor Author

aswinmprabhu commented Jan 5, 2019

This approach using godoctor has been tried before and was abandoned. But I guess it has gotten better and vim-go uses it. It works for the simple use cases that I tried. I am waiting for the maintainer to review it or give me some feedback.

@Nerzal
Copy link

Nerzal commented Feb 1, 2019

@ramya-rao-a sorry for the ping, but did u allready notice this pr? :)

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for the PR @aswinmprabhu!

My apologies for not taking a look at this earlier.

I have left a few comments based on just code review, please do take a look.

I'll try out the feature this week and get back here if I see any issues.

src/goDoctor.ts Outdated Show resolved Hide resolved
src/goDoctor.ts Outdated Show resolved Hide resolved
src/goDoctor.ts Outdated Show resolved Hide resolved
src/goDoctor.ts Outdated Show resolved Hide resolved
src/goRefactor.ts Show resolved Hide resolved
src/goDoctor.ts Outdated
return resolve('Could not find godoctor');
}
if (err) {
return resolve(`Could not extract function : \n\n${stderr}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use reject here instead of resolve to deal with errors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type Thenable<string> returned by runGoDoctor doesn't have a catch method to handle rejected promises.

src/goDoctor.ts Outdated Show resolved Hide resolved
src/goDoctor.ts Outdated Show resolved Hide resolved
@Nerzal
Copy link

Nerzal commented Feb 6, 2019

Sadly i'm really bad at JavaScript, so sorry for the dumb question.
Will these features be usable through a shortcut? Something like ctrl+r+m (refactor + method - for extract method) and ctrl+r+v (refactor+variable).
If not, how could I (or someone) make these actions assignable to shortcuts?

And btw: Thank u so much for reviewing ramya-rao-a 🥇

@ramya-rao-a
Copy link
Contributor

@Nerzal Yes, I believe all the shortcuts as described in https://code.visualstudio.com/docs/editor/refactoring should be usable.

@aswinmprabhu
Copy link
Contributor Author

Also the refactoring seems to extremely slow in decently large files. This does not seem to be a problem with the implementation, as the godoctor command if run from the terminal with the same args, takes the same amount of time as in the editor.

@helphi
Copy link

helphi commented Mar 1, 2019

strong desire !!!

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aswinmprabhu!

I have pushed a few more commits to make some changes and use diffs instead of overwriting the file on disk to allow undo.

Can you take a look and see if the feature still works as expected for you?

@aswinmprabhu
Copy link
Contributor Author

Works good. But why do you still have the -w option if you are using diffs.

This reverts commit 7edc8bf.
@ramya-rao-a
Copy link
Contributor

Looks like sometime in the past year, VS Code is able to do the undo feature even for the changes that were made directly to the file on disk. (It didn't do this when I was working a feature 2 years ago).

So we don't need to use the diffs after all. Reverting that commit

@aswinmprabhu
Copy link
Contributor Author

Ok. Cool. Tested it out again. Can confirm that undo works fine.

@ramya-rao-a ramya-rao-a merged commit fa94793 into microsoft:master Mar 4, 2019
@ramya-rao-a
Copy link
Contributor

The latest beta version (0.9.3-beta.2) of this extension has this feature. Please see Refactoring in VS Code on how you can use the refactor feature.

Thanks @aswinmprabhu!

@helphi
Copy link

helphi commented Mar 11, 2019

it seems that not support go mod.

@ramya-rao-a
Copy link
Contributor

Thanks for the observation @helphi
I have updated golang/go#24661 with a request to track godoctor for module support

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants