From ae721d3a6f49224ff11ea739ca58d92dfa024772 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 3 Aug 2015 16:03:15 -0700 Subject: [PATCH] respond to cyrus --- src/harness/fourslash.ts | 8 +++--- src/services/services.ts | 34 +++++++++++++++---------- src/services/utilities.ts | 40 ++++++++++++++++-------------- tests/cases/fourslash/fourslash.ts | 2 +- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index a65a6b21e38..c44fa9dbc09 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1950,20 +1950,20 @@ module FourSlash { return; } else { - this.raiseError(name + ' failed - expected no template but got {newText: \"' + actual.newText + '\" offsetInNewText: ' + actual.offsetInNewText + '}'); + this.raiseError(name + ' failed - expected no template but got {newText: \"' + actual.newText + '\" caretOffset: ' + actual.caretOffset + '}'); } } else { if (actual === undefined) { - this.raiseError(name + ' failed - expected the template {newText: \"' + actual.newText + '\" offsetInNewText: ' + actual.offsetInNewText + '} but got nothing instead'); + this.raiseError(name + ' failed - expected the template {newText: \"' + actual.newText + '\" caretOffset: ' + actual.caretOffset + '} but got nothing instead'); } if (actual.newText !== expected.newText) { this.raiseError(name + ' failed - expected insertion:\n' + expected.newText + '\nactual insertion:\n' + actual.newText); } - if (actual.offsetInNewText !== expected.offsetInNewText) { - this.raiseError(name + ' failed - expected offsetInNewText: ' + expected.offsetInNewText + ',\nactual offsetInNewText:' + actual.offsetInNewText); + if (actual.caretOffset !== expected.caretOffset) { + this.raiseError(name + ' failed - expected caretOffset: ' + expected.caretOffset + ',\nactual caretOffset:' + actual.caretOffset); } } } diff --git a/src/services/services.ts b/src/services/services.ts index 260f18e4630..b72ef7ca111 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1090,7 +1090,8 @@ namespace ts { export interface TextInsertion { newText: string; - offsetInNewText: number; + /** The position in newText the caret should point to after the insertion. */ + caretOffset: number; } export interface RenameLocation { @@ -6758,7 +6759,6 @@ namespace ts { function getIndentationAtPosition(fileName: string, position: number, editorOptions: EditorOptions) { let start = new Date().getTime(); let sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - log("getIndentationAtPosition: getCurrentSourceFile: " + (new Date().getTime() - start)); start = new Date().getTime(); @@ -6800,11 +6800,11 @@ namespace ts { * Valid positions are * * outside of comments, statements, and expressions, and * * preceding a function declaration. - * + * * Hosts should ideally check that: * * The line is all whitespace up to 'position' before performing the insertion. * * If the keystroke sequence "/\*\*" induced the call, we also check that the next - * non-whitespace character is '*', which (approximately) indicates whether we added + * non-whitespace character is '*', which (approximately) indicates whether we added * the second '*' to complete an existing (JSDoc) comment. * @param fileName The file in which to perform the check. * @param position The (character-indexed) position in the file where the check should @@ -6843,16 +6843,24 @@ namespace ts { let docParams = parameters.map((p, index) => indentationStr + " * @param " + (p.name.kind === SyntaxKind.Identifier ? (p.name).text : "param" + index.toString()) + newLine); - let result = - /* opening comment */ "/**" + newLine + - /* first line for function info */ indentationStr + " * " + newLine + - /* paramters */ docParams.reduce((prev, cur) => prev + cur, "") + - /* closing comment */ indentationStr + " */" + - /* newline if at decl start */ (tokenStart === position ? newLine + indentationStr : ""); - - let cursorOffset = /* "/**" */ 3 + /* newLine */ newLine.length + indentationStr.length + /* " * " */ 3; - return {newText: result, offsetInNewText: cursorOffset }; + // A doc comment consists of the following + // * The opening comment line + // * the first line (without a param) for the object's untagged info (this is also where the caret ends up) + // * the '@param'-tagged lines + // * TODO: other tags. + // * the closing comment line + // * if the caret was directly in front of the object, then we add an extra line and indentation. + let result = + "/**" + newLine + + indentationStr + " * " + newLine + + docParams.reduce((prev, cur) => prev + cur, "") + + indentationStr + " */" + + (tokenStart === position ? newLine + indentationStr : ""); + + let cursorOffset = "/**".length + newLine.length + indentationStr.length + " * ".length; + + return { newText: result, caretOffset: cursorOffset }; } function getTodoComments(fileName: string, descriptors: TodoCommentDescriptor[]): TodoComment[] { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 8b8cadfc7fe..36ae22ca46a 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -421,33 +421,32 @@ namespace ts { } export function isInComment(sourceFile: SourceFile, position: number) { - return isInCommentHelper(sourceFile, position, /*extraCheck*/ c => true); + return isInCommentHelper(sourceFile, position, /*predicate*/ c => true); } /** * Returns true if the cursor at position in sourceFile is within a comment that additionally - * satisfies extraCheck, and false otherwise. + * satisfies predicate, and false otherwise. */ - export function isInCommentHelper(sourceFile: SourceFile, position: number, - extraCheck: (c: CommentRange) => boolean): boolean { + export function isInCommentHelper(sourceFile: SourceFile, position: number, predicate: (c: CommentRange) => boolean): boolean { let token = getTokenAtPosition(sourceFile, position); if (token && position <= token.getStart()) { let commentRanges = getLeadingCommentRanges(sourceFile.text, token.pos); - + + // The end marker of a single-line comment does not include the newline character. + // In the following case, we are inside a comment (^ denotes the cursor position): + // + // // asdf ^\n + // + // But for multi-line comments, we don't want to be inside the comment in the following case: + // + // /* asdf */^ + // + // Internally, we represent the end of the comment at the newline and closing '/', respectively. return forEach(commentRanges, c => c.pos < position && - // The end marker of a single-line comment does not include the newline character. - // In the following case, we are inside a comment (^ denotes the cursor position): - // - // // asdf ^\n - // - // But for multi-line comments, we don't want to be inside the comment in the following case: - // - // /* asdf */^ - // - // Internally, we represent the end of the comment at the newline and closing '/', respectively. (c.kind == SyntaxKind.SingleLineCommentTrivia ? position <= c.end : position < c.end) && - extraCheck(c)); + predicate(c)); } return false; @@ -456,12 +455,15 @@ namespace ts { export function hasDocComment(sourceFile: SourceFile, position: number) { let token = getTokenAtPosition(sourceFile, position); - let JSDocPrefixRegex = /^\/\*\*\s*/; - // First, we have to see if this position actually landed in a comment. let commentRanges = getLeadingCommentRanges(sourceFile.text, token.pos); - return forEach(commentRanges, c => JSDocPrefixRegex.test(sourceFile.text.substring(c.pos, c.end))); + return forEach(commentRanges, c => jsDocPrefix); + + function jsDocPrefix(c: CommentRange): boolean { + var text = sourceFile.text; + return text.length >= c.pos + 3 && text[c.pos] === '/' && text[c.pos + 1] === '*' && text[c.pos + 2] === '*'; + } } function nodeHasTokens(n: Node): boolean { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index b778fb3fc24..465d84cb0da 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -379,7 +379,7 @@ module FourSlashInterface { } public DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean) { - FourSlash.currentTestState.verifyDocCommentTemplate(empty ? undefined : { newText: expectedText, offsetInNewText: expectedOffset }); + FourSlash.currentTestState.verifyDocCommentTemplate(empty ? undefined : { newText: expectedText, caretOffset: expectedOffset }); } public noDocCommentTemplate() {