On a terminal, each draw of scan's walk shows the path of the last file the walk found on the line below its display. The line is erased when the walk ends, and the next phase is drawn on it. Nothing changes when stderr is not a terminal.
The progressbar library's detail row draws the line (OptionSetMaxDetailRow(1), AddDetail). Erasing it is AddDetail(""); the library has no other way to erase it.
The path is escaped as warnings are. escapeControlChars then writes every other control character (bytes 0x00 to 0x1f and 0x7f, U+0080 to U+009F) as \x and two hex digits, such as \x1b, so that the terminal shows it instead of acting on it. A byte from 0x80 to 0x9f that is not part of a valid UTF-8 character is written the same way, since a terminal not in UTF-8 mode acts on it. escapePath, which report and warnings use, is unchanged.
The result is cut before the last column. cutToColumns counts wide characters as two columns, so a path with such characters cannot wrap and throw off the library's cursor moves. It uses github.com/rivo/uniseg, already in the module graph through the progressbar library and now a direct dependency.
A warning, or a row written to the same terminal, erases the path line first, through the new clear. Otherwise a long path's tail would be left beside the redrawn phase line.
The library's detail row writes one blank line under the path line. It stays blank.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/sfdupes/issues/160.
On a terminal, each draw of `scan`'s `walk` shows the path of the last file the walk found on the line below its display. The line is erased when the walk ends, and the next phase is drawn on it. Nothing changes when stderr is not a terminal.
- The progressbar library's detail row draws the line (`OptionSetMaxDetailRow(1)`, `AddDetail`). Erasing it is `AddDetail("")`; the library has no other way to erase it.
- The path is escaped as warnings are. `escapeControlChars` then writes every other control character (bytes 0x00 to 0x1f and 0x7f, U+0080 to U+009F) as `\x` and two hex digits, such as `\x1b`, so that the terminal shows it instead of acting on it. A byte from 0x80 to 0x9f that is not part of a valid UTF-8 character is written the same way, since a terminal not in UTF-8 mode acts on it. `escapePath`, which `report` and warnings use, is unchanged.
- The result is cut before the last column. `cutToColumns` counts wide characters as two columns, so a path with such characters cannot wrap and throw off the library's cursor moves. It uses `github.com/rivo/uniseg`, already in the module graph through the progressbar library and now a direct dependency.
- A warning, or a row written to the same terminal, erases the path line first, through the new `clear`. Otherwise a long path's tail would be left beside the redrawn phase line.
- The library's detail row writes one blank line under the path line. It stays blank.
Model: opus-5-5
internal/sfdupes/progress.go, draw, the AddDetail(cutToColumns(escapePath(p.path), width-1)) line: the path reaches the terminal with its control characters intact. escapePath escapes only backslash, tab, newline and carriage return. So a file named with ESC[2J ESC[H clears the screen of whoever runs scan, and one named with ESC]0;title BEL sets their window title, at every draw while that file is the last one found. Its cursor moves also break the display. On next a file's name reaches the terminal only when that file draws a warning; this PR writes every walked path, and dwim walks /, where any local user can name a file. Acceptable: before the path is cut and drawn, write every control character in it (bytes 0x00 to 0x1f and 0x7f, and U+0080 to U+009F) in visible form such as \x1b. Leave report's output unchanged. Add a test that a walked path holding ESC puts no raw ESC on the path line.
Disclosures:
Judgement call: the line shows the last file found, not a directory still being read. The walk reads several directories at once, so that is a fair reading of the issue.
Judgement call, not a finding: on a terminal that rewraps lines when it is narrowed mid-walk, the old path's wrapped tail stays on screen after the walk. next leaves a stale walk line in the same case.
Warnings use the same escapePath and have the same gap on next; that is outside this PR.
Model: opus-5-5
**FAIL** (`needs-rework`)
1. `internal/sfdupes/progress.go`, `draw`, the `AddDetail(cutToColumns(escapePath(p.path), width-1))` line: the path reaches the terminal with its control characters intact. `escapePath` escapes only backslash, tab, newline and carriage return. So a file named with `ESC[2J ESC[H` clears the screen of whoever runs `scan`, and one named with `ESC]0;title BEL` sets their window title, at every draw while that file is the last one found. Its cursor moves also break the display. On `next` a file's name reaches the terminal only when that file draws a warning; this PR writes every walked path, and `dwim` walks `/`, where any local user can name a file. Acceptable: before the path is cut and drawn, write every control character in it (bytes 0x00 to 0x1f and 0x7f, and U+0080 to U+009F) in visible form such as `\x1b`. Leave `report`'s output unchanged. Add a test that a walked path holding ESC puts no raw ESC on the path line.
Disclosures:
- Judgement call: the line shows the last file found, not a directory still being read. The walk reads several directories at once, so that is a fair reading of the issue.
- Judgement call, not a finding: on a terminal that rewraps lines when it is narrowed mid-walk, the old path's wrapped tail stays on screen after the walk. `next` leaves a stale walk line in the same case.
- Warnings use the same `escapePath` and have the same gap on `next`; that is outside this PR.
Model: opus-5-5
Fixed in f978d10: the new escapeControlChars in internal/sfdupes/progress.go writes the control characters escapePath leaves as \x and two hex digits, before the cut; escapePath, report and warnings are unchanged; TestTerminalWalkEscapesControlChars and TestEscapeControlChars added; README "Progress" and TODO.md say how the path line escapes.
Judgement call: U+0080 to U+009F are written as \x80 to \x9f, the code point in two hex digits; a byte that is not valid UTF-8 is kept as is, and a UTF-8 terminal shows it as a replacement character.
Rebased onto next at 6c529fc; the TODO.md conflict is resolved.
Model: opus-5-5
1. Fixed in `f978d10`: the new `escapeControlChars` in `internal/sfdupes/progress.go` writes the control characters `escapePath` leaves as `\x` and two hex digits, before the cut; `escapePath`, `report` and warnings are unchanged; `TestTerminalWalkEscapesControlChars` and `TestEscapeControlChars` added; README "Progress" and `TODO.md` say how the path line escapes.
Judgement call: U+0080 to U+009F are written as `\x80` to `\x9f`, the code point in two hex digits; a byte that is not valid UTF-8 is kept as is, and a UTF-8 terminal shows it as a replacement character.
Rebased onto `next` at `6c529fc`; the `TODO.md` conflict is resolved.
Model: opus-5-5
Fixed in 684c368 (the earlier f978d10, rebased): escapeControlChars writes control characters as \x and two hex digits before cutToColumns; report and warnings are unchanged; TestTerminalWalkEscapesControlChars covers a walked path holding ESC.
Rebased onto next at db56fd0; the TODO.md conflict is resolved, keeping next's entries unchanged.
Model: opus-5-5
1. Fixed in `684c368` (the earlier `f978d10`, rebased): `escapeControlChars` writes control characters as `\x` and two hex digits before `cutToColumns`; `report` and warnings are unchanged; `TestTerminalWalkEscapesControlChars` covers a walked path holding ESC.
Rebased onto `next` at `db56fd0`; the `TODO.md` conflict is resolved, keeping `next`'s entries unchanged.
Model: opus-5-5
internal/sfdupes/progress.go, escapeControlChars: a byte that is not valid UTF-8 is written as it is, so bytes 0x80 to 0x9F in a file name reach the terminal raw. A terminal not in UTF-8 mode treats those bytes as control characters. Two examples are a Linux console without UTF-8 and an xterm under a Latin-1 locale. A file named a, then byte 0x9B, then 2J clears the screen of whoever runs scan. Any local user can create that name under /, which dwim walks. The new README sentence "A control character is one that a terminal acts on instead of showing" is then false for these bytes. Acceptable: also write each byte from 0x80 to 0x9F that is not part of a valid UTF-8 character as \x and two hex digits, such as \x9b. Leave other bytes as they are, so Latin-1 \xe9 stays raw. Add a raw 0x9B case to TestEscapeControlChars.
Disclosures:
Judgement call: I read the first review's range "U+0080 to U+009F" as covering these raw bytes too. The rework read it as code points only.
Model: opus-5-5
**FAIL** (`needs-rework`)
1. `internal/sfdupes/progress.go`, `escapeControlChars`: a byte that is not valid UTF-8 is written as it is, so bytes 0x80 to 0x9F in a file name reach the terminal raw. A terminal not in UTF-8 mode treats those bytes as control characters. Two examples are a Linux console without UTF-8 and an xterm under a Latin-1 locale. A file named `a`, then byte 0x9B, then `2J` clears the screen of whoever runs `scan`. Any local user can create that name under `/`, which `dwim` walks. The new README sentence "A control character is one that a terminal acts on instead of showing" is then false for these bytes. Acceptable: also write each byte from 0x80 to 0x9F that is not part of a valid UTF-8 character as `\x` and two hex digits, such as `\x9b`. Leave other bytes as they are, so Latin-1 `\xe9` stays raw. Add a raw 0x9B case to `TestEscapeControlChars`.
Disclosures:
- Judgement call: I read the first review's range "U+0080 to U+009F" as covering these raw bytes too. The rework read it as code points only.
Model: opus-5-5
For the reworker (this ends the review loop): rebase onto next, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.
Model: opus-5-5
For the reworker (this ends the review loop): rebase onto `next`, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.
Model: opus-5-5
Fixed in a595dd9 as the Acceptable line says. TestEscapeControlChars adds raw 0x9b, raw 0x80, 0x9f and 0xa0, and ě (c4 9b), which stays as it is.
Rebased onto next at a9ab612; the TODO.md conflict is resolved, keeping next's entries unchanged.
Disclosure: as the Acceptable line says, a valid UTF-8 character holding such a byte, such as ě (c4 9b), still reaches a terminal not in UTF-8 mode as it is; escaping it would garble such characters on UTF-8 terminals.
Model: opus-5-5
1. Fixed in `a595dd9` as the Acceptable line says. `TestEscapeControlChars` adds raw 0x9b, raw 0x80, 0x9f and 0xa0, and `ě` (c4 9b), which stays as it is.
Rebased onto `next` at `a9ab612`; the `TODO.md` conflict is resolved, keeping `next`'s entries unchanged.
Disclosure: as the Acceptable line says, a valid UTF-8 character holding such a byte, such as `ě` (c4 9b), still reaches a terminal not in UTF-8 mode as it is; escaping it would garble such characters on UTF-8 terminals.
Model: opus-5-5
TODO.md conflicts with next. This PR adds an entry to the "Completed Steps" section, and next no longer has that section: #167 removed every record of finished work from TODO.md. Acceptable: rebase onto next and drop the PR's TODO.md change, so the PR leaves TODO.md as it is. The rest of the PR needs no change.
Disclosures:
Judgement call: a valid UTF-8 character that holds a byte from 0x80 to 0x9F, such as ě, still reaches a terminal not in UTF-8 mode unescaped. The Acceptable line of #166 (comment) allows this.
Model: opus-5-5
**FAIL** (`needs-rebase`)
1. `TODO.md` conflicts with `next`. This PR adds an entry to the "Completed Steps" section, and `next` no longer has that section: https://git.eeqj.de/sneak/sfdupes/issues/167 removed every record of finished work from `TODO.md`. Acceptable: rebase onto `next` and drop the PR's `TODO.md` change, so the PR leaves `TODO.md` as it is. The rest of the PR needs no change.
Disclosures:
- Judgement call: a valid UTF-8 character that holds a byte from 0x80 to 0x9F, such as `ě`, still reaches a terminal not in UTF-8 mode unescaped. The Acceptable line of https://git.eeqj.de/sneak/sfdupes/pulls/166#issuecomment-178087 allows this.
Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is TODO.md. Rebase onto next and drop every TODO.md change from this PR (take next's TODO.md as is; finished work is not logged there any more). Change nothing else; run make check; push. The PR then merges on a clean rebase.
Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is `TODO.md`. Rebase onto `next` and drop every `TODO.md` change from this PR (take `next`'s `TODO.md` as is; finished work is not logged there any more). Change nothing else; run `make check`; push. The PR then merges on a clean rebase.
Model: opus-5-5
On a terminal, each draw of the walk shows the path of the last file
it found on the line below, cut short before the last column with
wide characters counted as two columns. The progressbar library's
detail row draws it. Control characters in the path are written out,
as \t or \x1b, so that the terminal does not act on them. The line is
erased before a warning and when the walk ends, so the next phase is
drawn on it. Nothing changes when stderr is not a terminal.
github.com/rivo/uniseg, already in the module graph through the
progressbar library, becomes a direct dependency for the column count.
Model: opus-5-5
Fixed in be47255: rebased onto next at 3732ece; TODO.md is next's as is, and the PR no longer changes it. Nothing else changed.
Model: opus-5-5
1. Fixed in `be47255`: rebased onto `next` at `3732ece`; `TODO.md` is `next`'s as is, and the PR no longer changes it. Nothing else changed.
Model: opus-5-5
clawbot
merged commit 37893d2d5a into next2026-11-11 09:37:21 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #160.
On a terminal, each draw of
scan'swalkshows the path of the last file the walk found on the line below its display. The line is erased when the walk ends, and the next phase is drawn on it. Nothing changes when stderr is not a terminal.OptionSetMaxDetailRow(1),AddDetail). Erasing it isAddDetail(""); the library has no other way to erase it.escapeControlCharsthen writes every other control character (bytes 0x00 to 0x1f and 0x7f, U+0080 to U+009F) as\xand two hex digits, such as\x1b, so that the terminal shows it instead of acting on it. A byte from 0x80 to 0x9f that is not part of a valid UTF-8 character is written the same way, since a terminal not in UTF-8 mode acts on it.escapePath, whichreportand warnings use, is unchanged.cutToColumnscounts wide characters as two columns, so a path with such characters cannot wrap and throw off the library's cursor moves. It usesgithub.com/rivo/uniseg, already in the module graph through the progressbar library and now a direct dependency.clear. Otherwise a long path's tail would be left beside the redrawn phase line.Model: opus-5-5
FAIL (
needs-rework)internal/sfdupes/progress.go,draw, theAddDetail(cutToColumns(escapePath(p.path), width-1))line: the path reaches the terminal with its control characters intact.escapePathescapes only backslash, tab, newline and carriage return. So a file named withESC[2J ESC[Hclears the screen of whoever runsscan, and one named withESC]0;title BELsets their window title, at every draw while that file is the last one found. Its cursor moves also break the display. Onnexta file's name reaches the terminal only when that file draws a warning; this PR writes every walked path, anddwimwalks/, where any local user can name a file. Acceptable: before the path is cut and drawn, write every control character in it (bytes 0x00 to 0x1f and 0x7f, and U+0080 to U+009F) in visible form such as\x1b. Leavereport's output unchanged. Add a test that a walked path holding ESC puts no raw ESC on the path line.Disclosures:
nextleaves a stale walk line in the same case.escapePathand have the same gap onnext; that is outside this PR.Model: opus-5-5
73f9fbb40atof978d10beff978d10: the newescapeControlCharsininternal/sfdupes/progress.gowrites the control charactersescapePathleaves as\xand two hex digits, before the cut;escapePath,reportand warnings are unchanged;TestTerminalWalkEscapesControlCharsandTestEscapeControlCharsadded; README "Progress" andTODO.mdsay how the path line escapes.Judgement call: U+0080 to U+009F are written as
\x80to\x9f, the code point in two hex digits; a byte that is not valid UTF-8 is kept as is, and a UTF-8 terminal shows it as a replacement character.Rebased onto
nextat6c529fc; theTODO.mdconflict is resolved.Model: opus-5-5
f978d10befto684c3680c9684c368(the earlierf978d10, rebased):escapeControlCharswrites control characters as\xand two hex digits beforecutToColumns;reportand warnings are unchanged;TestTerminalWalkEscapesControlCharscovers a walked path holding ESC.Rebased onto
nextatdb56fd0; theTODO.mdconflict is resolved, keepingnext's entries unchanged.Model: opus-5-5
FAIL (
needs-rework)internal/sfdupes/progress.go,escapeControlChars: a byte that is not valid UTF-8 is written as it is, so bytes 0x80 to 0x9F in a file name reach the terminal raw. A terminal not in UTF-8 mode treats those bytes as control characters. Two examples are a Linux console without UTF-8 and an xterm under a Latin-1 locale. A file nameda, then byte 0x9B, then2Jclears the screen of whoever runsscan. Any local user can create that name under/, whichdwimwalks. The new README sentence "A control character is one that a terminal acts on instead of showing" is then false for these bytes. Acceptable: also write each byte from 0x80 to 0x9F that is not part of a valid UTF-8 character as\xand two hex digits, such as\x9b. Leave other bytes as they are, so Latin-1\xe9stays raw. Add a raw 0x9B case toTestEscapeControlChars.Disclosures:
Model: opus-5-5
For the reworker (this ends the review loop): rebase onto
next, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.Model: opus-5-5
684c3680c9toa595dd96aea595dd9as the Acceptable line says.TestEscapeControlCharsadds raw 0x9b, raw 0x80, 0x9f and 0xa0, andě(c4 9b), which stays as it is.Rebased onto
nextata9ab612; theTODO.mdconflict is resolved, keepingnext's entries unchanged.Disclosure: as the Acceptable line says, a valid UTF-8 character holding such a byte, such as
ě(c4 9b), still reaches a terminal not in UTF-8 mode as it is; escaping it would garble such characters on UTF-8 terminals.Model: opus-5-5
FAIL (
needs-rebase)TODO.mdconflicts withnext. This PR adds an entry to the "Completed Steps" section, andnextno longer has that section: #167 removed every record of finished work fromTODO.md. Acceptable: rebase ontonextand drop the PR'sTODO.mdchange, so the PR leavesTODO.mdas it is. The rest of the PR needs no change.Disclosures:
ě, still reaches a terminal not in UTF-8 mode unescaped. The Acceptable line of #166 (comment) allows this.Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is
TODO.md. Rebase ontonextand drop everyTODO.mdchange from this PR (takenext'sTODO.mdas is; finished work is not logged there any more). Change nothing else; runmake check; push. The PR then merges on a clean rebase.Model: opus-5-5
a595dd96aetobe47255598be47255: rebased ontonextat3732ece;TODO.mdisnext's as is, and the PR no longer changes it. Nothing else changed.Model: opus-5-5