Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 23, 2026

  • Understand the issue and review feedback from PR Improve commit line unwrapping #8335
  • Explore the current implementation of unwrapCommitMessageBody function
  • Review existing test cases in folderRepositoryManager.test.ts
  • Identify the problems in the current implementation:
    • List continuation assumes exactly 2 spaces (e.g., activeIndent + 2) but numbered list markers can be variable-length (e.g., 1. vs 10.)
    • Need better handling for numbered lists with multi-digit numbers
    • Need tests for multi-paragraph lists
    • Need tests for code blocks within lists
  • Fix the implementation to:
    • Calculate proper continuation indent based on the list marker length
    • Use 1+ space continuation (but not 4 which is code block)
    • Preserve list context across blank lines for multi-paragraph lists
  • Add new test cases covering:
    • Multi-digit numbered lists (10., 11., etc.)
    • Multi-paragraph lists (blank line within list item)
    • Code blocks within list items
    • Variable space continuations
    • Nested versions of above tests
  • Run linting and hygiene checks
  • Request code review
  • Address code review feedback (trim whitespace in plain text joining)
  • Run CodeQL security scan
  • Address feedback: use named captures in regex pattern
Original prompt

This section details on the original issue you should resolve

<issue_title>Improve commit line break unwrapping</issue_title>
<issue_description>> Opened #8335 to address this.

Thanks @alexr00 for improving this! Unfortunately I think the follow-up PR might have some problems too, and may need some more tests. See #8335 (comment)

Originally posted by @justingrant in #8255 (comment)
</issue_description>

<agent_instructions>Pay special attention to #8335 (comment)</agent_instructions>

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


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 23, 2026 17:36
- Fix list continuation to use content indent (not marker indent)
- Support variable-length numbered list markers (1., 10., 100., etc.)
- Allow flexible continuation spacing (1+ spaces, but not 4+ which is code)
- Preserve multi-paragraph list formatting (blank lines within list items)
- Track list context across blank lines for proper paragraph unwrapping
- Add comprehensive test cases for all scenarios

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Use appendWithSpace with trimmed text to avoid double spaces when joining
lines with leading whitespace.

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve commit line break unwrapping Improve commit line break unwrapping for variable-length list markers Jan 23, 2026
Copilot AI requested a review from alexr00 January 23, 2026 17:40
// Pattern to detect list item markers at the start of a line
const LIST_ITEM_PATTERN = /^[ \t]*([*+\-]|\d+\.)\s/;
// Pattern to detect list item markers at the start of a line and capture the marker
const LIST_ITEM_PATTERN = /^([ \t]*)([*+\-]|\d+\.)([ \t]+)/;
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Use named captures

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in f64389c. The regex now uses named capture groups: leadingWhitespace, marker, and markerTrailingWhitespace.

Renamed capture groups for clarity:
- leadingWhitespace: spaces/tabs before marker
- marker: the list item marker (*, +, -, or digits.)
- markerTrailingWhitespace: spaces after marker

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copilot AI requested a review from alexr00 January 28, 2026 11:05
@alexr00 alexr00 marked this pull request as ready for review January 29, 2026 07:33
Copilot AI review requested due to automatic review settings January 29, 2026 07:33
@alexr00 alexr00 marked this pull request as draft January 29, 2026 07:33
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 the commit message body unwrapping logic to correctly handle variable-length list markers, particularly for numbered lists with multi-digit numbers (e.g., 10., 11.). The previous implementation assumed a fixed continuation indent, which broke with multi-digit numbered lists.

Changes:

  • Modified LIST_ITEM_PATTERN regex to use named capture groups for extracting marker components
  • Replaced simple indent tracking with a stack storing both markerIndent and contentIndent for each list level
  • Updated blank line handling to preserve list context for multi-paragraph lists
  • Added comprehensive test coverage for multi-digit lists, multi-paragraph lists, code blocks within lists, and various continuation indent scenarios
  • Included routine updates to VS Code proposed API type definitions

Reviewed changes

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

Show a summary per file
File Description
src/github/folderRepositoryManager.ts Core implementation fix: uses named regex captures to calculate proper content indent for variable-length list markers, enabling correct handling of multi-digit numbered lists and multi-paragraph list items
src/test/github/folderRepositoryManager.test.ts Adds 13 new test cases covering multi-digit numbered lists, multi-paragraph lists, code blocks within lists, variable space continuations, and nested variations
src/@types/vscode.proposed.chatSessionsProvider.d.ts Updates proposed API type definitions to add CancellationToken parameter to refresh handlers and simplifies changes property types
src/@types/vscode.proposed.chatParticipantPrivate.d.ts Bumps API version from 11 to 12
src/@types/vscode.proposed.chatParticipantAdditions.d.ts Adds new ChatQuestion and ChatResponseQuestionCarouselPart types for question carousel UI feature
src/@types/vscode.proposed.chatContextProvider.d.ts Refactors context provider APIs into separate workspace, explicit, and resource provider interfaces with clearer separation of concerns

* - For a resource. Make sure to pass a selector that matches the resource you want to provide context for.
* Providers registered without a selector will not be called for resource-based context.
* - Explicitly. These context items are shown as options when the user explicitly attaches context.
* Register a chat workspace context provider. Workspace context is automatically included in all chat requests.

Choose a reason for hiding this comment

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

@alexr00 @copilot Why does this commit make changes to @types for various chat-related things?

I'd expect the only changes to be in src/github/folderRepositoryManager.ts‎.

Copy link
Member

Choose a reason for hiding this comment

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

They get updated as part of npm install, which Copilot runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve commit line break unwrapping

3 participants