Skip to content

Allow creating doc comments around existing comments#59763

Merged
davidwengier merged 5 commits intodotnet:mainfrom
davidwengier:AllowExistingCommentsInDocComments
Mar 16, 2022
Merged

Allow creating doc comments around existing comments#59763
davidwengier merged 5 commits intodotnet:mainfrom
davidwengier:AllowExistingCommentsInDocComments

Conversation

@davidwengier
Copy link
Copy Markdown
Member

Maybe there is a reason this wasn't supported, and it's a bad idea, but its something that has bit me before because I'm very very silly, and since I saw someone else fall victim to it recently, I thought I'd see how you all feel.

Basically sometimes I write normal comments above my methods, and then on a PR someone tells me to make them XML comments, so I add a /, commit and push. And then the build fails because I forgot that XML comments aren't that simple. This fixes that.

Looks like this:
DocCommentsWithComments

@davidwengier davidwengier requested a review from a team as a code owner February 25, 2022 05:14
@ghost ghost added the Area-IDE label Feb 25, 2022
string.Format("Caret positioned incorrectly. Should have been {0}, but was {1}.", expectedPosition, endCaretPosition));
var actualWithCaret = actual.Insert(endCaretPosition, "$$");

Assert.Equal(expectedMarkup, actualWithCaret);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This makes it easier to reason about the caret position failure

protected override bool IsEndOfLineTrivia(SyntaxTrivia trivia)
=> trivia.RawKind == (int)SyntaxKind.EndOfLineTrivia;

protected override bool IsSingleExteriorTrivia(DocumentationCommentTriviaSyntax documentationComment, bool allowWhitespace = false)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nothing ever passed in a value for allowWhitespace

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Maybe there is a reason this wasn't supported, and it's a bad idea

Seems like a great idea to me :)

Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

can we do the same for VB?

@davidwengier
Copy link
Copy Markdown
Member Author

can we do the same for VB?

I tried, but VB seemed to be surprisingly different as far as parsing doc comments go, so I need to spend more time on that.

Good to at least get the idea validated.

@davidwengier davidwengier enabled auto-merge (squash) March 15, 2022 23:40
@davidwengier
Copy link
Copy Markdown
Member Author

/azp run roslyn-integration-CI

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@davidwengier davidwengier merged commit 4e83075 into dotnet:main Mar 16, 2022
@ghost ghost added this to the Next milestone Mar 16, 2022
@davidwengier davidwengier deleted the AllowExistingCommentsInDocComments branch March 16, 2022 07:22
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants