Skip to content

Plumb formatting to getDocCommentTemplateAtPosition #52349

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

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jan 21, 2023

Fixes #52348

This will fail at the moment because it (rightfully!) fails tests. The fourslash verify call expects the newlines to match the newlines in the source file's multi-line template, which in our repo is CRLF at checkout, but the format settings are:

export const testFormatSettings = getDefaultFormatCodeSettings("\n");

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 21, 2023
@jakebailey jakebailey marked this pull request as ready for review January 23, 2023 19:26
@jakebailey
Copy link
Member Author

Turns out this one fourslash call did its own normalization. Honestly these should be baselines or something but for now, just fix it.

@@ -604,7 +604,7 @@ export interface LanguageService {
getFormattingEditsForDocument(fileName: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[];
getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[];

getDocCommentTemplateAtPosition(fileName: string, position: number, options?: DocCommentTemplateOptions): TextInsertion | undefined;
getDocCommentTemplateAtPosition(fileName: string, position: number, options?: DocCommentTemplateOptions, formatOptions?: FormatCodeSettings): TextInsertion | undefined;
Copy link
Member Author

Choose a reason for hiding this comment

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

It's unfortunate that this has to be another optional parameter and not a required one (like other functions here), but I can't see a way to write an overloaded function that can actually detect which one is which.

@jakebailey jakebailey merged commit 1094188 into microsoft:main Jan 23, 2023
@jakebailey jakebailey deleted the fix-52348 branch January 23, 2023 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSDoc comment templates/scaffolding may not use formatter newlines
3 participants