Skip to content

Commit 65a2cf4

Browse files
committed
docs: standardize carriage return (\r) and line feed (\n) terminology
Improve code clarity by consistently adding escape sequence notation to all references of carriage returns and line feeds throughout documentation and tests. This makes the code more readable and avoids ambiguity when discussing these special characters.
1 parent 91456ab commit 65a2cf4

File tree

3 files changed

+115
-95
lines changed

3 files changed

+115
-95
lines changed

.changeset/cuddly-cows-sip.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
"roo-cline": patch
33
---
44

5-
I introduced a new method `processCarriageReturns` in `TerminalProcess.ts` to process carriage returns in terminal output. This method splits the output into lines, handles each line with `\r` by retaining only the content after the last carriage return, and preserves escape sequences to avoid breaking terminal formatting. The method is called within `getUnretrievedOutput` to ensure output is processed before being displayed. Additionally, I added comprehensive test cases in `TerminalProcess.test.ts` under a new `describe("processCarriageReturns", ...)` block to validate various scenarios, including basic progress bars, multiple lines, and ANSI escape sequences.
5+
I introduced a new method `processCarriageReturns` in `TerminalProcess.ts` to process carriage returns (\r) in terminal output. This method splits the output into lines, handles each line with carriage returns (\r) by retaining only the content after the last carriage return (\r), and preserves escape sequences to avoid breaking terminal formatting. The method is called within `getUnretrievedOutput` to ensure output is processed before being displayed. Additionally, I added comprehensive test cases in `TerminalProcess.test.ts` under a new `describe("processCarriageReturns", ...)` block to validate various scenarios, including basic progress bars, multiple lines with mixed carriage returns (\r) and line feeds (\n), and ANSI escape sequences.
66

77
Key implementation details:
88

99
- The solution carefully handles special characters and escape sequences to maintain terminal integrity.
10-
- Tradeoff: Slightly increased processing overhead for outputs with carriage returns, but this is negligible compared to the improved user experience.
11-
- Id like reviewers to pay close attention to the handling of edge cases in `processCarriageReturns` (e.g., lines ending with `\r` or mixed content with escape sequences) to ensure no unintended side effects.
10+
- Tradeoff: Slightly increased processing overhead for outputs with carriage returns (\r), but this is negligible compared to the improved user experience.
11+
- I'd like reviewers to pay close attention to the handling of edge cases in `processCarriageReturns` (e.g., lines ending with carriage returns (\r) or mixed content with carriage returns (\r), line feeds (\n), and escape sequences) to ensure no unintended side effects.

src/integrations/misc/__tests__/extract-text.test.ts

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -264,53 +264,53 @@ describe("applyRunLengthEncoding", () => {
264264
})
265265

266266
describe("processCarriageReturns", () => {
267-
it("should return original input if no carriage returns present", () => {
267+
it("should return original input if no carriage returns (\r) present", () => {
268268
const input = "Line 1\nLine 2\nLine 3"
269269
expect(processCarriageReturns(input)).toBe(input)
270270
})
271271

272-
it("should process basic progress bar with carriage returns", () => {
272+
it("should process basic progress bar with carriage returns (\r)", () => {
273273
const input = "Progress: [===>---------] 30%\rProgress: [======>------] 60%\rProgress: [==========>] 100%"
274274
const expected = "Progress: [==========>] 100%%"
275275
expect(processCarriageReturns(input)).toBe(expected)
276276
})
277277

278-
it("should handle multi-line outputs with carriage returns", () => {
278+
it("should handle multi-line outputs with carriage returns (\r)", () => {
279279
const input = "Line 1\rUpdated Line 1\nLine 2\rUpdated Line 2\rFinal Line 2"
280280
const expected = "Updated Line 1\nFinal Line 2 2"
281281
expect(processCarriageReturns(input)).toBe(expected)
282282
})
283283

284-
it("should handle carriage returns at end of line", () => {
285-
// A carriage return at the end of a line should be treated as if the cursor is at the start
284+
it("should handle carriage returns (\r) at end of line", () => {
285+
// A carriage return (\r) at the end of a line should be treated as if the cursor is at the start
286286
// with no content following it, so we keep the existing content
287287
const input = "Initial text\rReplacement text\r"
288288
// Depending on terminal behavior:
289-
// Option 1: If last CR is ignored because nothing follows it to replace text
289+
// Option 1: If last carriage return (\r) is ignored because nothing follows it to replace text
290290
const expected = "Replacement text"
291291
expect(processCarriageReturns(input)).toBe(expected)
292292
})
293293

294294
// Additional test to clarify behavior with a terminal-like example
295-
it("should handle carriage returns in a way that matches terminal behavior", () => {
295+
it("should handle carriage returns (\r) in a way that matches terminal behavior", () => {
296296
// In a real terminal:
297297
// 1. "Hello" is printed
298-
// 2. CR moves cursor to start of line
298+
// 2. Carriage return (\r) moves cursor to start of line
299299
// 3. "World" overwrites, becoming "World"
300-
// 4. CR moves cursor to start again
300+
// 4. Carriage return (\r) moves cursor to start again
301301
// 5. Nothing follows, so the line remains "World" (cursor just sitting at start)
302302
const input = "Hello\rWorld\r"
303303
const expected = "World"
304304
expect(processCarriageReturns(input)).toBe(expected)
305305

306-
// Same principle applies to CR+NL
306+
// Same principle applies to carriage return (\r) + line feed (\n)
307307
// 1. "Line1" is printed
308-
// 2. CR moves cursor to start
309-
// 3. NL moves to next line, so the line remains "Line1"
308+
// 2. Carriage return (\r) moves cursor to start
309+
// 3. Line feed (\n) moves to next line, so the line remains "Line1"
310310
expect(processCarriageReturns("Line1\r\n")).toBe("Line1\n")
311311
})
312312

313-
it("should preserve lines without carriage returns", () => {
313+
it("should preserve lines without carriage returns (\r)", () => {
314314
const input = "Line 1\nLine 2\rUpdated Line 2\nLine 3"
315315
const expected = "Line 1\nUpdated Line 2\nLine 3"
316316
expect(processCarriageReturns(input)).toBe(expected)
@@ -329,7 +329,7 @@ describe("processCarriageReturns", () => {
329329
expect(processCarriageReturns(input)).toBe(expected)
330330
})
331331

332-
it("should handle mixed content with carriage returns and newlines", () => {
332+
it("should handle mixed content with carriage returns (\r) and line feeds (\n)", () => {
333333
const input =
334334
"Step 1: Starting\rStep 1: In progress\rStep 1: Done\nStep 2: Starting\rStep 2: In progress\rStep 2: Done"
335335
const expected = "Step 1: Donerogress\nStep 2: Donerogress"
@@ -340,8 +340,8 @@ describe("processCarriageReturns", () => {
340340
expect(processCarriageReturns("")).toBe("")
341341
})
342342

343-
it("should handle large number of carriage returns efficiently", () => {
344-
// Create a string with many carriage returns
343+
it("should handle large number of carriage returns (\r) efficiently", () => {
344+
// Create a string with many carriage returns (\r)
345345
let input = ""
346346
for (let i = 0; i < 10000; i++) {
347347
input += `Progress: ${i / 100}%\r`
@@ -353,44 +353,44 @@ describe("processCarriageReturns", () => {
353353
})
354354

355355
// Additional edge cases to stress test processCarriageReturns
356-
it("should handle consecutive carriage returns", () => {
356+
it("should handle consecutive carriage returns (\r)", () => {
357357
const input = "Initial\r\r\r\rFinal"
358358
const expected = "Finalal"
359359
expect(processCarriageReturns(input)).toBe(expected)
360360
})
361361

362-
it("should handle carriage returns at the start of a line", () => {
362+
it("should handle carriage returns (\r) at the start of a line", () => {
363363
const input = "\rText after carriage return"
364364
const expected = "Text after carriage return"
365365
expect(processCarriageReturns(input)).toBe(expected)
366366
})
367367

368-
it("should handle only carriage returns", () => {
368+
it("should handle only carriage returns (\r)", () => {
369369
const input = "\r\r\r\r"
370370
const expected = ""
371371
expect(processCarriageReturns(input)).toBe(expected)
372372
})
373373

374-
it("should handle carriage returns with empty strings between them", () => {
374+
it("should handle carriage returns (\r) with empty strings between them", () => {
375375
const input = "Start\r\r\r\r\rEnd"
376376
const expected = "Endrt"
377377
expect(processCarriageReturns(input)).toBe(expected)
378378
})
379379

380-
it("should handle multiline with carriage returns at different positions", () => {
380+
it("should handle multiline with carriage returns (\r) at different positions", () => {
381381
const input = "Line1\rLine1Updated\nLine2\nLine3\rLine3Updated\rLine3Final\nLine4"
382382
const expected = "Line1Updated\nLine2\nLine3Finaled\nLine4"
383383
expect(processCarriageReturns(input)).toBe(expected)
384384
})
385385

386-
it("should handle carriage returns with special characters", () => {
386+
it("should handle carriage returns (\r) with special characters", () => {
387387
// This test demonstrates our handling of multi-byte characters (like emoji) when they get partially overwritten.
388-
// When a carriage return causes partial overwrite of a multi-byte character (like an emoji),
388+
// When a carriage return (\r) causes partial overwrite of a multi-byte character (like an emoji),
389389
// we need to handle this special case to prevent display issues or corruption.
390390
//
391391
// In this example:
392392
// 1. "Line with 🚀 emoji" is printed (note that the emoji is a multi-byte character)
393-
// 2. CR moves cursor to start of line
393+
// 2. Carriage return (\r) moves cursor to start of line
394394
// 3. "Line with a" is printed, which partially overwrites the line
395395
// 4. The 'a' character ends at a position that would split the 🚀 emoji
396396
// 5. Instead of creating corrupted output, we insert a space to replace the partial emoji
@@ -402,8 +402,8 @@ describe("processCarriageReturns", () => {
402402
expect(processCarriageReturns(input)).toBe(expected)
403403
})
404404

405-
it("should correctly handle multiple consecutive newlines with carriage returns", () => {
406-
// Another test case for multi-byte character handling during carriage return overwrites.
405+
it("should correctly handle multiple consecutive line feeds (\n) with carriage returns (\r)", () => {
406+
// Another test case for multi-byte character handling during carriage return (\r) overwrites.
407407
// In this case, we're testing with a different emoji and pattern to ensure robustness.
408408
//
409409
// When a new line with an emoji partially overlaps with text from the previous line,
@@ -418,36 +418,36 @@ describe("processCarriageReturns", () => {
418418
expect(processCarriageReturns(input)).toBe(expected)
419419
})
420420

421-
it("should handle carriage returns in the middle of non-ASCII text", () => {
421+
it("should handle carriage returns (\r) in the middle of non-ASCII text", () => {
422422
// Tests handling of non-Latin text (like Chinese characters)
423423
// Non-ASCII text uses multi-byte encodings, so this test verifies our handling works
424424
// properly with such character sets.
425425
//
426426
// Our implementation ensures we preserve character boundaries and don't create
427-
// invalid sequences when carriage returns cause partial overwrites.
427+
// invalid sequences when carriage returns (\r) cause partial overwrites.
428428
const input = "你好世界啊\r你好地球"
429429
const expected = "你好地球啊"
430430
expect(processCarriageReturns(input)).toBe(expected)
431431
})
432432

433-
it("should correctly handle complex patterns of alternating carriage returns and newlines", () => {
433+
it("should correctly handle complex patterns of alternating carriage returns (\r) and line feeds (\n)", () => {
434434
// Break down the example:
435-
// 1. "Line1" + CR + NL: CR moves cursor to start of line, NL moves to next line, preserving "Line1"
436-
// 2. "Line2" + CR: CR moves cursor to start of line
435+
// 1. "Line1" + carriage return (\r) + line feed (\n): carriage return (\r) moves cursor to start of line, line feed (\n) moves to next line, preserving "Line1"
436+
// 2. "Line2" + carriage return (\r): carriage return (\r) moves cursor to start of line
437437
// 3. "Line2Updated" overwrites "Line2"
438-
// 4. NL: moves to next line
439-
// 5. "Line3" + CR + NL: CR moves cursor to start, NL moves to next line, preserving "Line3"
438+
// 4. Line feed (\n): moves to next line
439+
// 5. "Line3" + carriage return (\r) + line feed (\n): carriage return (\r) moves cursor to start, line feed (\n) moves to next line, preserving "Line3"
440440
const input = "Line1\r\nLine2\rLine2Updated\nLine3\r\n"
441441
const expected = "Line1\nLine2Updated\nLine3\n"
442442
expect(processCarriageReturns(input)).toBe(expected)
443443
})
444444

445-
it("should handle partial overwrites with carriage returns", () => {
445+
it("should handle partial overwrites with carriage returns (\r)", () => {
446446
// In this case:
447447
// 1. "Initial text" is printed
448-
// 2. CR moves cursor to start of line
448+
// 2. Carriage return (\r) moves cursor to start of line
449449
// 3. "next" is printed, overwriting only the first 4 chars
450-
// 4. CR moves cursor to start, but nothing follows
450+
// 4. Carriage return (\r) moves cursor to start, but nothing follows
451451
// Final result should be "nextial text" (first 4 chars overwritten)
452452
const input = "Initial text\rnext\r"
453453
const expected = "nextial text"

0 commit comments

Comments
 (0)