Skip to content

Removed text truncation logic from Introduce Variable codeaction#21374

Merged
jinujoseph merged 1 commit intodotnet:masterfrom
zaytsev-victor:Fixed5258
Jun 9, 2018
Merged

Removed text truncation logic from Introduce Variable codeaction#21374
jinujoseph merged 1 commit intodotnet:masterfrom
zaytsev-victor:Fixed5258

Conversation

@zaytsev-victor
Copy link
Copy Markdown
Contributor

@zaytsev-victor zaytsev-victor commented Aug 8, 2017

Fixes #5258

@Pilchie Pilchie added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Sep 8, 2017
@Pilchie Pilchie requested a review from a team September 8, 2017 14:42
Copy link
Copy Markdown
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Assuming the UI is tested to ensure that everything looks OK, I think this is fine.

@sharwell sharwell changed the base branch from master to post-dev15.5-contrib November 9, 2017 23:01
@shyamnamboodiripad shyamnamboodiripad changed the base branch from post-dev15.5-contrib to master November 17, 2017 20:17
@jinujoseph jinujoseph closed this May 24, 2018
@jinujoseph jinujoseph reopened this May 24, 2018
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Interesting. THe link mentions that they also remove newlines and whatnot. So we could get rid of our ConvertToSingleLine helper in several cases.

@sharwell sharwell added this to the 15.8 milestone May 24, 2018
Copy link
Copy Markdown
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

LGTM as long as someone tries it locally with a long expression spanning multiple lines. Ideally that person adds a screenshot here of the before/after so it's clear what the change did.

@jinujoseph
Copy link
Copy Markdown
Contributor

@JieCarolHu to validate the change

@JieCarolHu
Copy link
Copy Markdown
Contributor

Verified it works as expected

image

@JieCarolHu JieCarolHu closed this Jun 8, 2018
@JieCarolHu JieCarolHu reopened this Jun 8, 2018
@JieCarolHu
Copy link
Copy Markdown
Contributor

@jinujoseph Could you approve this merge? Thanks

@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge for 15.8.Preview4

@jinujoseph jinujoseph merged commit 66a6aac into dotnet:master Jun 9, 2018
@jinujoseph
Copy link
Copy Markdown
Contributor

thanks zaytsev-victor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved to merge Area-IDE cla-already-signed Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants