detached ab6686c

fix(kittygraphics): advance cursor by derived cells when c or r is zero

Treat c=0/r=0 as unset when sizing cursor movement, validate deriveCells
inputs, and cover cursor movement, known cell pixels, a=p placements and
overflow in tests.
commit
ab6686c0df2c672931f719eeddf9864d3d88deb8
author
brice · · 2026-10-21 07:17 UTC
parent
0c10420
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
checks
12/12
refs
maintag: v0.7.2
4 files · +57 -20
Unified
UnifiedSplit
M protocol/kittygraphics/kittygraphics_test.go +24 -0
@@ -647,3 +647,27 @@ func TestPlacementZeroColumnsAndRowsMeansAuto(t *testing.T) {
647647 t.Fatalf("cells = %#v, want none", p.Cells())
648648 }
649649 }
650+
651+func TestDeriveCells(t *testing.T) {
652+ const maxInt64 = int64(^uint64(0) >> 1)
653+ tests := []struct {
654+ name string
655+ given, givenSrc, otherSrc, givenCell, otherCell int64
656+ want int64
657+ wantOK bool
658+ }{
659+ {"exact division", 4, 4, 4, 10, 20, 2, true},
660+ {"rounds up", 5, 4, 4, 10, 20, 3, true},
661+ {"tiny result is at least one", 1, 1000, 1, 1, 1, 1, true},
662+ {"overflow", maxInt64, 1, 1000, 1000, 1, 0, false},
663+ {"zero source", 5, 0, 4, 10, 20, 0, false},
664+ }
665+ for _, tt := range tests {
666+ t.Run(tt.name, func(t *testing.T) {
667+ got, ok := deriveCells(tt.given, tt.givenSrc, tt.otherSrc, tt.givenCell, tt.otherCell)
668+ if got != tt.want || ok != tt.wantOK {
669+ t.Fatalf("deriveCells = %d, %v; want %d, %v", got, ok, tt.want, tt.wantOK)
670+ }
671+ })
672+ }
673+}
@@ -647,3 +647,27 @@ func TestPlacementZeroColumnsAndRowsMeansAuto(t *testing.T) {
647 t.Fatalf("cells = %#v, want none", p.Cells())647 t.Fatalf("cells = %#v, want none", p.Cells())
648 }648 }
649 }649 }
650+
651+func TestDeriveCells(t *testing.T) {
652+ const maxInt64 = int64(^uint64(0) >> 1)
653+ tests := []struct {
654+ name string
655+ given, givenSrc, otherSrc, givenCell, otherCell int64
656+ want int64
657+ wantOK bool
658+ }{
659+ {"exact division", 4, 4, 4, 10, 20, 2, true},
660+ {"rounds up", 5, 4, 4, 10, 20, 3, true},
661+ {"tiny result is at least one", 1, 1000, 1, 1, 1, 1, true},
662+ {"overflow", maxInt64, 1, 1000, 1000, 1, 0, false},
663+ {"zero source", 5, 0, 4, 10, 20, 0, false},
664+ }
665+ for _, tt := range tests {
666+ t.Run(tt.name, func(t *testing.T) {
667+ got, ok := deriveCells(tt.given, tt.givenSrc, tt.otherSrc, tt.givenCell, tt.otherCell)
668+ if got != tt.want || ok != tt.wantOK {
669+ t.Fatalf("deriveCells = %d, %v; want %d, %v", got, ok, tt.want, tt.wantOK)
670+ }
671+ })
672+ }
673+}
M protocol/kittygraphics/session.go +6 -8
@@ -660,22 +660,20 @@ func placementSpec(c Controls, assetID graphics.AssetID, width, height int64, or
660660 }
661661
662662 // deriveCells returns the cell count along the unspecified axis that preserves
663-// the source aspect ratio: ceil(given*givenCell*otherSrc / (givenSrc*otherCell)),
664-// at least 1. The boolean is false if the result does not fit in int64.
663+// the source aspect ratio: ceil(given*givenCell*otherSrc / (givenSrc*otherCell)).
664+// All inputs are positive, so the result is at least 1. The boolean is false
665+// if an input is not positive or the result does not fit in int64.
665666 func deriveCells(given, givenSrc, otherSrc, givenCell, otherCell int64) (int64, bool) {
667+ if given <= 0 || givenSrc <= 0 || otherSrc <= 0 || givenCell <= 0 || otherCell <= 0 {
668+ return 0, false
669+ }
666670 num := new(big.Int).Mul(big.NewInt(given), big.NewInt(givenCell))
667671 num.Mul(num, big.NewInt(otherSrc))
668672 den := new(big.Int).Mul(big.NewInt(givenSrc), big.NewInt(otherCell))
669- if den.Sign() <= 0 {
670- return 0, false
671- }
672673 q, r := new(big.Int).QuoRem(num, den, new(big.Int))
673674 if r.Sign() != 0 {
674675 q.Add(q, big.NewInt(1))
675676 }
676- if q.Sign() == 0 {
677- return 1, true
678- }
679677 if !q.IsInt64() {
680678 return 0, false
681679 }
@@ -660,22 +660,20 @@ func placementSpec(c Controls, assetID graphics.AssetID, width, height int64, or
660 }660 }
661 661
662 // deriveCells returns the cell count along the unspecified axis that preserves662 // deriveCells returns the cell count along the unspecified axis that preserves
663-// the source aspect ratio: ceil(given*givenCell*otherSrc / (givenSrc*otherCell)),663+// the source aspect ratio: ceil(given*givenCell*otherSrc / (givenSrc*otherCell)).
664-// at least 1. The boolean is false if the result does not fit in int64.664+// All inputs are positive, so the result is at least 1. The boolean is false
665+// if an input is not positive or the result does not fit in int64.
665 func deriveCells(given, givenSrc, otherSrc, givenCell, otherCell int64) (int64, bool) {666 func deriveCells(given, givenSrc, otherSrc, givenCell, otherCell int64) (int64, bool) {
667+ if given <= 0 || givenSrc <= 0 || otherSrc <= 0 || givenCell <= 0 || otherCell <= 0 {
668+ return 0, false
669+ }
666 num := new(big.Int).Mul(big.NewInt(given), big.NewInt(givenCell))670 num := new(big.Int).Mul(big.NewInt(given), big.NewInt(givenCell))
667 num.Mul(num, big.NewInt(otherSrc))671 num.Mul(num, big.NewInt(otherSrc))
668 den := new(big.Int).Mul(big.NewInt(givenSrc), big.NewInt(otherCell))672 den := new(big.Int).Mul(big.NewInt(givenSrc), big.NewInt(otherCell))
669- if den.Sign() <= 0 {
670- return 0, false
671- }
672 q, r := new(big.Int).QuoRem(num, den, new(big.Int))673 q, r := new(big.Int).QuoRem(num, den, new(big.Int))
673 if r.Sign() != 0 {674 if r.Sign() != 0 {
674 q.Add(q, big.NewInt(1))675 q.Add(q, big.NewInt(1))
675 }676 }
676- if q.Sign() == 0 {
677- return 1, true
678- }
679 if !q.IsInt64() {677 if !q.IsInt64() {
680 return 0, false678 return 0, false
681 }679 }
M screen.go +8 -5
@@ -616,14 +616,17 @@ func (s *Screen) advanceKittyCursorRows(rows int) {
616616 }
617617
618618 func (s *Screen) kittyPlacementGridSize(c kittygraphics.Controls, mutation kittygraphics.Mutation) (columns, rows int) {
619+ // c=0 and r=0 mean unset, as in Kitty.
620+ hasColumns := c.HasColumns && c.Columns != 0
621+ hasRows := c.HasRows && c.Rows != 0
619622 columns, rows = 1, 1
620- if c.HasColumns {
623+ if hasColumns {
621624 columns = max(kittyCursorAdvance(c.Columns), 1)
622625 }
623- if c.HasRows {
626+ if hasRows {
624627 rows = max(kittyCursorAdvance(c.Rows), 1)
625628 }
626- if c.HasColumns && c.HasRows || s.graphics == nil || s.graphics.scene == nil {
629+ if hasColumns && hasRows || s.graphics == nil || s.graphics.scene == nil {
627630 return columns, rows
628631 }
629632 placement, ok := s.graphics.scene.Snapshot().Placement(mutation.ScenePlacementID)
@@ -639,11 +642,11 @@ func (s *Screen) kittyPlacementGridSize(c kittygraphics.Controls, mutation kitty
639642 return columns, rows
640643 }
641644 destination := placement.Destination()
642- if !c.HasColumns {
645+ if !hasColumns {
643646 offset := destination.X % int64(cellWidth)
644647 columns = kittyPixelCellSpan(offset, destination.Width, cellWidth)
645648 }
646- if !c.HasRows {
649+ if !hasRows {
647650 offset := destination.Y % int64(cellHeight)
648651 rows = kittyPixelCellSpan(offset, destination.Height, cellHeight)
649652 }
@@ -616,14 +616,17 @@ func (s *Screen) advanceKittyCursorRows(rows int) {
616 }616 }
617 617
618 func (s *Screen) kittyPlacementGridSize(c kittygraphics.Controls, mutation kittygraphics.Mutation) (columns, rows int) {618 func (s *Screen) kittyPlacementGridSize(c kittygraphics.Controls, mutation kittygraphics.Mutation) (columns, rows int) {
619+ // c=0 and r=0 mean unset, as in Kitty.
620+ hasColumns := c.HasColumns && c.Columns != 0
621+ hasRows := c.HasRows && c.Rows != 0
619 columns, rows = 1, 1622 columns, rows = 1, 1
620- if c.HasColumns {623+ if hasColumns {
621 columns = max(kittyCursorAdvance(c.Columns), 1)624 columns = max(kittyCursorAdvance(c.Columns), 1)
622 }625 }
623- if c.HasRows {626+ if hasRows {
624 rows = max(kittyCursorAdvance(c.Rows), 1)627 rows = max(kittyCursorAdvance(c.Rows), 1)
625 }628 }
626- if c.HasColumns && c.HasRows || s.graphics == nil || s.graphics.scene == nil {629+ if hasColumns && hasRows || s.graphics == nil || s.graphics.scene == nil {
627 return columns, rows630 return columns, rows
628 }631 }
629 placement, ok := s.graphics.scene.Snapshot().Placement(mutation.ScenePlacementID)632 placement, ok := s.graphics.scene.Snapshot().Placement(mutation.ScenePlacementID)
@@ -639,11 +642,11 @@ func (s *Screen) kittyPlacementGridSize(c kittygraphics.Controls, mutation kitty
639 return columns, rows642 return columns, rows
640 }643 }
641 destination := placement.Destination()644 destination := placement.Destination()
642- if !c.HasColumns {645+ if !hasColumns {
643 offset := destination.X % int64(cellWidth)646 offset := destination.X % int64(cellWidth)
644 columns = kittyPixelCellSpan(offset, destination.Width, cellWidth)647 columns = kittyPixelCellSpan(offset, destination.Width, cellWidth)
645 }648 }
646- if !c.HasRows {649+ if !hasRows {
647 offset := destination.Y % int64(cellHeight)650 offset := destination.Y % int64(cellHeight)
648 rows = kittyPixelCellSpan(offset, destination.Height, cellHeight)651 rows = kittyPixelCellSpan(offset, destination.Height, cellHeight)
649 }652 }
M screen_graphics_test.go +19 -7
@@ -471,25 +471,37 @@ func TestScreenKittySingleColumnOrRowPlacement(t *testing.T) {
471471 var buf bytes.Buffer
472472 require.NoError(t, png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 4, 4))))
473473 payload := base64.StdEncoding.EncodeToString(buf.Bytes())
474+ // 4x4 image. Unknown pixel size uses a 1:2 cell; the 10x20 cell geometry
475+ // comes from SetGeometry. Without C=1 the cursor moves past the placement.
474476 tests := []struct {
475- name string
476- header string
477- cols, rows int64
477+ name string
478+ geometry bool
479+ commands string
480+ cols, rows int64
481+ wantCol, wantRow int
478482 }{
479- {"columns only", "c=5", 5, 3},
480- {"rows only", "r=3", 6, 3},
481- {"both", "c=5,r=3", 5, 3},
483+ {"columns only", false, "a=T,f=100,c=5;" + payload, 5, 3, 5, 2},
484+ {"rows only", false, "a=T,f=100,r=3;" + payload, 6, 3, 6, 2},
485+ {"zero rows is unset", false, "a=T,f=100,c=5,r=0;" + payload, 5, 3, 5, 2},
486+ {"zero columns is unset", false, "a=T,f=100,c=0,r=3;" + payload, 6, 3, 6, 2},
487+ {"known cell pixels", true, "a=T,f=100,c=4;" + payload, 4, 2, 4, 1},
488+ {"put existing image", false, "a=t,i=7,f=100;" + payload + "\x1b\\\x1b_Ga=p,i=7,q=2,c=5", 5, 3, 5, 2},
482489 }
483490 for _, tt := range tests {
484491 t.Run(tt.name, func(t *testing.T) {
485492 screen := NewScreen(100, 30)
486- screen.Write([]byte("\x1b_Ga=T,f=100,C=1," + tt.header + ";" + payload + "\x1b\\"))
493+ if tt.geometry {
494+ screen.SetGeometry(Geometry{Cols: 100, Rows: 30, PixelWidth: 1000, PixelHeight: 600})
495+ }
496+ screen.Write([]byte("\x1b_G" + tt.commands + "\x1b\\"))
487497 usage := screen.GraphicsSnapshot().Usage()
488498 require.Equal(t, uint64(1), usage.Assets)
489499 require.Equal(t, uint64(1), usage.Placements)
490500 cells := screen.GraphicsSnapshot().Placements()[0].Cells()
491501 require.Equal(t, tt.cols, cells.Width)
492502 require.Equal(t, tt.rows, cells.Height)
503+ require.Equal(t, tt.wantCol, screen.Col)
504+ require.Equal(t, tt.wantRow, screen.Row)
493505 })
494506 }
495507 }
@@ -471,25 +471,37 @@ func TestScreenKittySingleColumnOrRowPlacement(t *testing.T) {
471 var buf bytes.Buffer471 var buf bytes.Buffer
472 require.NoError(t, png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 4, 4))))472 require.NoError(t, png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 4, 4))))
473 payload := base64.StdEncoding.EncodeToString(buf.Bytes())473 payload := base64.StdEncoding.EncodeToString(buf.Bytes())
474+ // 4x4 image. Unknown pixel size uses a 1:2 cell; the 10x20 cell geometry
475+ // comes from SetGeometry. Without C=1 the cursor moves past the placement.
474 tests := []struct {476 tests := []struct {
475- name string477+ name string
476- header string478+ geometry bool
477- cols, rows int64479+ commands string
480+ cols, rows int64
481+ wantCol, wantRow int
478 }{482 }{
479- {"columns only", "c=5", 5, 3},483+ {"columns only", false, "a=T,f=100,c=5;" + payload, 5, 3, 5, 2},
480- {"rows only", "r=3", 6, 3},484+ {"rows only", false, "a=T,f=100,r=3;" + payload, 6, 3, 6, 2},
481- {"both", "c=5,r=3", 5, 3},485+ {"zero rows is unset", false, "a=T,f=100,c=5,r=0;" + payload, 5, 3, 5, 2},
486+ {"zero columns is unset", false, "a=T,f=100,c=0,r=3;" + payload, 6, 3, 6, 2},
487+ {"known cell pixels", true, "a=T,f=100,c=4;" + payload, 4, 2, 4, 1},
488+ {"put existing image", false, "a=t,i=7,f=100;" + payload + "\x1b\\\x1b_Ga=p,i=7,q=2,c=5", 5, 3, 5, 2},
482 }489 }
483 for _, tt := range tests {490 for _, tt := range tests {
484 t.Run(tt.name, func(t *testing.T) {491 t.Run(tt.name, func(t *testing.T) {
485 screen := NewScreen(100, 30)492 screen := NewScreen(100, 30)
486- screen.Write([]byte("\x1b_Ga=T,f=100,C=1," + tt.header + ";" + payload + "\x1b\\"))493+ if tt.geometry {
494+ screen.SetGeometry(Geometry{Cols: 100, Rows: 30, PixelWidth: 1000, PixelHeight: 600})
495+ }
496+ screen.Write([]byte("\x1b_G" + tt.commands + "\x1b\\"))
487 usage := screen.GraphicsSnapshot().Usage()497 usage := screen.GraphicsSnapshot().Usage()
488 require.Equal(t, uint64(1), usage.Assets)498 require.Equal(t, uint64(1), usage.Assets)
489 require.Equal(t, uint64(1), usage.Placements)499 require.Equal(t, uint64(1), usage.Placements)
490 cells := screen.GraphicsSnapshot().Placements()[0].Cells()500 cells := screen.GraphicsSnapshot().Placements()[0].Cells()
491 require.Equal(t, tt.cols, cells.Width)501 require.Equal(t, tt.cols, cells.Width)
492 require.Equal(t, tt.rows, cells.Height)502 require.Equal(t, tt.rows, cells.Height)
503+ require.Equal(t, tt.wantCol, screen.Col)
504+ require.Equal(t, tt.wantRow, screen.Row)
493 })505 })
494 }506 }
495 }507 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close