getTokenAtPosition: default includeJsDocComment to true #25015
Conversation
ghost
force-pushed
the
getTokenAtPosition
branch
from
f451235 to
35da4ae
Compare
src/services/utilities.ts
Outdated
| @@ -661,8 +661,8 @@ namespace ts { | |||
| } | |||
|
|
|||
| /** Returns a token if position is in [start-of-leading-trivia, end) */ | |||
| export function getTokenAtPosition(sourceFile: SourceFile, position: number, includeJsDocComment: boolean, includeEndPosition = false): Node { | |||
| return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ true, /*includePrecedingTokenAtEndPosition*/ undefined, includeEndPosition, includeJsDocComment); | |||
| export function getTokenAtPosition(sourceFile: SourceFile, position: number, includeJsDocComment = true): Node { | |||
There was a problem hiding this comment.
From the change it looks like there is no place remaining where we need includeJsDocComment as false? Why have a parameter then. If this is rare scenario may be change the parameter to take optional value without assigning default and do !!ignoreJsDocComment so that we save assigning default value to the parameter
There was a problem hiding this comment.
There are a few left actually, though they're suspicious. #25162
There was a problem hiding this comment.
If that's the case I wouldn't make this parameter optional (could be done in #25612) but here we should keep the comments and pass true I think. Making it optional makes it more confusing.
ghost
mentioned this pull request
|
Latest commit fixes #25162 |
|
@sheetalkamat Please re-review |
| @@ -38,7 +38,7 @@ verify.completions( | |||
| { marker: "quoteInComment", exact: undefined, triggerCharacter: '"' }, | |||
| { marker: "lessInComment", exact: undefined, triggerCharacter: "<" }, | |||
|
|
|||
| { marker: "openTag", includes: "div", triggerCharacter: "<" }, | |||
| { marker: "openTag", exact: undefined, triggerCharacter: "<" }, // TODO: `includes: "div"` | |||
There was a problem hiding this comment.
is there issue opened for this?
Sequel to #23379. Would have made #25014 unnecessary.