Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support auto doc comment generation #4261

Merged
merged 5 commits into from
Dec 16, 2020

Conversation

333fred
Copy link
Member

@333fred 333fred commented Dec 4, 2020

Adds '\n' and '/' to the on-type format list, enabling vscode support for the newly-added omnisharp feature of auto-generating documentation comments. Closes #8.

Adds '\n' and '/' to the on-type format list, enabling vscode support for the newly-added omnisharp feature of auto-generating documentation comments. Closes dotnet#8.
@333fred
Copy link
Member Author

333fred commented Dec 4, 2020

@NTaylorMullen, I think the razor plugin will have to update whatever on-type formatting you have to also call on / and \n, since the C# extension itself does not get called back on razor files.

@kasecato, this exposes the support I recently added to omnisharp that calls the roslyn functionality for this. I expect that either you or we will get issues about duplicate comments being generated: this generation will only work if the user has on-type formatting turned on. I'm not sure what mechanism you're using, so it's possible that you will still have some functionality this does not (in addition to positioning the cursor in a better location, which this cannot do as it's using the on-type formatting mechanism which cannot control the cursor). But I wanted to give you a heads up before it was merged.


const onTypeFormatProviderCommand = 'vscode.executeFormatOnTypeProvider';

function normalizeNewlines(original: string): string {
Copy link
Member Author

Choose a reason for hiding this comment

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

node.js does not have replaceAll, unfortunately...

@kasecato
Copy link

kasecato commented Dec 4, 2020

@333fred This is a feature to output doccoment from AST, right? My extension is completely regular expression, so it was getting harder and harder to maintain... I've been wanting to get the AST from the roslyn for some time. I'll announce the removal of my extension if necessary.

@333fred
Copy link
Member Author

333fred commented Dec 4, 2020

@333fred This is a feature to output doccoment from AST, right? My extension is completely regular expression, so it was getting harder and harder to maintain... I've been wanting to get the AST from the roslyn for some time. I'll announce the removal of my extension if necessary.

Yep. It's using the same functionality that inserts the doc comments in VS itself.

@NTaylorMullen
Copy link
Contributor

@NTaylorMullen, I think the razor plugin will have to update whatever on-type formatting you have to also call on / and \n, since the C# extension itself does not get called back on razor files.

We don't currently support OnTypeFormatting in Razor's VSCode implementation so I'd imagine things would continue to work as is. Out of curiosity, how do you all control the cursor when generating doc comments? We've made it work in VS via a custom protocol message; however, that doesn't exist in VSCode AFAIK.

@333fred
Copy link
Member Author

333fred commented Dec 4, 2020

Out of curiosity, how do you all control the cursor when generating doc comments? We've made it work in VS via a custom protocol message; however, that doesn't exist in VSCode AFAIK.

That's the secret: we do not. It does mean that the cursor will be placed at the end of the generated comment, but I think that's a worthwhile tradeoff to not having the generation at all.

@NTaylorMullen
Copy link
Contributor

That's the secret: we do not. It does mean that the cursor will be placed at the end of the generated comment, but I think that's a worthwhile tradeoff to not having the generation at all.

Gotcha. Ya that's unfortunate in terms of parity but agree that it's better then nothing at all

@333fred
Copy link
Member Author

333fred commented Dec 12, 2020

@JoeRobich do you have any idea what is going on with travis here? I can't actually see any errors?

@JoeRobich
Copy link
Member

@333fred I hate how the gulp command swallows errors. You can get the error locally by running tsc -p ..

~/Source/omnisharp-vscode [auto-doc-gen +0 ~2 -0 !]> tsc -p .
test/integrationTests/documentationCommentAutoFormatting.integration.test.ts:6:1 - error TS6133: 'skip' is declared but its value is never read.

6 import { skip } from 'rxjs/operators';
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error.

@333fred
Copy link
Member Author

333fred commented Dec 15, 2020

Alright, now I believe this is just waiting for a new omnisharp-roslyn release.

@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #4261 (b69cf12) into master (9abb4de) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4261   +/-   ##
=======================================
  Coverage   85.99%   85.99%           
=======================================
  Files          60       60           
  Lines        1857     1857           
  Branches      215      215           
=======================================
  Hits         1597     1597           
  Misses        200      200           
  Partials       60       60           
Flag Coverage Δ
integration 100.00% <ø> (ø)
unit 85.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 687edc7...b69cf12. Read the comment docs.

@JoeRobich JoeRobich merged commit 54cc13e into dotnet:master Dec 16, 2020
@333fred 333fred deleted the doc-comment-formatting branch December 16, 2020 16:28
@vchirikov
Copy link

vchirikov commented Dec 17, 2020

It doesn't work on https://github.com/OmniSharp/omnisharp-vscode/releases/tag/v1.23.8
omnisharp-roslyn: 1.37.6-beta.3
p.s. I have snippets that start with //, maybe what's why. Can you try to check with this scenario?
cc: @333fred

"/// <inheritdoc>": {
	"scope": "csharp",
	"prefix": "//i",
	"body": [
		"/// <inheritdoc />",
	],
	"description": "Adds '<inheritdoc />'"
},

@333fred
Copy link
Member Author

333fred commented Dec 17, 2020

@vchirikov do you have on-type formatting turned on? This support comes via that mechanism.

@vchirikov
Copy link

@333fred
No, I have "editor.formatOnType": false,. It's not obliviously that this feature included in formatOnType, maybe better to change description in changelog at least.
So, I have to use formatOnType or use k--kato.docomment, isn't it? :(

@333fred
Copy link
Member Author

333fred commented Dec 21, 2020

So, I have to use formatOnType or use k--kato.docomment, isn't it?

Correct.

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

Successfully merging this pull request may close these issues.

Auto generate triple slash comments with parameters
5 participants