Fixup misported TokensAreOnSameLine - #64597
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Conversation
typescript-automation
Bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| func (this *FormattingContext) TokensAreOnSameLine() bool { | ||
| if this.tokensAreOnSameLine == core.TSUnknown { | ||
| this.tokensAreOnSameLine = this.rangeIsOnOneLine(core.NewTextRange(this.currentTokenSpan.Loc.Pos(), this.nextTokenSpan.Loc.End())) | ||
| this.tokensAreOnSameLine = this.rangeIsOnOneLine(core.NewTextRange(this.currentTokenSpan.Loc.Pos(), this.nextTokenSpan.Loc.Pos())) |
There was a problem hiding this comment.
Strada's logic here:
TypeScript/src/services/formatting/formattingContext.ts
Lines 69 to 77 in 050880c
There was a problem hiding this comment.
I'm surprised this matters, what tokens are multiline?
There was a problem hiding this comment.
For example, the TemplateTail here:
const s = `${1}
`;| defer done() | ||
| f.FormatDocument(t, "") | ||
| f.VerifyCurrentFileContent(t, "const x = <HangupButton customClass= 'ha\n") | ||
| f.VerifyCurrentFileContent(t, "const x = <HangupButton customClass='ha\n") |
There was a problem hiding this comment.
This test didn't exist in Strada and it just crashes there. That's why the output is different here (and why this test isn't simply unskipped like the other ones)
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: None
What changed in this PR
Enables previously skipped fourslash formatter tests and fixes a misported implementation of TokensAreOnSameLine, updating an expected formatting output accordingly.
Changes:
- Removed
t.Skipfrom two template-literal spacing formatter tests so they run in CI. - Corrected
TokensAreOnSameLineto compute same-line status using the next token’s start position. - Updated JSX unterminated-string formatting expectation to match the formatter’s output.
| File | Description |
|---|---|
| tsc/internal/fourslash/tests/formatSpaceAfterTemplateHeadAndMiddle_test.go | Unskips a formatter fourslash test for template literal spacing. |
| tsc/internal/fourslash/tests/formatNoSpaceAfterTemplateHeadAndMiddle_test.go | Unskips a formatter fourslash test for template literal spacing. |
| tsc/internal/fourslash/tests/formatDocumentNoCrashJsxAttrUnterminatedString_test.go | Updates expected formatted output for unterminated JSX attribute string case. |
| tsc/internal/format/context.go | Fixes TokensAreOnSameLine logic to use next token start position. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
No description provided.