Comment out a new line when return is pressed while the caret is within a // comment #48217

CyrusNajmabadi · 2020-10-01T17:17:03Z

Supersedes #40408

Fixes #38516

Mirror the string literal splitting logic to split single line comments when return is pressed.

UI for this looks like this:

(the second option is the new one).

…in a comment

…nition

…litCommentCommandHandler.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

…dHandler.CommentSplitter.vb Co-Authored-By: CyrusNajmabadi <[email protected]>

…ler.CommentSplitter.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

…ler.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

…ler.CommentSplitter.cs Co-authored-by: CyrusNajmabadi <[email protected]>

…to patch-38516

…mmentService.vb Co-authored-by: Sam Harwell <[email protected]>

sharwell · 2020-10-02T00:51:41Z

src/EditorFeatures/Core/Implementation/SplitComment/SplitCommentCommandHandler.cs

+            using var transaction = CaretPreservingEditTransaction.TryCreate(
+                EditorFeaturesResources.Split_comment, textView, _undoHistoryRegistry, _editorOperationsFactoryService);
+
+            subjectBuffer.Replace(
+                GetReplacementSpan(triviaLine, position),
+                GetReplacementText(textView, document, options, triviaLine, trivia, position));
+
+            transaction.Complete();


❔ Does this need to be on the main thread?

that is a good question. I observe no bad behavior doing things this way... but it is somewhat suspect. Wondering if @jasonmalinowski or @olegtk might know... i can rejigger this though just to be safe in the meantime.

sharwell

My review focused on tests. Another review of the implementation would be good.

ghost · 2020-10-02T20:01:49Z

Hello @CyrusNajmabadi!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost

Auto-approval

weitzhandler · 2020-10-03T22:17:44Z

src/EditorFeatures/CSharpTest/SplitComment/SplitCommentCommandHandlerTests.cs

+    public static void Main(string[] args) 
+    { 
+        //
+        //Comment


Shouldn't there be a space between slashes and Comment?

Suggested change

//Comment

// Comment

I took the policy that left point of the selection helped indicate how much space to consume.

davidwengier · 2020-10-04T22:32:04Z

src/EditorFeatures/Core/Implementation/SplitComment/SplitCommentCommandHandler.cs

+            while (replacementStart > triviaLine.Start && textSnapshot[replacementStart - 1] == ' ')
+                replacementStart--;
+
+            while (replacementEnd < triviaLine.End && textSnapshot[replacementEnd] == ' ')


Should this consume tabs too?

06needhamt and others added 30 commits December 16, 2019 12:08

Comment out a new line when return is pressed while the caret is with…

ef433f7

…in a comment

Remove Unused Variable

c602333

Add Tests

a8502a6

Fix Indentation Issue

f1bb7ba

Remove Abstract Comment Splitter and Add Abstract CommandHandler Defi…

1957373

…nition

Add More Tests

80e0396

Fix Broken Test

8a1169e

[WIP] Visual Basic Comment Splitter Implementation

9a71a5a

[WIP] VB Comment Splitter GetNoteToReplace Incorrect

b1f3c6f

Fix VB Comment Splitter

a2c724e

Implement VB Split Comment Test Worker

515c332

Add More Test Cases

8981612

Handle Line Continuation When Splitting Comments in VB

604709e

Update src/EditorFeatures/Core/Implementation/SplitComment/AbstractSp…

41d257f

…litCommentCommandHandler.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

Update src/EditorFeatures/VisualBasic/SplitComment/SplitCommentComman…

c6a856a

…dHandler.CommentSplitter.vb Co-Authored-By: CyrusNajmabadi <[email protected]>

Merge branch 'master' into patch-38516

304f5de

Fix Tests

83bb3da

Address most of the review feedback

79192ca

All Tests Now Pass

ab9f2b7

Merge Master

65ab354

Add Space after split comment if it was present on the original

f4abf65

Address ArrowCase's Comments

8dfcd54

Update src/EditorFeatures/CSharp/SplitComment/SplitCommentCommandHand…

b056134

…ler.CommentSplitter.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

Update src/EditorFeatures/CSharp/SplitComment/SplitCommentCommandHand…

b8277a5

…ler.cs Co-Authored-By: CyrusNajmabadi <[email protected]>

Update src/EditorFeatures/CSharp/SplitComment/SplitCommentCommandHand…

e05db8f

…ler.CommentSplitter.cs Co-authored-by: CyrusNajmabadi <[email protected]>

Merge Master

0db9999

Merge branch 'patch-38516' of https://github.com/06needhamt/roslyn in…

451045d

…to patch-38516

Fix Build

29bb7c8

Update Licences

3d7be65

Merge branch 'patch-38516' of https://github.com/06needhamt/roslyn in…

b2d427a

…to patch-38516

CyrusNajmabadi and others added 7 commits October 1, 2020 17:15

Remove partial

7d85da3

Parameterize test

fc04d01

Revert

47477f3

Align code with UI

a22430e

Update src/EditorFeatures/VisualBasic/SplitComment/VisualBasicSplitCo…

48bae5a

…mmentService.vb Co-authored-by: Sam Harwell <[email protected]>

Handle cases like ////

59b2a44

Add vb tests

8735f6a

CyrusNajmabadi requested a review from sharwell October 2, 2020 00:37

sharwell reviewed Oct 2, 2020

View reviewed changes

sharwell approved these changes Oct 2, 2020

View reviewed changes

CyrusNajmabadi added 5 commits October 1, 2020 17:52

Remove parameter

0b61756

use IsKind

271937e

Update buffer on UI thread

7a55545

Support feature when span is selected

4f8c243

Add test

ff77147

CyrusNajmabadi added the auto-merge label Oct 2, 2020

ghost approved these changes Oct 2, 2020

View reviewed changes

Move code

2c05b36

CyrusNajmabadi force-pushed the patch-38516 branch from 4ba8bb9 to 2c05b36 Compare October 3, 2020 05:03

jinujoseph added the Area-IDE label Oct 3, 2020

weitzhandler reviewed Oct 3, 2020

View reviewed changes

davidwengier reviewed Oct 4, 2020

View reviewed changes

davidwengier approved these changes Oct 4, 2020

View reviewed changes

ghost merged commit 1464851 into dotnet:master Oct 5, 2020

ghost added this to the Next milestone Oct 5, 2020

Cosifne modified the milestones: Next, 16.9.P1 Oct 12, 2020

sharwell mentioned this pull request Oct 13, 2020

Comment splitting doesn't account for manual comment splitting #48547

Closed

CyrusNajmabadi deleted the patch-38516 branch April 11, 2021 18:24

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comment out a new line when return is pressed while the caret is within a // comment #48217

Comment out a new line when return is pressed while the caret is within a // comment #48217

Uh oh!

CyrusNajmabadi commented Oct 1, 2020 •

edited

Loading

Uh oh!

sharwell Oct 2, 2020

Uh oh!

CyrusNajmabadi Oct 2, 2020

Uh oh!

sharwell left a comment

Uh oh!

ghost commented Oct 2, 2020

Uh oh!

ghost left a comment

Uh oh!

weitzhandler Oct 3, 2020

Uh oh!

CyrusNajmabadi Oct 4, 2020

Uh oh!

davidwengier Oct 4, 2020

Uh oh!

Uh oh!

Comment out a new line when return is pressed while the caret is within a // comment #48217

Comment out a new line when return is pressed while the caret is within a // comment #48217

Uh oh!

Conversation

CyrusNajmabadi commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sharwell Oct 2, 2020

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Oct 2, 2020

Choose a reason for hiding this comment

Uh oh!

sharwell left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 2, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

weitzhandler Oct 3, 2020

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Oct 4, 2020

Choose a reason for hiding this comment

Uh oh!

davidwengier Oct 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CyrusNajmabadi commented Oct 1, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.