Skip to content

Fix incorrect formatting for comments inside multi-line argument lists and method chains #1929

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 22, 2025
Merged

Fix incorrect formatting for comments inside multi-line argument lists and method chains #1929

merged 6 commits into from
Oct 22, 2025

Conversation

Copy link
Contributor

Copilot AI commented Oct 22, 2025 •
edited by jakebailey
Loading

Problem

The formatter was incorrectly removing indentation from comments inside multi-line argument lists and chained method calls. This caused comments to be flush with the left margin instead of maintaining their proper indentation level, and in some cases caused a panic with "strings: negative Repeat count".

Example 1: Multi-line argument list

// Before (incorrect)
console.log(
	"a",
// the second arg  ← Lost indentation!
	"b"
);

// After (correct)
console.log(
	"a",
	// the second arg  ← Indentation preserved!
	"b"
);

Example 2: Chained method calls

// Before (incorrect - caused panic)
foo
	.bar()
// A second call  ← Lost indentation!
	.baz();

// After (correct)
foo
	.bar()
	// A second call  ← Indentation preserved!
	.baz();

Root Cause

The getIndentationForComment function in internal/format/span.go was not using the tokenIndentation parameter passed to it. This parameter contains the calculated indentation for the token that follows the comment. When available (not -1), it should be used instead of the base indentation level.

The TypeScript reference implementation at _submodules/TypeScript/src/services/formatting/formatting.ts:704 includes this check:

return tokenIndentation !== Constants.Unknown ? tokenIndentation : indentation;

But the Go port was missing this logic, always returning the base indentation regardless of the token-specific indentation. When tokenIndentation was -1, it was passed directly to the indentation string generation, causing a panic in strings.Repeat.

Solution

Added a check to use tokenIndentation when it's not -1 (the "unknown" sentinel value):

if tokenIndentation != -1 {
    return tokenIndentation
}
return i.indentation

This ensures that comments inherit the indentation of the token they precede, maintaining consistent formatting within multi-line structures.

Testing

  • Added comprehensive test cases for both reported scenarios
  • Added explicit regression test for issue Panic formatting chained method call #1928 (panic when formatting chained method calls)
  • All existing tests continue to pass
  • Full test suite runs successfully with no regressions

Fixes #1927
Fixes #1928
Fixes #1925

Original prompt

This section details on the original issue you should resolve

<issue_title>Incorrect formatting for comment inside of multi-line argument list</issue_title>
<issue_description>Repo
Format the following file:

console.log(
	"a",
	// the second arg
	"b"
);

Bug
This incorrectly removes the space before the comment, resulting in:

console.log(
	"a",
// the second arg
	"b"
); 
```</issue_description>

<agent_instructions>Create formatting tests first, then examine the code in comparison to _submodules/TypeScript/src/services/formatting/formatting.ts.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@mjbvz</author><body>
Similar issue for chained method calls:

```ts

foo
	.bar()
	// A second call
	.baz(); 
```</body></comment_new>
</comments>

Fixes #1927

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix incorrect formatting for comments in multi-line argument lists Fix incorrect formatting for comments inside multi-line argument lists and method chains Oct 22, 2025
Copilot AI requested a review from jakebailey October 22, 2025 18:51
Copilot finished work on behalf of jakebailey October 22, 2025 18:51
@jakebailey jakebailey marked this pull request as ready for review October 22, 2025 18:51
Copilot AI review requested due to automatic review settings October 22, 2025 18:51
Copilot AI reviewed Oct 22, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a formatting bug where comments inside multi-line argument lists and chained method calls were incorrectly losing their indentation. The fix aligns the Go port with the TypeScript reference implementation by checking and using the token-specific indentation value when available.

Key changes:

  • Added logic to use tokenIndentation when it's not -1 (unknown) in getIndentationForComment
  • Added two comprehensive test cases verifying comment indentation is preserved in multi-line argument lists and method chains

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/format/span.go Added check to use token-specific indentation for comments when available
internal/format/comment_test.go Added tests for comment formatting in multi-line argument lists and chained method calls

@jakebailey
Copy link
Member

jakebailey commented Oct 22, 2025

@copilot also add the test for #1928, since the fix in this PR also fixes that issue.

@jakebailey jakebailey mentioned this pull request Oct 22, 2025
Closed
7 tasks
Copy link
Contributor Author

Copilot AI commented Oct 22, 2025

@copilot also add the test for #1928, since the fix in this PR also fixes that issue.

Added regression test for issue #1928 in commit ccd9922. The test explicitly documents that the panic was caused by the same root issue - tokenIndentation being -1 was not being handled correctly.

Copilot finished work on behalf of jakebailey October 22, 2025 19:00
@jakebailey jakebailey mentioned this pull request Oct 22, 2025
Closed
@jakebailey jakebailey enabled auto-merge October 22, 2025 19:11
@jakebailey jakebailey added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit cfbb8b4 Oct 22, 2025
22 checks passed
@jakebailey jakebailey deleted the copilot/fix-comment-formatting-multiline-args branch October 22, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@RyanCavanaugh RyanCavanaugh RyanCavanaugh approved these changes

+1 more reviewer

@jakebailey jakebailey jakebailey approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jakebailey jakebailey

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@jakebailey @RyanCavanaugh