-
Notifications
You must be signed in to change notification settings - Fork 230
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
[Not Ready] [typespec-vscode] Add codefix for triple quate indent #5458
base: main
Are you sure you want to change the base?
[Not Ready] [typespec-vscode] Add codefix for triple quate indent #5458
Conversation
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
You can try these changes here
|
const startPos = location.pos; | ||
const endPos = location.end; | ||
const offSet = 3; | ||
const splitOrIndentStr = "\r\n"; |
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.
the name is confusing, it's not a indent, right?
const splitStr = "\r\n"; | ||
|
||
const lines = splitLines(location.file.text, startPos, endPos); | ||
|
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.
handle the special case first, like 0 line, 1 line.
result.push(context.prependText({ ...location, pos: startPos }, splitStr + prefix)); | ||
} | ||
|
||
if (lastLine.lineText.trim() === lines[lines.length - 1].lineText.trim()) { |
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.
what's this for?
let indentDiff = lastLine.indent - minIndentLine.indent; | ||
const prefix = " ".repeat(indentDiff); | ||
// start triple-quote is not on a new line | ||
if (firstLine.lineText.trim() !== "") { |
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.
you can save the check resolve instead of trim every time
lines.pop(); | ||
} | ||
|
||
if (lines.length > 0 && firstLine.lineText.trim() === lines[0].lineText.trim()) { |
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.
what's this for?
: lastLine.indent - minIndentLine.indent; | ||
result.push( | ||
context.prependText({ ...location, pos: endPos }, splitStr + " ".repeat(indentDiff)), | ||
); |
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.
dont you need a '\n'?
const prefix = " ".repeat(indentDiff); | ||
// start triple-quote is not on a new line | ||
if (firstLine.lineText.trim() !== "") { | ||
result.push(context.prependText({ ...location, pos: startPos }, splitStr + prefix)); |
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.
instead of adding fix for each line, can't we just have one fix for the whole string?
|
||
function getIndentNumbInLine(lineText: string): number { | ||
let curStart = 0; | ||
const curEnd = lineText.length; |
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.
usually end means a position, not lenght
}); | ||
} | ||
|
||
function splitLines( |
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.
any reason not use the existing one?
Fix: #4609
Add codefix for
triple-quote-indent
warning ,and meanwhile add test case for it