From 17e933a16ff7de1b06d62504e78c40e615c8b553 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Tue, 15 Sep 2026 00:29:09 +0200 Subject: review and undo headings wrap past the label column; format characters take a column --- cmd/krino/render.go | 9 ++++++--- cmd/krino/render_test.go | 8 +++++++- cmd/krino/review.go | 7 ++++++- cmd/krino/review_test.go | 39 +++++++++++++++++++++++++++++++++++++++ cmd/krino/undo.go | 12 ++++++++++-- 5 files changed, 68 insertions(+), 7 deletions(-) (limited to 'cmd') diff --git a/cmd/krino/render.go b/cmd/krino/render.go index 7205488..5b75df3 100644 --- a/cmd/krino/render.go +++ b/cmd/krino/render.go @@ -325,8 +325,11 @@ func wrapText(s string, max int) []string { } // cols is how many terminal columns s takes: two for a wide or full-width -// character (CJK), none for a combining mark or format character, one for -// the rest (triage 28j). +// character (CJK), none for a combining mark, one for the rest - format +// characters included: a terminal may draw one (a soft hyphen), and +// counting a column too many only wraps early, while one too few runs past +// the edge (triage 28j, plan 11 review L4). A terminal can still draw some +// characters wider (emoji, ambiguous-width letters); see KNOWN LIMITATIONS. func cols(s string) int { n := 0 for _, r := range s { @@ -336,7 +339,7 @@ func cols(s string) int { } func runeCols(r rune) int { - if unicode.In(r, unicode.Mn, unicode.Me, unicode.Cf) { + if unicode.In(r, unicode.Mn, unicode.Me) { return 0 } switch width.LookupRune(r).Kind() { diff --git a/cmd/krino/render_test.go b/cmd/krino/render_test.go index a964b43..1b2f8cc 100644 --- a/cmd/krino/render_test.go +++ b/cmd/krino/render_test.go @@ -406,9 +406,15 @@ func TestColumnsCountWideAndCombiningCharacters(t *testing.T) { if n := cols("漢字"); n != 4 { t.Errorf("cols(漢字) = %d, want 4", n) } - if n := cols("éx"); n != 2 { + if n := cols("e\u0301x"); n != 2 { t.Errorf("cols(e + combining acute + x) = %d, want 2", n) } + // A format character is counted as one column: a terminal may draw it + // (a soft hyphen), and counting one too many only wraps a line early, + // while one too few lets it run past the edge (plan 11 review L4). + if n := cols("a\u00adb"); n != 3 { + t.Errorf("cols(a + soft hyphen + b) = %d, want 3", n) + } if got := padCell("漢字", 6); got != "漢字 " { t.Errorf("padCell(漢字, 6) = %q, want two spaces of padding", got) } diff --git a/cmd/krino/review.go b/cmd/krino/review.go index dcf7663..8253405 100644 --- a/cmd/krino/review.go +++ b/cmd/krino/review.go @@ -122,7 +122,12 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string continue } - fmt.Fprintf(out, "\n[%d/%d] %s\n", i+1, len(chains), display(c.File.Rel)) + // The heading wraps with its continuation past the label column, so + // a long name cannot pass for a step line (plan 11 review L4). + fmt.Fprintln(out) + for _, l := range wrapped("", fmt.Sprintf("[%d/%d] %s", i+1, len(chains), display(c.File.Rel)), 7+labelWidth+1, widthPolicy(out), plainText) { + fmt.Fprintln(out, l) + } for _, l := range stepLines(c, 7, root, p, widthPolicy(out)) { fmt.Fprintln(out, l) } diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go index b138451..fc175cb 100644 --- a/cmd/krino/review_test.go +++ b/cmd/krino/review_test.go @@ -11,6 +11,7 @@ import ( "testing" "unicode/utf8" + "krino/internal/engine" "krino/internal/plan" "krino/internal/scan" ) @@ -326,3 +327,41 @@ func TestStopAfterApply(t *testing.T) { } } } + +// TestReviewHeadingsCannotFakeAStepLine: a long name in the per-file +// heading of review and of undo wraps with its continuation past the column +// step labels use, so the name cannot pass for a step (plan 11 review L4). +func TestReviewHeadingsCannotFakeAStepLine(t *testing.T) { + old := widthPolicy + t.Cleanup(func() { widthPolicy = old }) + widthPolicy = func(io.Writer) int { return 60 } + name := strings.Repeat("x", 50) + " DELETE permanently" + + out := new(strings.Builder) + cs := []plan.Chain{{File: scan.File{Rel: name}, Steps: []plan.Step{{Kind: plan.Move, Rule: "r", Dst: "/w/Out/a.pdf"}}}} + if _, _, _, err := reviewChains(strings.NewReader("cy"), out, cs, "/w", palette{}); err != nil { + t.Fatal(err) + } + for _, l := range strings.Split(out.String(), "\n") { + if strings.HasPrefix(strings.TrimLeft(l, " "), "DELETE") { + t.Errorf("review: a line reads as a step: %q\n%s", l, out) + } + if n := cols(l); n > 60 { + t.Errorf("review: a line of %d columns, over 60, which the terminal wraps: %q", n, l) + } + } + + out.Reset() + files := []engine.UndoFile{{Dir: "dl", File: name, Steps: []engine.UndoStep{{Action: "undo-move", Src: "/t/a", Dst: "/s/a"}}}} + if _, _, err := reviewUndoFiles(strings.NewReader("cy"), out, files, palette{}); err != nil { + t.Fatal(err) + } + for _, l := range strings.Split(out.String(), "\n") { + if strings.HasPrefix(strings.TrimLeft(l, " "), "DELETE") { + t.Errorf("undo: a line reads as a step: %q\n%s", l, out) + } + if n := cols(l); n > 60 { + t.Errorf("undo: a line of %d columns, over 60, which the terminal wraps: %q", n, l) + } + } +} diff --git a/cmd/krino/undo.go b/cmd/krino/undo.go index 4c71fdd..ecab2f3 100644 --- a/cmd/krino/undo.go +++ b/cmd/krino/undo.go @@ -404,9 +404,17 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p approved = map[int]bool{} yesRest := false for i, f := range files { - fmt.Fprintf(out, "\n[%d/%d] %s/%s\n", i+1, len(files), display(f.Dir), display(f.File)) + // Heading and steps wrap past the action column, so a long name or + // path cannot pass for a step line (plan 11 review L4). + width := widthPolicy(out) + fmt.Fprintln(out) + for _, l := range wrapped("", fmt.Sprintf("[%d/%d] %s/%s", i+1, len(files), display(f.Dir), display(f.File)), 7+undoStepWidth+2, width, plainText) { + fmt.Fprintln(out, l) + } for _, s := range f.Steps { - fmt.Fprintf(out, " %s\n", undoActionCell(s)) + for _, l := range wrapped(" ", undoActionCell(s), 7+undoStepWidth+2, width, plainText) { + fmt.Fprintln(out, l) + } } if f.Refused != "" { fmt.Fprintf(out, " refused: %s\n", display(f.Refused)) -- cgit v1.3