diff --git a/internal/inline/render.go b/internal/inline/render.go index 15e2b3a..114259a 100644 --- a/internal/inline/render.go +++ b/internal/inline/render.go @@ -6,8 +6,6 @@ import ( "fmt" "os" "path/filepath" - "slices" - "sort" "strconv" "strings" "unicode/utf8" @@ -74,7 +72,7 @@ func (s *inlineScratch) ensureCapacity(n int) { type hunkMoveMetadata struct { srcLine1Badges map[int]string // Line index -> " ➔ L..." dstLine1Badges map[int]string // Line index -> " ⤹ L..." - hunkHeaders map[int]string // Hunk index -> " (moved {from/to} L...[, modified])" + hunkHeaders map[int]string // Hunk index -> " (moved {from/to} L...)" } // Render formats the diff envelope as an inline diff with AST highlights and move markers. @@ -453,7 +451,7 @@ func Render(srcFile, dstFile string, srcBytes, dstBytes []byte, env *serialize.E lineRendered := renderLineWithSpans(l.text, leftSpansByLine[l.srcLineIdx], true, "left", opts.Color, scratch, peerDepictsEdit(leftToRight, rightSpansByLine, l.srcLineIdx)) if opts.Color { if badge != "" { - badge = color.Italic + color.OverlayFg + badge + color.Reset + badge = color.MoveFg + badge + color.Reset } if opts.LineNumbers { out.WriteString(gutter + lineRendered + badge + "\n") @@ -504,7 +502,7 @@ func Render(srcFile, dstFile string, srcBytes, dstBytes []byte, env *serialize.E lineRendered := renderLineWithSpans(l.text, rightSpansByLine[l.dstLineIdx], false, "right", opts.Color, scratch, peerDepictsEdit(rightToLeft, leftSpansByLine, l.dstLineIdx)) if opts.Color { if badge != "" { - badge = color.Italic + color.OverlayFg + badge + color.Reset + badge = color.MoveFg + badge + color.Reset } if opts.LineNumbers { out.WriteString(gutter + lineRendered + badge + "\n") @@ -662,19 +660,6 @@ func buildHunkMoveMetadata(actions []serialize.Action, hunks []interval, pairs [ return meta } - // Collect destination mutating byte offsets to check whether moved nodes were edited - var dstMutOffsets []uint32 - for _, a := range actions { - if a.Action == "insert" || a.Action == "update" || a.Action == "move_update" { - if a.DestStartByte != nil { - dstMutOffsets = append(dstMutOffsets, *a.DestStartByte) - } else if a.DestNode != nil { - dstMutOffsets = append(dstMutOffsets, a.DestNode.StartByte) - } - } - } - slices.Sort(dstMutOffsets) - // Index lines into hunks so we can tell if moves cross hunk boundaries srcLineToHunk := make(map[int]int) dstLineToHunk := make(map[int]int) @@ -699,15 +684,12 @@ func buildHunkMoveMetadata(actions []serialize.Action, hunks []interval, pairs [ sEnd, _ := serialize.ByteToLineCol(srcOffsets, a.Node.EndByte) var dStart, dEnd int - var dStartByte, dEndByte uint32 if a.DestStartByte != nil && a.DestEndByte != nil { dStart, _ = serialize.ByteToLineCol(dstOffsets, *a.DestStartByte) dEnd, _ = serialize.ByteToLineCol(dstOffsets, *a.DestEndByte) - dStartByte, dEndByte = *a.DestStartByte, *a.DestEndByte } else if a.DestNode != nil { dStart, _ = serialize.ByteToLineCol(dstOffsets, a.DestNode.StartByte) dEnd, _ = serialize.ByteToLineCol(dstOffsets, a.DestNode.EndByte) - dStartByte, dEndByte = a.DestNode.StartByte, a.DestNode.EndByte } else { continue } @@ -715,16 +697,7 @@ func buildHunkMoveMetadata(actions []serialize.Action, hunks []interval, pairs [ hSrc, inSrcHunk := srcLineToHunk[sStart] hDst, inDstHunk := dstLineToHunk[dStart] - // Omit annotations for moves staying within the same hunk - if inSrcHunk && inDstHunk && hSrc == hDst { - continue - } - - // Check if any mutations fall inside the destination range - idx := sort.Search(len(dstMutOffsets), func(i int) bool { - return dstMutOffsets[i] >= dStartByte - }) - isModified := idx < len(dstMutOffsets) && dstMutOffsets[idx] < dEndByte + // Teal alone doesn't say where it went, so badge every structural move. isDecl := r != nil && r.IsDeclaration(a.Node.Type) isBlock := r != nil && r.IsBlock(a.Node.Type) @@ -733,24 +706,24 @@ func buildHunkMoveMetadata(actions []serialize.Action, hunks []interval, pairs [ if !isDecl && !isBlock && !isMultiLine && !isStatement { continue } + // Same-hunk one-liners can see their destination on screen, so skip the badge. + if !isDecl && !isBlock && !isMultiLine && inSrcHunk && inDstHunk && hSrc == hDst { + continue + } if isDecl { // Top-level declaration moves are summarized directly in the hunk header sig := extractDeclarationSignature(a.Node, srcLines, sStart, sEnd) if sig == "declaration" { sig = extractDeclarationSignature(a.Node, dstLines, dStart, dEnd) } - modStr := "" - if isModified { - modStr = ", modified" - } if inSrcHunk && sig != "" { if _, exists := meta.hunkHeaders[hSrc]; !exists { - meta.hunkHeaders[hSrc] = fmt.Sprintf(" %s (moved to L%d%s)", sig, dStart+1, modStr) + meta.hunkHeaders[hSrc] = fmt.Sprintf(" %s (moved to L%d)", sig, dStart+1) } } if inDstHunk && sig != "" { if _, exists := meta.hunkHeaders[hDst]; !exists { - meta.hunkHeaders[hDst] = fmt.Sprintf(" %s (moved from L%d%s)", sig, sStart+1, modStr) + meta.hunkHeaders[hDst] = fmt.Sprintf(" %s (moved from L%d)", sig, sStart+1) } } } else { diff --git a/internal/inline/render_test.go b/internal/inline/render_test.go index 3867ca9..3e44210 100644 --- a/internal/inline/render_test.go +++ b/internal/inline/render_test.go @@ -202,7 +202,8 @@ func TestRender_Tier1_IntraHunkMoveCleanliness(t *testing.T) { got := Render("a.go", "b.go", src, dst, dr.Envelope, RenderOptions{Color: false, ContextLines: 3, LineNumbers: true}) - // Under Tier 1, intra-hunk moves suppress right-margin ghost text completely. + // One-line shifts in the same hunk don't get badges. You can already see + // where they went. if strings.Contains(got, "←") || strings.Contains(got, "➔") || strings.Contains(got, "⤹") || strings.Contains(got, "moved to line") { t.Errorf("expected zero right-margin trailing ghost annotations for intra-hunk move, got:\n%s", got) } diff --git a/internal/inline/slicer.go b/internal/inline/slicer.go index 42f4160..92cde7e 100644 --- a/internal/inline/slicer.go +++ b/internal/inline/slicer.go @@ -306,14 +306,14 @@ func sliceInlineLine( if len(chunks) > 0 { last := len(chunks) - 1 if colorMode { - chunks[last] = append(chunks[last], color.Italic+color.OverlayFg+badgeText+color.Reset...) + chunks[last] = append(chunks[last], color.MoveFg+badgeText+color.Reset...) } else { chunks[last] = append(chunks[last], badgeText...) } } else { var b []byte if colorMode { - b = append(b, color.Italic+color.OverlayFg+badgeText+color.Reset...) + b = append(b, color.MoveFg+badgeText+color.Reset...) } else { b = append(b, badgeText...) } diff --git a/internal/serialize/spans.go b/internal/serialize/spans.go index 4311e5d..a2c5c2f 100644 --- a/internal/serialize/spans.go +++ b/internal/serialize/spans.go @@ -59,6 +59,10 @@ func BuildHighlightSpans(fileBytes []byte, actions []Action, side string, extraS lineIndex := BuildLineIndex(fileBytes) spansByLine := make(map[int][]internalSpan) + // Only the outermost move of a relocated block gets spans. The nested + // ones would paint the same teal twice. + skipNestedMove := nestedMoveActions(actions, side) + for i := range actions { a := &actions[i] switch a.Action { @@ -84,6 +88,9 @@ func BuildHighlightSpans(fileBytes []byte, actions []Action, side string, extraS case "move": actType := "move" + if skipNestedMove[i] { + continue + } if side == "left" && a.Node != nil { parent := a.OldParent if parent == nil { @@ -270,6 +277,62 @@ func isOnlyNonCharacters(b []byte) bool { return true } +// nestedMoveActions finds moves buried inside a bigger move on the same side. +// Skipping them keeps a relocated block to one span instead of one per token. +func nestedMoveActions(actions []Action, side string) map[int]bool { + type byteRange struct { + idx int + s uint32 + e uint32 + } + var ranges []byteRange + for i := range actions { + a := &actions[i] + if a.Action != "move" { + continue + } + var s, e uint32 + var ok bool + if side == "left" && a.Node != nil { + s, e, ok = a.Node.StartByte, a.Node.EndByte, true + } else if side == "right" { + if a.DestStartByte != nil && a.DestEndByte != nil { + s, e, ok = *a.DestStartByte, *a.DestEndByte, true + } else if a.DestNode != nil { + s, e, ok = a.DestNode.StartByte, a.DestNode.EndByte, true + } + } + if !ok || e <= s { + continue + } + ranges = append(ranges, byteRange{idx: i, s: s, e: e}) + } + // Plain O(n^2) scan, there are never enough moves per file for this to matter. + slices.SortFunc(ranges, func(a, b byteRange) int { + return cmp.Or( + cmp.Compare(b.e-b.s, a.e-a.s), + cmp.Compare(a.s, b.s), + ) + }) + skip := make(map[int]bool) + var kept []byteRange + for _, r := range ranges { + contained := false + for _, k := range kept { + if k.s <= r.s && k.e >= r.e { + contained = true + break + } + } + if contained { + skip[r.idx] = true + continue + } + kept = append(kept, r) + } + return skip +} + func nodeRefsEqual(n1, n2 *NodeRef) bool { if n1 == n2 { return true diff --git a/internal/serialize/spans_test.go b/internal/serialize/spans_test.go index e2c8361..4ac3c85 100644 --- a/internal/serialize/spans_test.go +++ b/internal/serialize/spans_test.go @@ -429,3 +429,21 @@ func TestBuildHighlightSpansWithDelimiterSpan(t *testing.T) { t.Errorf("expected delimiter span on line 2 cols 0..1 action='delete', got %+v", leftSpans[1]) } } + +func TestNestedMoveActionsKeepsOutermost(t *testing.T) { + src := []byte("0123456789abcdef\n") + actions := []Action{ + {Action: "move", Node: &NodeRef{Tree: "before", Type: "block", StartByte: 0, EndByte: 16}}, + {Action: "move", Node: &NodeRef{Tree: "before", Type: "identifier", StartByte: 2, EndByte: 5}}, + {Action: "move", Node: &NodeRef{Tree: "before", Type: "identifier", StartByte: 10, EndByte: 12}}, + } + spans := BuildHighlightSpans(src, actions, "left") + for _, s := range spans { + if s.Action == "move" && s.ActionRef != nil && s.ActionRef.Node.StartByte != 0 { + t.Fatalf("expected only outermost move spans, got nested %+v", s) + } + } + if len(spans) == 0 { + t.Fatal("expected outermost move span, got none") + } +} diff --git a/internal/sidebyside/render.go b/internal/sidebyside/render.go index 67fb575..2c6e6eb 100644 --- a/internal/sidebyside/render.go +++ b/internal/sidebyside/render.go @@ -1,12 +1,9 @@ package sidebyside import ( - "cmp" "fmt" "io" "os" - "slices" - "sort" "strconv" "strings" @@ -738,27 +735,6 @@ func buildMoveBadges( r = rules.Get(lang.Name) } - type mutatingSpan struct { - startByte uint32 - endByte uint32 - } - var dstMutations []mutatingSpan - for _, a := range actions { - if a.Action == "insert" || a.Action == "delete" || a.Action == "update" || a.Action == "move_update" { - if a.Node != nil && a.Node.Tree == "after" { - dstMutations = append(dstMutations, mutatingSpan{startByte: a.Node.StartByte, endByte: a.Node.EndByte}) - } else if a.DestStartByte != nil && a.DestEndByte != nil { - dstMutations = append(dstMutations, mutatingSpan{startByte: *a.DestStartByte, endByte: *a.DestEndByte}) - } - } - } - slices.SortFunc(dstMutations, func(a, b mutatingSpan) int { - return cmp.Or( - cmp.Compare(a.startByte, b.startByte), - cmp.Compare(a.endByte, b.endByte), - ) - }) - type crossHunkMove struct { sStartLine int sEndLine int @@ -767,7 +743,6 @@ func buildMoveBadges( sHunk int dHunk int isDecl bool - nMut int } var crossMoves []crossHunkMove @@ -802,28 +777,7 @@ func buildMoveBadges( dHunk = -1 } - // Skip badges if the move stays within the same hunk and is close by. - lineDist := sStartLine - dStartLine - if lineDist < 0 { - lineDist = -lineDist - } - if sHunk != -1 && dHunk != -1 && sHunk == dHunk && lineDist < 10 { - continue - } - - // Count edits inside the moved node so we know if it was modified. - nMut := 0 - if dEndByte > dStartByte && len(dstMutations) > 0 { - idx := sort.Search(len(dstMutations), func(i int) bool { - return dstMutations[i].startByte >= dStartByte - }) - for idx < len(dstMutations) && dstMutations[idx].startByte < dEndByte { - if dstMutations[idx].endByte <= dEndByte { - nMut++ - } - idx++ - } - } + // Teal alone doesn't say where it went, so badge every structural move. isDecl := r != nil && r.IsDeclaration(a.Node.Type) isBlock := r != nil && r.IsBlock(a.Node.Type) @@ -832,6 +786,10 @@ func buildMoveBadges( if !isDecl && !isBlock && !isMultiLine && !isStatement { continue } + // Same-hunk one-liners can see their destination on screen, so skip the badge. + if !isDecl && !isBlock && !isMultiLine && sHunk != -1 && dHunk != -1 && sHunk == dHunk { + continue + } m := crossHunkMove{ sStartLine: sStartLine, @@ -841,7 +799,6 @@ func buildMoveBadges( sHunk: sHunk, dHunk: dHunk, isDecl: isDecl, - nMut: nMut, } crossMoves = append(crossMoves, m) } @@ -856,11 +813,7 @@ func buildMoveBadges( if m.dStartLine >= 0 && m.dStartLine < len(dstLines) { if _, exists := dstLineBadges[m.dStartLine]; !exists { - modStr := "" - if m.nMut > 0 { - modStr = ", modified" - } - dstLineBadges[m.dStartLine] = fmt.Sprintf(" ⤹ L%d%s", m.sStartLine+1, modStr) + dstLineBadges[m.dStartLine] = fmt.Sprintf(" ⤹ L%d", m.sStartLine+1) } } } diff --git a/internal/sidebyside/slicer.go b/internal/sidebyside/slicer.go index f4c4601..094ccb9 100644 --- a/internal/sidebyside/slicer.go +++ b/internal/sidebyside/slicer.go @@ -63,7 +63,7 @@ func (s *RenderScratch) SliceLineToChunks( // Put the move badge on the first chunk only. if isFirstChunk && badgeText != "" { if colorMode { - curChunk = append(curChunk, color.Italic+color.OverlayFg+badgeText+color.Reset...) + curChunk = append(curChunk, color.MoveFg+badgeText+color.Reset...) } else { curChunk = append(curChunk, badgeText...) } diff --git a/tests/testdata/c_redis_dict_resize/expected_ui.json.gz b/tests/testdata/c_redis_dict_resize/expected_ui.json.gz index 2a33e3e..46fb5f2 100644 Binary files a/tests/testdata/c_redis_dict_resize/expected_ui.json.gz and b/tests/testdata/c_redis_dict_resize/expected_ui.json.gz differ diff --git a/tests/testdata/c_redis_quadratic_search/expected_ui.json.gz b/tests/testdata/c_redis_quadratic_search/expected_ui.json.gz index 331401c..17faf32 100644 Binary files a/tests/testdata/c_redis_quadratic_search/expected_ui.json.gz and b/tests/testdata/c_redis_quadratic_search/expected_ui.json.gz differ diff --git a/tests/testdata/go_gin_using_keyed/expected_ui.json.gz b/tests/testdata/go_gin_using_keyed/expected_ui.json.gz index 8b06a9b..730cc4f 100644 Binary files a/tests/testdata/go_gin_using_keyed/expected_ui.json.gz and b/tests/testdata/go_gin_using_keyed/expected_ui.json.gz differ diff --git a/tests/testdata/lua_kong_add_missing_select/expected_ui.json.gz b/tests/testdata/lua_kong_add_missing_select/expected_ui.json.gz index eecf5b1..306a110 100644 Binary files a/tests/testdata/lua_kong_add_missing_select/expected_ui.json.gz and b/tests/testdata/lua_kong_add_missing_select/expected_ui.json.gz differ diff --git a/tests/testdata/lua_neovim_fs_api_refactor/expected_ui.json.gz b/tests/testdata/lua_neovim_fs_api_refactor/expected_ui.json.gz index 4928ec0..33b9af2 100644 Binary files a/tests/testdata/lua_neovim_fs_api_refactor/expected_ui.json.gz and b/tests/testdata/lua_neovim_fs_api_refactor/expected_ui.json.gz differ diff --git a/tests/testdata/lua_neovim_gen_help_html_change/expected_ui.json.gz b/tests/testdata/lua_neovim_gen_help_html_change/expected_ui.json.gz index 80c1ecb..1c5e8af 100644 Binary files a/tests/testdata/lua_neovim_gen_help_html_change/expected_ui.json.gz and b/tests/testdata/lua_neovim_gen_help_html_change/expected_ui.json.gz differ diff --git a/tests/testdata/py_requests_refactor_prefer/expected_ui.json.gz b/tests/testdata/py_requests_refactor_prefer/expected_ui.json.gz index ab71897..83584cb 100644 Binary files a/tests/testdata/py_requests_refactor_prefer/expected_ui.json.gz and b/tests/testdata/py_requests_refactor_prefer/expected_ui.json.gz differ diff --git a/tests/testdata/yaml_microservices_skaffold_pipeline_refactor/expected_ui.json.gz b/tests/testdata/yaml_microservices_skaffold_pipeline_refactor/expected_ui.json.gz index 411a07c..1d0b0e5 100644 Binary files a/tests/testdata/yaml_microservices_skaffold_pipeline_refactor/expected_ui.json.gz and b/tests/testdata/yaml_microservices_skaffold_pipeline_refactor/expected_ui.json.gz differ