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
Truncating lineText
in references
#51137
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3522,7 +3522,26 @@ namespace ts.server { | |
const scriptInfo = Debug.checkDefined(projectService.getScriptInfo(fileName)); | ||
const span = toProtocolTextSpanWithContext(textSpan, contextSpan, scriptInfo); | ||
const lineSpan = scriptInfo.lineToTextSpan(span.start.line - 1); | ||
const lineText = scriptInfo.getSnapshot().getText(lineSpan.start, textSpanEnd(lineSpan)).replace(/\r|\n/g, ""); | ||
if (lineSpan.length > 1024) { | ||
var start = (span.start.offset > 500) ? (span.start.offset - 501 + lineSpan.start) : lineSpan.start; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We never use |
||
var end = (lineSpan.length -2 - span.end.offset > 500) ? (span.end.offset + 499 + lineSpan.start) : ts.textSpanEnd(lineSpan)-2; // need to ask why length is 2 more | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I probably said "500", but I meant "half of the max length" (easy, if you have a constant for the max length). Also, you may discover after playing with it, that you want less text before and more text after the reference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually when we subtract two from the length of a line, it's for |
||
var lineText; | ||
const sourceFile = scriptInfo.getDefaultProject().getLanguageService().getProgram()!.getSourceFile(fileName)!; | ||
let tokenAtPosition = getTokenAtPosition(sourceFile, start); | ||
let tokenAtPositionEnd = getTokenAtPosition(sourceFile, end - 1); | ||
if (tokenAtPosition.getStart() < start) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feed in the optional |
||
const nextToken = findNextToken(tokenAtPosition, tokenAtPosition.parent, sourceFile)?.getStart()!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might call this "nextTokenStart", to make it clearer that it's not a token. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In here, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if the token looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, the Also, if you are calling |
||
start = (nextToken - start) > 10 ? start : nextToken; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this gets more polished, it might help to add comments. |
||
} | ||
if (tokenAtPositionEnd.getEnd() > end) { | ||
const precedingToken = findPrecedingToken(end, sourceFile, tokenAtPositionEnd)?.getStart()!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment with |
||
end = (end - precedingToken) > 10 ? end: precedingToken; | ||
} | ||
lineText = scriptInfo.getSnapshot().getText(start, end).replace(/\r|\n/g, "")!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we want to add "..." or something when we truncate? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside from the |
||
} | ||
else { | ||
lineText = scriptInfo.getSnapshot().getText(lineSpan.start, textSpanEnd(lineSpan)).replace(/\r|\n/g, ""); | ||
} | ||
return { | ||
file: fileName, | ||
...span, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since 1024 is just a number I made up, it may need to be tuned. That would be easier if it were a constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid using a
magic number
, it would be better to create a new variable with a proper name and store 1024 into it.