Skip to content

Commit e1635cb

Browse files
Copilotalexr00
andauthored
Unwrap 72-character wrapped commit message lines when creating PRs (#8255)
* Initial plan * Initial plan for commit message line unwrapping Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Implement commit message line unwrapping for PR creation - Add unwrapCommitMessageBody function to join wrapped lines - Preserve blank lines, list items, quotes, and indented content - Add comprehensive tests for various unwrapping scenarios - Matches GitHub's behavior when converting commit messages to PR descriptions Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Address code review feedback - Extract regex pattern to named constant PRESERVE_LINE_PATTERN - Replace deprecated substr() with slice() - Improve regex to match only spaces/tabs explicitly instead of \s - Add detailed comments explaining the pattern Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Refine code based on additional review feedback - Place hyphen at end of character class for clarity - Extract hasBody condition to avoid duplication Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Add support for fenced code blocks and fix indented code detection - Support fenced code blocks with ``` markers - Fix indented code detection to distinguish between: - Actual code blocks (4+ spaces, not in list context) - List item continuations (2+ spaces in list context) - Nested list items (detected by list markers with indentation) - Track list context state to handle list continuations correctly - Add comprehensive tests for all new scenarios Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 690554c commit e1635cb

File tree

2 files changed

+272
-2
lines changed

2 files changed

+272
-2
lines changed

src/github/folderRepositoryManager.ts

Lines changed: 144 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3020,15 +3020,157 @@ const ownedByMe: AsyncPredicate<GitHubRepository> = async repo => {
30203020
export const byRemoteName = (name: string): Predicate<GitHubRepository> => ({ remote: { remoteName } }) =>
30213021
remoteName === name;
30223022

3023+
/**
3024+
* Unwraps lines that were wrapped for conventional commit message formatting (typically at 72 characters).
3025+
* Similar to GitHub's behavior when converting commit messages to PR descriptions.
3026+
*
3027+
* Rules:
3028+
* - Preserves blank lines as paragraph breaks
3029+
* - Preserves fenced code blocks (```)
3030+
* - Preserves list items (-, *, +, numbered)
3031+
* - Preserves blockquotes (>)
3032+
* - Preserves indented code blocks (4+ spaces at start, when not in a list context)
3033+
* - Joins consecutive plain text lines that appear to be wrapped mid-sentence
3034+
*/
3035+
function unwrapCommitMessageBody(body: string): string {
3036+
if (!body) {
3037+
return body;
3038+
}
3039+
3040+
// Pattern to detect list item markers at the start of a line
3041+
const LIST_ITEM_PATTERN = /^[ \t]*([*+\-]|\d+\.)\s/;
3042+
// Pattern to detect blockquote markers
3043+
const BLOCKQUOTE_PATTERN = /^[ \t]*>/;
3044+
// Pattern to detect fenced code block markers
3045+
const FENCE_PATTERN = /^[ \t]*```/;
3046+
3047+
const lines = body.split('\n');
3048+
const result: string[] = [];
3049+
let i = 0;
3050+
let inFencedBlock = false;
3051+
let inListContext = false;
3052+
3053+
while (i < lines.length) {
3054+
const line = lines[i];
3055+
3056+
// Preserve blank lines
3057+
if (line.trim() === '') {
3058+
result.push(line);
3059+
i++;
3060+
inListContext = false; // Reset list context on blank line
3061+
continue;
3062+
}
3063+
3064+
// Check for fenced code block markers
3065+
if (FENCE_PATTERN.test(line)) {
3066+
inFencedBlock = !inFencedBlock;
3067+
result.push(line);
3068+
i++;
3069+
continue;
3070+
}
3071+
3072+
// Preserve everything inside fenced code blocks
3073+
if (inFencedBlock) {
3074+
result.push(line);
3075+
i++;
3076+
continue;
3077+
}
3078+
3079+
// Check if this line is a list item
3080+
const isListItem = LIST_ITEM_PATTERN.test(line);
3081+
3082+
// Check if this line is a blockquote
3083+
const isBlockquote = BLOCKQUOTE_PATTERN.test(line);
3084+
3085+
// Check if this line is indented (4+ spaces) but NOT a list continuation
3086+
// List continuations have leading spaces but we're in list context
3087+
const leadingSpaces = line.match(/^[ \t]*/)?.[0].length || 0;
3088+
const isIndentedCode = leadingSpaces >= 4 && !inListContext;
3089+
3090+
// Determine if this line should be preserved (not joined)
3091+
const shouldPreserveLine = isListItem || isBlockquote || isIndentedCode;
3092+
3093+
if (shouldPreserveLine) {
3094+
result.push(line);
3095+
i++;
3096+
// If this is a list item, we're now in list context
3097+
if (isListItem) {
3098+
inListContext = true;
3099+
}
3100+
continue;
3101+
}
3102+
3103+
// If we have leading spaces but we're in a list context, this is a list continuation
3104+
// We should preserve it to maintain list formatting
3105+
if (inListContext && leadingSpaces >= 2) {
3106+
result.push(line);
3107+
i++;
3108+
continue;
3109+
}
3110+
3111+
// Start accumulating lines that should be joined (plain text)
3112+
let joinedLine = line;
3113+
i++;
3114+
3115+
// Keep joining lines until we hit a blank line or a line that shouldn't be joined
3116+
while (i < lines.length) {
3117+
const nextLine = lines[i];
3118+
3119+
// Stop at blank lines
3120+
if (nextLine.trim() === '') {
3121+
break;
3122+
}
3123+
3124+
// Stop at fenced code blocks
3125+
if (FENCE_PATTERN.test(nextLine)) {
3126+
break;
3127+
}
3128+
3129+
// Stop at list items
3130+
if (LIST_ITEM_PATTERN.test(nextLine)) {
3131+
break;
3132+
}
3133+
3134+
// Stop at blockquotes
3135+
if (BLOCKQUOTE_PATTERN.test(nextLine)) {
3136+
break;
3137+
}
3138+
3139+
// Check if next line is indented code (4+ spaces, not in list context)
3140+
const nextLeadingSpaces = nextLine.match(/^[ \t]*/)?.[0].length || 0;
3141+
const nextIsIndentedCode = nextLeadingSpaces >= 4 && !inListContext;
3142+
3143+
if (nextIsIndentedCode) {
3144+
break;
3145+
}
3146+
3147+
// If in list context and next line is indented, it's a list continuation
3148+
if (inListContext && nextLeadingSpaces >= 2) {
3149+
break;
3150+
}
3151+
3152+
// Join this line with a space
3153+
joinedLine += ' ' + nextLine;
3154+
i++;
3155+
}
3156+
3157+
result.push(joinedLine);
3158+
}
3159+
3160+
return result.join('\n');
3161+
}
3162+
30233163
export const titleAndBodyFrom = async (promise: Promise<string | undefined>): Promise<{ title: string; body: string } | undefined> => {
30243164
const message = await promise;
30253165
if (!message) {
30263166
return;
30273167
}
30283168
const idxLineBreak = message.indexOf('\n');
3169+
const hasBody = idxLineBreak !== -1;
3170+
const rawBody = hasBody ? message.slice(idxLineBreak + 1).trim() : '';
30293171
return {
3030-
title: idxLineBreak === -1 ? message : message.substr(0, idxLineBreak),
3172+
title: hasBody ? message.slice(0, idxLineBreak) : message,
30313173

3032-
body: idxLineBreak === -1 ? '' : message.slice(idxLineBreak + 1).trim(),
3174+
body: unwrapCommitMessageBody(rawBody),
30333175
};
30343176
};

src/test/github/folderRepositoryManager.test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,132 @@ describe('titleAndBodyFrom', function () {
9494
assert.strictEqual(result?.title, 'title');
9595
assert.strictEqual(result?.body, '');
9696
});
97+
98+
it('unwraps wrapped lines in body', async function () {
99+
const message = Promise.resolve('title\n\nThis is a long line that has been wrapped at 72 characters\nto fit the conventional commit message format.');
100+
101+
const result = await titleAndBodyFrom(message);
102+
assert.strictEqual(result?.title, 'title');
103+
assert.strictEqual(result?.body, 'This is a long line that has been wrapped at 72 characters to fit the conventional commit message format.');
104+
});
105+
106+
it('preserves blank lines as paragraph breaks', async function () {
107+
const message = Promise.resolve('title\n\nFirst paragraph that is wrapped\nacross multiple lines.\n\nSecond paragraph that is also wrapped\nacross multiple lines.');
108+
109+
const result = await titleAndBodyFrom(message);
110+
assert.strictEqual(result?.title, 'title');
111+
assert.strictEqual(result?.body, 'First paragraph that is wrapped across multiple lines.\n\nSecond paragraph that is also wrapped across multiple lines.');
112+
});
113+
114+
it('preserves list items', async function () {
115+
const message = Promise.resolve('title\n\n- First item\n- Second item\n- Third item');
116+
117+
const result = await titleAndBodyFrom(message);
118+
assert.strictEqual(result?.title, 'title');
119+
assert.strictEqual(result?.body, '- First item\n- Second item\n- Third item');
120+
});
121+
122+
it('preserves numbered list items', async function () {
123+
const message = Promise.resolve('title\n\n1. First item\n2. Second item\n3. Third item');
124+
125+
const result = await titleAndBodyFrom(message);
126+
assert.strictEqual(result?.title, 'title');
127+
assert.strictEqual(result?.body, '1. First item\n2. Second item\n3. Third item');
128+
});
129+
130+
it('preserves indented lines', async function () {
131+
const message = Promise.resolve('title\n\nNormal paragraph.\n\n Indented code block\n More code');
132+
133+
const result = await titleAndBodyFrom(message);
134+
assert.strictEqual(result?.title, 'title');
135+
assert.strictEqual(result?.body, 'Normal paragraph.\n\n Indented code block\n More code');
136+
});
137+
138+
it('unwraps but preserves asterisk list items', async function () {
139+
const message = Promise.resolve('title\n\n* First item\n* Second item');
140+
141+
const result = await titleAndBodyFrom(message);
142+
assert.strictEqual(result?.title, 'title');
143+
assert.strictEqual(result?.body, '* First item\n* Second item');
144+
});
145+
146+
it('handles mixed content with wrapped paragraphs and lists', async function () {
147+
const message = Promise.resolve('title\n\nThis is a paragraph that has been wrapped\nat 72 characters.\n\n- Item 1\n- Item 2\n\nAnother wrapped paragraph\nthat continues here.');
148+
149+
const result = await titleAndBodyFrom(message);
150+
assert.strictEqual(result?.title, 'title');
151+
assert.strictEqual(result?.body, 'This is a paragraph that has been wrapped at 72 characters.\n\n- Item 1\n- Item 2\n\nAnother wrapped paragraph that continues here.');
152+
});
153+
154+
it('preserves lines with special characters at the start', async function () {
155+
const message = Promise.resolve('title\n\n> Quote line 1\n> Quote line 2');
156+
157+
const result = await titleAndBodyFrom(message);
158+
assert.strictEqual(result?.title, 'title');
159+
assert.strictEqual(result?.body, '> Quote line 1\n> Quote line 2');
160+
});
161+
162+
it('handles wrapped lines with punctuation', async function () {
163+
const message = Promise.resolve('title\n\nThis is a sentence.\nThis is another sentence on a new line.');
164+
165+
const result = await titleAndBodyFrom(message);
166+
assert.strictEqual(result?.title, 'title');
167+
assert.strictEqual(result?.body, 'This is a sentence. This is another sentence on a new line.');
168+
});
169+
170+
it('preserves fenced code blocks', async function () {
171+
const message = Promise.resolve('title\n\nSome text before.\n\n```\ncode line 1\ncode line 2\n```\n\nSome text after.');
172+
173+
const result = await titleAndBodyFrom(message);
174+
assert.strictEqual(result?.title, 'title');
175+
assert.strictEqual(result?.body, 'Some text before.\n\n```\ncode line 1\ncode line 2\n```\n\nSome text after.');
176+
});
177+
178+
it('preserves fenced code blocks with language', async function () {
179+
const message = Promise.resolve('title\n\nSome text.\n\n```javascript\nconst x = 1;\nconst y = 2;\n```\n\nMore text.');
180+
181+
const result = await titleAndBodyFrom(message);
182+
assert.strictEqual(result?.title, 'title');
183+
assert.strictEqual(result?.body, 'Some text.\n\n```javascript\nconst x = 1;\nconst y = 2;\n```\n\nMore text.');
184+
});
185+
186+
it('preserves nested list items with proper indentation', async function () {
187+
const message = Promise.resolve('title\n\n- Item 1\n - Nested item 1.1\n - Nested item 1.2\n- Item 2');
188+
189+
const result = await titleAndBodyFrom(message);
190+
assert.strictEqual(result?.title, 'title');
191+
assert.strictEqual(result?.body, '- Item 1\n - Nested item 1.1\n - Nested item 1.2\n- Item 2');
192+
});
193+
194+
it('preserves list item continuations', async function () {
195+
const message = Promise.resolve('title\n\n- This is a list item that is long\n and continues on the next line\n- Second item');
196+
197+
const result = await titleAndBodyFrom(message);
198+
assert.strictEqual(result?.title, 'title');
199+
assert.strictEqual(result?.body, '- This is a list item that is long\n and continues on the next line\n- Second item');
200+
});
201+
202+
it('preserves indented code blocks but not list continuations', async function () {
203+
const message = Promise.resolve('title\n\nRegular paragraph.\n\n This is code\n More code\n\nAnother paragraph.');
204+
205+
const result = await titleAndBodyFrom(message);
206+
assert.strictEqual(result?.title, 'title');
207+
assert.strictEqual(result?.body, 'Regular paragraph.\n\n This is code\n More code\n\nAnother paragraph.');
208+
});
209+
210+
it('unwraps regular text but preserves list item continuations', async function () {
211+
const message = Promise.resolve('title\n\nThis is wrapped text\nthat should be joined.\n\n- List item with\n continuation\n- Another item');
212+
213+
const result = await titleAndBodyFrom(message);
214+
assert.strictEqual(result?.title, 'title');
215+
assert.strictEqual(result?.body, 'This is wrapped text that should be joined.\n\n- List item with\n continuation\n- Another item');
216+
});
217+
218+
it('handles complex nested lists with wrapped paragraphs', async function () {
219+
const message = Promise.resolve('title\n\nWrapped paragraph\nacross lines.\n\n- Item 1\n - Nested item\n More nested content\n- Item 2\n\nAnother wrapped paragraph\nhere.');
220+
221+
const result = await titleAndBodyFrom(message);
222+
assert.strictEqual(result?.title, 'title');
223+
assert.strictEqual(result?.body, 'Wrapped paragraph across lines.\n\n- Item 1\n - Nested item\n More nested content\n- Item 2\n\nAnother wrapped paragraph here.');
224+
});
97225
});

0 commit comments

Comments
 (0)