Skip to content

Improve commit line break unwrapping for variable-length list markers #8417

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
Draft
Copilot wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Improve commit line break unwrapping for variable-length list markers #8417

Copilot wants to merge 4 commits into from
+414 −68

Conversation

Copy link
Contributor

Copilot AI commented Jan 23, 2026 •
edited
Loading

  • 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
@alexr00
Improve commit line break unwrapping for lists
b6cf72f 
- 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 <[email protected]>
@alexr00
Fix plain text joining to trim whitespace
f6b1a77 
Use appendWithSpace with trimmed text to avoid double spaces when joining
lines with leading whitespace.

Co-authored-by: alexr00 <[email protected]>
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
alexr00
alexr00 reviewed Jan 28, 2026
View reviewed changes
src/github/folderRepositoryManager.ts Outdated
// 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

@alexr00 alexr00 Jan 28, 2026

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

Copilot AI Jan 28, 2026

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.

@alexr00
Use named captures in LIST_ITEM_PATTERN regex
f64389c 
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 <[email protected]>
Copilot AI requested a review from alexr00 January 28, 2026 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexr00 alexr00 Awaiting requested review from alexr00

At least 1 approving review is required to merge this pull request.

Assignees

@alexr00 alexr00

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve commit line break unwrapping

2 participants

@alexr00