Skip to content

Expand extractSymbol ranges #42770

New issue

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

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

Already on GitHub? # to your account

Merged
merged 6 commits into from
Feb 17, 2021

Conversation

jessetrinity
Copy link
Contributor

@jessetrinity jessetrinity commented Feb 11, 2021

Fixes #41469

When a refactor is invoked through a keyboard command we search a bit harder for the extract symbol refactor by treating the selected span as though it covers the tokens containing the start and end tokens.

That is, selecting

const foo = ba[|r + b|]az;

will be equivalent to selecting

const foo = [|bar + baz|];

for the purposes of offering the refactor to extract bar + baz.

I also separated out the unit tests into individual tests so we can tell them apart on failure.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Feb 11, 2021
@jessetrinity jessetrinity changed the title Expand extract ranges Expand extract symbol ranges Feb 11, 2021
@jessetrinity jessetrinity changed the title Expand extract symbol ranges Expand extractSymbol ranges Feb 11, 2021
@jessetrinity
Copy link
Contributor Author

cc @mjbvz

@amcasey
Copy link
Member

amcasey commented Feb 16, 2021

Does this resolve #41469 or is it just related? That issue seems to be about doing this for all refactorings.

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

Mostly questions.

@jessetrinity
Copy link
Contributor Author

jessetrinity commented Feb 16, 2021

I expect that #41469 only relates to extractSymbol. Other refactorings generally only use the start of the span to search for an applicable refactor, so the "explicit cursor request" from #38378 should suffice. extractSymbol relies more on the start and end nodes both being in the right places so it needed special logic to "try harder".

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

Thanks!

@jessetrinity jessetrinity merged commit d640313 into microsoft:master Feb 17, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not seeing refactorings try to intelligently expand based on the current selection
3 participants