Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ Other settings:
- `markdown_view`
- `file_list`
- `file_list_width`
- `diff_signs`: `true` marks changed lines with `+` and `-`, in place of a coloured bar
- `review_prefix`
- `url_opener`
- `focus_agent_after_send`
Expand Down
6 changes: 6 additions & 0 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ type Options struct {
// Wrap continues a line that is wider than Width on the next lines,
// instead of cutting it. Then Offset does not apply.
Wrap bool
// Signs marks a changed line with + or -. Without Signs, a bar in the
// colour of the line marks it.
Signs bool
// Theme sets the colours.
Theme Theme
}
Expand Down Expand Up @@ -96,6 +99,9 @@ func Render(path, old, new string, o Options) *Diff {
if o.Split {
dv = dv.Split()
}
if o.Signs {
dv = dv.Signs()
}
if o.Wrap {
dv = dv.Wrap()
}
Expand Down
89 changes: 88 additions & 1 deletion diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestRenderSameContentIsEmpty(t *testing.T) {

func TestRenderTabsAndOffset(t *testing.T) {
d := diff.Render("x.go", "", "\tx := 1\n", diff.Options{Width: 80})
if got := ansi.Strip(d.Lines[1]); !strings.Contains(got, "+ x := 1") {
if got := ansi.Strip(d.Lines[1]); !strings.Contains(got, "▌ x := 1") {
t.Errorf("tab: line = %q", got)
}
d = diff.Render("x.go", "", "\tx := 1\n", diff.Options{Width: 80, Offset: 6})
Expand All @@ -118,6 +118,93 @@ func TestRenderTabsAndOffset(t *testing.T) {
}
}

// TestMarks checks the mark before the code of each line, in the two
// layouts. By default a bar marks a changed line, as in herdr-reviewr.
// [diff.Options.Signs] gives + and -. The mark takes two cells in both
// cases, so the columns do not move.
func TestMarks(t *testing.T) {
const old, new = "a\nb\nc\n", "a\nB\nc\n"
theme := diff.Theme{
Background: color.Black,
Green: color.RGBA{0, 0xff, 0, 0xff},
Red: color.RGBA{0xff, 0, 0, 0xff},
}
tests := []struct {
name string
o diff.Options
lines []string // the screen lines under the hunk header, without colours
}{
{"unified bar", diff.Options{Width: 40}, []string{
" 1 1 a", " 2 ▌ b", " 2 ▌ B", " 3 3 c",
}},
{"unified signs", diff.Options{Width: 40, Signs: true}, []string{
" 1 1 a", " 2 - b", " 2 + B", " 3 3 c",
}},
{"split bar", diff.Options{Width: 40, Split: true}, []string{
" 1 a 1 a",
" 2 ▌ b 2 ▌ B",
" 3 c 3 c",
}},
{"split signs", diff.Options{Width: 40, Split: true, Signs: true}, []string{
" 1 a 1 a",
" 2 - b 2 + B",
" 3 c 3 c",
}},
}
for _, tt := range tests {
tt.o.Theme = theme
d := diff.Render("x.txt", old, new, tt.o)
if len(d.Lines) != 1+len(tt.lines) {
t.Fatalf("%s: %d lines, want %d", tt.name, len(d.Lines), 1+len(tt.lines))
}
for i, want := range tt.lines {
got := strings.TrimRight(ansi.Strip(d.Lines[i+1]), " ")
if got != want {
t.Errorf("%s: line %d = %q, want %q", tt.name, i+1, got, want)
}
}
// The mark has the colour of the line: ANSI red for a delete, and
// ANSI green for an insert.
for _, c := range []struct {
line int
fg string
}{{2, "31"}, {3, "32"}} {
if tt.o.Split && c.line == 3 {
continue // the split layout has the insert on the same row
}
if got := markColour(d.Lines[c.line]); got != c.fg {
t.Errorf("%s: the mark of line %d has colour %q, want %q", tt.name, c.line, got, c.fg)
}
}
}
// With a scroll to the right, the bar keeps its colour and shows an
// ellipsis after it.
d := diff.Render("x.txt", "", "abcdefgh\n", diff.Options{Width: 40, Offset: 2})
if got := ansi.Strip(d.Lines[1]); !strings.Contains(got, "▌…cdefgh") {
t.Errorf("offset: line = %q, want ▌…cdefgh", got)
}
// The review text of a comment uses + and - for both kinds of mark.
for _, signs := range []bool{false, true} {
d := diff.Render("x.txt", old, new, diff.Options{Width: 40, Signs: signs})
got := snippet(d.Rows[2:5], old, new)
if want := []string{"-b", "+B", " c"}; !slices.Equal(got, want) {
t.Errorf("signs %v: snippet = %q, want %q", signs, got, want)
}
}
}

// markColour returns the foreground of the SGR sequence just before the
// first bar, + or - in line, for example "31".
func markColour(line string) string {
i := strings.IndexAny(line, "▌+-")
if i < 0 {
return ""
}
sgr := line[strings.LastIndex(line[:i], "\x1b[")+2 : i]
fg, _, _ := strings.Cut(strings.TrimSuffix(sgr, "m"), ";")
return fg
}

func TestSideAt(t *testing.T) {
// The unified layout has " 1 " old numbers, " 1 " new numbers, and the
// code.
Expand Down
2 changes: 2 additions & 0 deletions internal/VENDORED.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@
`Height`, `YOffset`, `InfiniteYScroll` (and the height cut and the scroll
code); `MatchLexer`, `LineStyle` and the `Style`
fields are unexported; `RowFile` removed
- a coloured bar `▌` marks a changed line, in place of crush's `+` and `-`; `Signs` gives
the signs back
- the syntax cache key tells "no background" from black, so a black tint does not
leak onto equal lines
19 changes: 15 additions & 4 deletions internal/diffview/diffview.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ const (
ellipsis = "…" // shows where a line is cut
)

// The symbols before the code of a line.
// The symbols before the code of a line. By default, a bar in the colour
// of the line marks an inserted or a deleted line. [View.Signs] changes the
// bar to a sign.
const (
symNone = " " // an equal line, a missing line, or a continued line
symBar = "▌ "
symInsert = "+ "
symDelete = "- "
)
Expand All @@ -52,6 +55,7 @@ type View struct {
style Style
tabWidth int
wrap bool // wrap long lines instead of cutting them
signs bool // mark a changed line with + or -, not with a bar

isComputed bool
err error
Expand Down Expand Up @@ -424,9 +428,15 @@ func fillTo(lines []string, n int, fill func() string) []string {
func (v *View) kindStyle(k udiff.OpKind) (lineStyle, string) {
switch k {
case udiff.Insert:
return v.style.insertLine, symInsert
if v.signs {
return v.style.insertLine, symInsert
}
return v.style.insertLine, symBar
case udiff.Delete:
return v.style.deleteLine, symDelete
if v.signs {
return v.style.deleteLine, symDelete
}
return v.style.deleteLine, symBar
}
return v.style.equalLine, symNone
}
Expand Down Expand Up @@ -467,7 +477,8 @@ func (v *View) codeLines(
code = ansi.GraphemeWidth.Cut(code, v.xOffset, len(code))
parts[0] = ansi.Truncate(code, width, ellipsis)
if v.xOffset > 0 && strings.TrimSpace(parts[0]) != "" {
sym = sym[:1] + ellipsis
sym, _ = strings.CutSuffix(sym, " ")
sym += ellipsis
}
}
full := lipgloss.NewStyle().MaxWidth(width + leadingSymbolsSize)
Expand Down
7 changes: 7 additions & 0 deletions internal/diffview/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ import (
"charm.land/lipgloss/v2"
)

// Signs marks an inserted line with + and a deleted line with -, in place
// of a bar.
func (v *View) Signs() *View {
v.signs = true
return v
}

// Wrap makes long lines continue on the next screen lines, instead of
// cutting them. The x offset does not apply.
func (v *View) Wrap() *View {
Expand Down
2 changes: 2 additions & 0 deletions tui/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ type config struct {
FileList position `json:"file_list"`
// FileListWidth is in columns, minListWidth or more.
FileListWidth int `json:"file_list_width"`
// DiffSigns marks a changed line with + or -, in place of a bar.
DiffSigns bool `json:"diff_signs"`
// FocusAgent tells if the focus moves to the agent after a send. Nil
// means true.
FocusAgent *bool `json:"focus_agent_after_send"`
Expand Down
1 change: 1 addition & 0 deletions tui/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (m *model) diffOptions(xoff int) diff.Options {
Offset: xoff,
Wrap: m.wrap,
Split: m.split,
Signs: m.cfg.DiffSigns,
Theme: diff.Theme{
Background: m.bg,
Foreground: m.fg,
Expand Down
36 changes: 33 additions & 3 deletions tui/tui_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,21 +484,51 @@ func TestParseConfig(t *testing.T) {
"editor": "code -g {file}:{line}",
"gitlab_host": "git.corp.example",
"focus_agent_after_send": false,
"file_list": "left"
"file_list": "left",
"diff_signs": true
}`
c, err := parseConfig(good)
if err != nil {
t.Fatalf("good config: %v", err)
}
if c.focusAgent() || c.FileList != positionLeft || c.Layout != LayoutUnified {
if c.focusAgent() || c.FileList != positionLeft || c.Layout != LayoutUnified || !c.DiffSigns {
t.Errorf("good config = %+v, want no focus, the list on the left, "+
"and the default layout", c)
"the default layout, and signs", c)
}
if c, err := parseConfig(""); err != nil || c.Layout != defaultConfig().Layout {
t.Errorf(`parseConfig("") = %+v, %v; want the defaults`, c, err)
}
}

// TestDiffSigns checks that a bar marks a changed line by default, and
// that the setting diff_signs changes the bar to + and -.
func TestDiffSigns(t *testing.T) {
line := func(m model, code string) string {
for l := range strings.Lines(view(m)) {
if strings.Contains(l, code) {
return l
}
}
return ""
}
m, _ := newTestModel(t)
if l := line(m, "x := 2"); !strings.Contains(l, "▌ x := 2") {
t.Errorf("by default, the inserted line is %q, want a bar", l)
}
cfg, err := parseConfig(`{"diff_signs": true}`)
if err != nil {
t.Fatal(err)
}
m.cfg = cfg
m.rerender()
if l := line(m, "x := 2"); !strings.Contains(l, "+ x := 2") {
t.Errorf("with diff_signs, the inserted line is %q, want +", l)
}
if l := line(m, "return 1"); !strings.Contains(l, "- return 1") {
t.Errorf("with diff_signs, the deleted line is %q, want -", l)
}
}

// TestConfigFlags checks that the flags win over the settings, and that
// the settings set the keys and the file list.
func TestConfigFlags(t *testing.T) {
Expand Down
Loading