Skip to content

Commit ae721d3

Browse files
author
Arthur Ozga
committed
respond to cyrus
1 parent 9d26b4e commit ae721d3

4 files changed

Lines changed: 47 additions & 37 deletions

File tree

‎src/harness/fourslash.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,20 +1950,20 @@ module FourSlash {
19501950
return;
19511951
}
19521952
else {
1953-
this.raiseError(name + ' failed - expected no template but got {newText: \"' + actual.newText + '\" offsetInNewText: ' + actual.offsetInNewText + '}');
1953+
this.raiseError(name + ' failed - expected no template but got {newText: \"' + actual.newText + '\" caretOffset: ' + actual.caretOffset + '}');
19541954
}
19551955
}
19561956
else {
19571957
if (actual === undefined) {
1958-
this.raiseError(name + ' failed - expected the template {newText: \"' + actual.newText + '\" offsetInNewText: ' + actual.offsetInNewText + '} but got nothing instead');
1958+
this.raiseError(name + ' failed - expected the template {newText: \"' + actual.newText + '\" caretOffset: ' + actual.caretOffset + '} but got nothing instead');
19591959
}
19601960

19611961
if (actual.newText !== expected.newText) {
19621962
this.raiseError(name + ' failed - expected insertion:\n' + expected.newText + '\nactual insertion:\n' + actual.newText);
19631963
}
19641964

1965-
if (actual.offsetInNewText !== expected.offsetInNewText) {
1966-
this.raiseError(name + ' failed - expected offsetInNewText: ' + expected.offsetInNewText + ',\nactual offsetInNewText:' + actual.offsetInNewText);
1965+
if (actual.caretOffset !== expected.caretOffset) {
1966+
this.raiseError(name + ' failed - expected caretOffset: ' + expected.caretOffset + ',\nactual caretOffset:' + actual.caretOffset);
19671967
}
19681968
}
19691969
}

‎src/services/services.ts‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,7 +1090,8 @@ namespace ts {
10901090

10911091
export interface TextInsertion {
10921092
newText: string;
1093-
offsetInNewText: number;
1093+
/** The position in newText the caret should point to after the insertion. */
1094+
caretOffset: number;
10941095
}
10951096

10961097
export interface RenameLocation {
@@ -6758,7 +6759,6 @@ namespace ts {
67586759
function getIndentationAtPosition(fileName: string, position: number, editorOptions: EditorOptions) {
67596760
let start = new Date().getTime();
67606761
let sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName);
6761-
log("getIndentationAtPosition: getCurrentSourceFile: " + (new Date().getTime() - start));
67626762

67636763
start = new Date().getTime();
67646764

@@ -6800,11 +6800,11 @@ namespace ts {
68006800
* Valid positions are
68016801
* * outside of comments, statements, and expressions, and
68026802
* * preceding a function declaration.
6803-
*
6803+
*
68046804
* Hosts should ideally check that:
68056805
* * The line is all whitespace up to 'position' before performing the insertion.
68066806
* * If the keystroke sequence "/\*\*" induced the call, we also check that the next
6807-
* non-whitespace character is '*', which (approximately) indicates whether we added
6807+
* non-whitespace character is '*', which (approximately) indicates whether we added
68086808
* the second '*' to complete an existing (JSDoc) comment.
68096809
* @param fileName The file in which to perform the check.
68106810
* @param position The (character-indexed) position in the file where the check should
@@ -6843,16 +6843,24 @@ namespace ts {
68436843
let docParams = parameters.map((p, index) =>
68446844
indentationStr + " * @param " + (p.name.kind === SyntaxKind.Identifier ? (<Identifier>p.name).text : "param" + index.toString()) + newLine);
68456845

6846-
let result =
6847-
/* opening comment */ "/**" + newLine +
6848-
/* first line for function info */ indentationStr + " * " + newLine +
6849-
/* paramters */ docParams.reduce((prev, cur) => prev + cur, "") +
6850-
/* closing comment */ indentationStr + " */" +
6851-
/* newline if at decl start */ (tokenStart === position ? newLine + indentationStr : "");
6852-
6853-
let cursorOffset = /* "/**" */ 3 + /* newLine */ newLine.length + indentationStr.length + /* " * " */ 3;
68546846

6855-
return {newText: result, offsetInNewText: cursorOffset };
6847+
// A doc comment consists of the following
6848+
// * The opening comment line
6849+
// * the first line (without a param) for the object's untagged info (this is also where the caret ends up)
6850+
// * the '@param'-tagged lines
6851+
// * TODO: other tags.
6852+
// * the closing comment line
6853+
// * if the caret was directly in front of the object, then we add an extra line and indentation.
6854+
let result =
6855+
"/**" + newLine +
6856+
indentationStr + " * " + newLine +
6857+
docParams.reduce((prev, cur) => prev + cur, "") +
6858+
indentationStr + " */" +
6859+
(tokenStart === position ? newLine + indentationStr : "");
6860+
6861+
let cursorOffset = "/**".length + newLine.length + indentationStr.length + " * ".length;
6862+
6863+
return { newText: result, caretOffset: cursorOffset };
68566864
}
68576865

68586866
function getTodoComments(fileName: string, descriptors: TodoCommentDescriptor[]): TodoComment[] {

‎src/services/utilities.ts‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -421,33 +421,32 @@ namespace ts {
421421
}
422422

423423
export function isInComment(sourceFile: SourceFile, position: number) {
424-
return isInCommentHelper(sourceFile, position, /*extraCheck*/ c => true);
424+
return isInCommentHelper(sourceFile, position, /*predicate*/ c => true);
425425
}
426426

427427
/**
428428
* Returns true if the cursor at position in sourceFile is within a comment that additionally
429-
* satisfies extraCheck, and false otherwise.
429+
* satisfies predicate, and false otherwise.
430430
*/
431-
export function isInCommentHelper(sourceFile: SourceFile, position: number,
432-
extraCheck: (c: CommentRange) => boolean): boolean {
431+
export function isInCommentHelper(sourceFile: SourceFile, position: number, predicate: (c: CommentRange) => boolean): boolean {
433432
let token = getTokenAtPosition(sourceFile, position);
434433

435434
if (token && position <= token.getStart()) {
436435
let commentRanges = getLeadingCommentRanges(sourceFile.text, token.pos);
437-
436+
437+
// The end marker of a single-line comment does not include the newline character.
438+
// In the following case, we are inside a comment (^ denotes the cursor position):
439+
//
440+
// // asdf ^\n
441+
//
442+
// But for multi-line comments, we don't want to be inside the comment in the following case:
443+
//
444+
// /* asdf */^
445+
//
446+
// Internally, we represent the end of the comment at the newline and closing '/', respectively.
438447
return forEach(commentRanges, c => c.pos < position &&
439-
// The end marker of a single-line comment does not include the newline character.
440-
// In the following case, we are inside a comment (^ denotes the cursor position):
441-
//
442-
// // asdf ^\n
443-
//
444-
// But for multi-line comments, we don't want to be inside the comment in the following case:
445-
//
446-
// /* asdf */^
447-
//
448-
// Internally, we represent the end of the comment at the newline and closing '/', respectively.
449448
(c.kind == SyntaxKind.SingleLineCommentTrivia ? position <= c.end : position < c.end) &&
450-
extraCheck(c));
449+
predicate(c));
451450
}
452451

453452
return false;
@@ -456,12 +455,15 @@ namespace ts {
456455
export function hasDocComment(sourceFile: SourceFile, position: number) {
457456
let token = getTokenAtPosition(sourceFile, position);
458457

459-
let JSDocPrefixRegex = /^\/\*\*\s*/;
460-
461458
// First, we have to see if this position actually landed in a comment.
462459
let commentRanges = getLeadingCommentRanges(sourceFile.text, token.pos);
463460

464-
return forEach(commentRanges, c => JSDocPrefixRegex.test(sourceFile.text.substring(c.pos, c.end)));
461+
return forEach(commentRanges, c => jsDocPrefix);
462+
463+
function jsDocPrefix(c: CommentRange): boolean {
464+
var text = sourceFile.text;
465+
return text.length >= c.pos + 3 && text[c.pos] === '/' && text[c.pos + 1] === '*' && text[c.pos + 2] === '*';
466+
}
465467
}
466468

467469
function nodeHasTokens(n: Node): boolean {

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ module FourSlashInterface {
379379
}
380380

381381
public DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean) {
382-
FourSlash.currentTestState.verifyDocCommentTemplate(empty ? undefined : { newText: expectedText, offsetInNewText: expectedOffset });
382+
FourSlash.currentTestState.verifyDocCommentTemplate(empty ? undefined : { newText: expectedText, caretOffset: expectedOffset });
383383
}
384384

385385
public noDocCommentTemplate() {

0 commit comments

Comments
 (0)