Fix nth attr merge order to respect precedence hierarchy (#4697)

* Fix nth attr merge order to respect precedence hierarchy

nth attrs were merged ON TOP of current-fg/selected-fg attrs, so
nth:regular would clear attrs like underline from current-fg. Fix the
merge chain to apply nth BEFORE the line-type overlay:

  fg < nth < selected-fg < current-fg < hl < selected-hl < current-hl

Store raw current-fg and selected-fg attrs in ColorTheme before they
get merged with fg/ListFg, then pass them as nthOverlay through
printHighlighted to colorOffsets where the correct merge chain is
computed: fgAttr.Merge(nthAttr).Merge(nthOverlay).

Fix #4687

* Make current-fg inherit from list-fg instead of fg

current-fg was inheriting from fg, not list-fg, so setting list-fg
had no effect on the current line. e.g.

  fzf --color fg:dim,list-fg:underline,nth:regular -d / --nth -1

The non-nth part of the current line was dim (from fg) instead of
underline (from list-fg). Resolve ListFg/ListBg earlier in InitTheme
so Current can inherit from them.

* Make selected-fg inherit from list-fg via merge instead of override

selected-fg used o() which replaces the attr from list-fg entirely.
e.g. with fg:dim,selected-fg:italic, the dim was lost on selected
lines because o() replaced dim with italic instead of merging them.
Use ColorAttr.Merge() so attrs are combined additively, consistent
with how current-fg inherits from list-fg.

* Apply selected-fg attrs on current line when item is selected

When an item is both current and selected, the current-line rendering
ignored selected-fg attrs entirely. e.g.

  echo foo | fzf --color fg:dim,nth:regular,current-fg:underline,selected-fg:italic --bind result:select --multi

The italic from selected-fg was lost. Merge NthSelectedAttr into the
overlay and rebuild colBase attr with the correct precedence chain:
fg < selected-fg < current-fg.
This commit is contained in:
Junegunn Choi
2026-03-02 12:00:21 +09:00
committed by GitHub
parent 97ac7794cf
commit b4e585779a
4 changed files with 82 additions and 22 deletions
+9 -5
View File
@@ -128,7 +128,7 @@ func minRank() Result {
return Result{item: &minItem, points: [4]uint16{math.MaxUint16, 0, 0, 0}} return Result{item: &minItem, points: [4]uint16{math.MaxUint16, 0, 0, 0}}
} }
func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, theme *tui.ColorTheme, colBase tui.ColorPair, colMatch tui.ColorPair, attrNth tui.Attr, hidden bool) []colorOffset { func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, theme *tui.ColorTheme, colBase tui.ColorPair, colMatch tui.ColorPair, attrNth tui.Attr, nthOverlay tui.Attr, hidden bool) []colorOffset {
itemColors := result.item.Colors() itemColors := result.item.Colors()
// No ANSI codes // No ANSI codes
@@ -213,6 +213,10 @@ func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, t
} }
return tui.NewColorPair(fg, bg, ansi.color.attr).WithUl(ansi.color.ul).MergeAttr(base) return tui.NewColorPair(fg, bg, ansi.color.attr).WithUl(ansi.color.ul).MergeAttr(base)
} }
fgAttr := tui.ColNormal.Attr()
nthAttrFinal := fgAttr.Merge(attrNth).Merge(nthOverlay)
nthBase := colBase.WithNewAttr(nthAttrFinal)
var colors []colorOffset var colors []colorOffset
add := func(idx int) { add := func(idx int) {
if curr.fbg >= 0 { if curr.fbg >= 0 {
@@ -226,7 +230,7 @@ func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, t
if curr.match { if curr.match {
var color tui.ColorPair var color tui.ColorPair
if curr.nth { if curr.nth {
color = colBase.WithAttr(attrNth).Merge(colMatch) color = nthBase.Merge(colMatch)
} else { } else {
color = colBase.Merge(colMatch) color = colBase.Merge(colMatch)
} }
@@ -246,7 +250,7 @@ func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, t
if color.Fg().IsDefault() && origColor.HasBg() { if color.Fg().IsDefault() && origColor.HasBg() {
color = origColor color = origColor
if curr.nth { if curr.nth {
color = color.WithAttr(attrNth &^ tui.AttrRegular) color = color.WithAttr((attrNth &^ tui.AttrRegular).Merge(nthOverlay))
} }
} else { } else {
color = origColor.MergeNonDefault(color) color = origColor.MergeNonDefault(color)
@@ -258,7 +262,7 @@ func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, t
ansi := itemColors[curr.index] ansi := itemColors[curr.index]
base := colBase base := colBase
if curr.nth { if curr.nth {
base = base.WithAttr(attrNth) base = nthBase
} }
if hidden { if hidden {
base = base.WithFg(theme.Nomatch) base = base.WithFg(theme.Nomatch)
@@ -270,7 +274,7 @@ func (result *Result) colorOffsets(matchOffsets []Offset, nthOffsets []Offset, t
match: false, match: false,
url: ansi.color.url}) url: ansi.color.url})
} else { } else {
color := colBase.WithAttr(attrNth) color := nthBase
if hidden { if hidden {
color = color.WithFg(theme.Nomatch) color = color.WithFg(theme.Nomatch)
} }
+33 -2
View File
@@ -132,7 +132,7 @@ func TestColorOffset(t *testing.T) {
colBase := tui.NewColorPair(89, 189, tui.AttrUndefined) colBase := tui.NewColorPair(89, 189, tui.AttrUndefined)
colMatch := tui.NewColorPair(99, 199, tui.AttrUndefined) colMatch := tui.NewColorPair(99, 199, tui.AttrUndefined)
colors := item.colorOffsets(offsets, nil, tui.Dark256, colBase, colMatch, tui.AttrUndefined, false) colors := item.colorOffsets(offsets, nil, tui.Dark256, colBase, colMatch, tui.AttrUndefined, 0, false)
assert := func(idx int, b int32, e int32, c tui.ColorPair) { assert := func(idx int, b int32, e int32, c tui.ColorPair) {
o := colors[idx] o := colors[idx]
if o.offset[0] != b || o.offset[1] != e || o.color != c { if o.offset[0] != b || o.offset[1] != e || o.color != c {
@@ -159,7 +159,7 @@ func TestColorOffset(t *testing.T) {
nthOffsets := []Offset{{37, 39}, {42, 45}} nthOffsets := []Offset{{37, 39}, {42, 45}}
for _, attr := range []tui.Attr{tui.AttrRegular, tui.StrikeThrough} { for _, attr := range []tui.Attr{tui.AttrRegular, tui.StrikeThrough} {
colors = item.colorOffsets(offsets, nthOffsets, tui.Dark256, colRegular, colUnderline, attr, false) colors = item.colorOffsets(offsets, nthOffsets, tui.Dark256, colRegular, colUnderline, attr, 0, false)
// [{[0 5] {1 5 0}} {[5 15] {1 5 8}} {[15 20] {1 5 0}} // [{[0 5] {1 5 0}} {[5 15] {1 5 8}} {[15 20] {1 5 0}}
// {[22 25] {2 6 1}} {[25 27] {2 6 9}} {[27 30] {-1 -1 8}} // {[22 25] {2 6 1}} {[25 27] {2 6 9}} {[27 30] {-1 -1 8}}
@@ -182,6 +182,37 @@ func TestColorOffset(t *testing.T) {
assert(10, 37, 39, tui.NewColorPair(4, 8, expected)) assert(10, 37, 39, tui.NewColorPair(4, 8, expected))
assert(11, 39, 40, tui.NewColorPair(4, 8, tui.Bold)) assert(11, 39, 40, tui.NewColorPair(4, 8, tui.Bold))
} }
// Test nthOverlay: simulates nth:regular with current-fg:underline
// The overlay (underline) should survive even though nth:regular clears attrs.
// Precedence: fg < nth < current-fg
colors = item.colorOffsets(offsets, nthOffsets, tui.Dark256, colRegular, colUnderline, tui.AttrRegular, tui.Underline, false)
// nth regions should have Underline (from overlay), not cleared by AttrRegular
// Non-nth regions keep colBase attrs (AttrUndefined)
assert(0, 0, 5, tui.NewColorPair(1, 5, tui.AttrUndefined))
assert(1, 5, 15, tui.NewColorPair(1, 5, tui.Underline))
assert(2, 15, 20, tui.NewColorPair(1, 5, tui.AttrUndefined))
assert(3, 22, 25, tui.NewColorPair(2, 6, tui.Bold))
assert(4, 25, 27, tui.NewColorPair(2, 6, tui.Bold|tui.Underline))
assert(5, 27, 30, colUnderline)
assert(6, 30, 32, tui.NewColorPair(3, 7, tui.Underline))
assert(7, 32, 33, colUnderline)
assert(8, 33, 35, tui.NewColorPair(4, 8, tui.Bold|tui.Underline))
assert(9, 35, 37, tui.NewColorPair(4, 8, tui.Bold))
// nth region within ANSI bold: AttrRegular clears, overlay adds Underline back
assert(10, 37, 39, tui.NewColorPair(4, 8, tui.Bold|tui.Underline))
assert(11, 39, 40, tui.NewColorPair(4, 8, tui.Bold))
// Test nthOverlay with additive attrs: nth:strikethrough with selected-fg:bold
colors = item.colorOffsets(offsets, nthOffsets, tui.Dark256, colRegular, colUnderline, tui.StrikeThrough, tui.Bold, false)
// Non-nth entries unchanged from overlay=0 case
assert(0, 0, 5, tui.NewColorPair(1, 5, tui.AttrUndefined))
assert(5, 27, 30, colUnderline) // match only, no nth
assert(7, 32, 33, colUnderline) // match only, no nth
// nth region within ANSI bold: StrikeThrough|Bold merged with ANSI Bold
assert(10, 37, 39, tui.NewColorPair(4, 8, tui.Bold|tui.StrikeThrough))
} }
func TestRadixSortResults(t *testing.T) { func TestRadixSortResults(t *testing.T) {
+22 -9
View File
@@ -1554,7 +1554,7 @@ func (t *Terminal) ansiLabelPrinter(str string, color *tui.ColorPair, fill bool)
printFn := func(window tui.Window, limit int) { printFn := func(window tui.Window, limit int) {
if offsets == nil { if offsets == nil {
// tui.Col* are not initialized until renderer.Init() // tui.Col* are not initialized until renderer.Init()
offsets = result.colorOffsets(nil, nil, t.theme, *color, *color, t.nthAttr, false) offsets = result.colorOffsets(nil, nil, t.theme, *color, *color, t.nthAttr, 0, false)
} }
for limit > 0 { for limit > 0 {
if length > limit { if length > limit {
@@ -1617,7 +1617,7 @@ func (t *Terminal) parsePrompt(prompt string) (func(), int) {
return 1 return 1
} }
t.printHighlighted( t.printHighlighted(
Result{item: item}, tui.ColPrompt, tui.ColPrompt, false, false, false, line, line, true, preTask, nil) Result{item: item}, tui.ColPrompt, tui.ColPrompt, false, false, false, line, line, true, preTask, nil, 0)
}) })
t.wrap = wrap t.wrap = wrap
} }
@@ -3185,7 +3185,7 @@ func (t *Terminal) printFooter() {
func(markerClass) int { func(markerClass) int {
t.footerWindow.Print(indent) t.footerWindow.Print(indent)
return indentSize return indentSize
}, nil) }, nil, 0)
} }
}) })
t.wrap = wrap t.wrap = wrap
@@ -3269,7 +3269,7 @@ func (t *Terminal) printHeaderImpl(window tui.Window, borderShape tui.BorderShap
func(markerClass) int { func(markerClass) int {
t.window.Print(indent) t.window.Print(indent)
return indentSize return indentSize
}, nil) }, nil, 0)
} }
t.wrap = wrap t.wrap = wrap
} }
@@ -3507,7 +3507,14 @@ func (t *Terminal) printItem(result Result, line int, maxLine int, index int, cu
} }
return indentSize return indentSize
} }
finalLineNum = t.printHighlighted(result, tui.ColCurrent, tui.ColCurrentMatch, true, true, !matched, line, maxLine, forceRedraw, preTask, postTask) colCurrent := tui.ColCurrent
nthOverlay := t.theme.NthCurrentAttr
if selected {
nthOverlay = t.theme.NthSelectedAttr.Merge(t.theme.NthCurrentAttr)
baseAttr := tui.ColNormal.Attr().Merge(t.theme.NthSelectedAttr).Merge(t.theme.NthCurrentAttr)
colCurrent = colCurrent.WithNewAttr(baseAttr)
}
finalLineNum = t.printHighlighted(result, colCurrent, tui.ColCurrentMatch, true, true, !matched, line, maxLine, forceRedraw, preTask, postTask, nthOverlay)
} else { } else {
preTask := func(marker markerClass) int { preTask := func(marker markerClass) int {
w := t.window.Width() - t.pointerLen w := t.window.Width() - t.pointerLen
@@ -3541,7 +3548,11 @@ func (t *Terminal) printItem(result Result, line int, maxLine int, index int, cu
base = base.WithBg(altBg) base = base.WithBg(altBg)
match = match.WithBg(altBg) match = match.WithBg(altBg)
} }
finalLineNum = t.printHighlighted(result, base, match, false, true, !matched, line, maxLine, forceRedraw, preTask, postTask) var nthOverlay tui.Attr
if selected {
nthOverlay = t.theme.NthSelectedAttr
}
finalLineNum = t.printHighlighted(result, base, match, false, true, !matched, line, maxLine, forceRedraw, preTask, postTask, nthOverlay)
} }
for i := 0; i < t.gap && finalLineNum < maxLine; i++ { for i := 0; i < t.gap && finalLineNum < maxLine; i++ {
finalLineNum++ finalLineNum++
@@ -3642,7 +3653,7 @@ func (t *Terminal) overflow(runes []rune, max int) bool {
return t.displayWidthWithLimit(runes, 0, max) > max return t.displayWidthWithLimit(runes, 0, max) > max
} }
func (t *Terminal) printHighlighted(result Result, colBase tui.ColorPair, colMatch tui.ColorPair, current bool, match bool, hidden bool, lineNum int, maxLineNum int, forceRedraw bool, preTask func(markerClass) int, postTask func(int, int, bool, bool, tui.ColorPair)) int { func (t *Terminal) printHighlighted(result Result, colBase tui.ColorPair, colMatch tui.ColorPair, current bool, match bool, hidden bool, lineNum int, maxLineNum int, forceRedraw bool, preTask func(markerClass) int, postTask func(int, int, bool, bool, tui.ColorPair), nthOverlay tui.Attr) int {
var displayWidth int var displayWidth int
item := result.item item := result.item
matchOffsets := []Offset{} matchOffsets := []Offset{}
@@ -3683,7 +3694,9 @@ func (t *Terminal) printHighlighted(result Result, colBase tui.ColorPair, colMat
// But if 'nth' is set to 'regular', it's a sign that you're applying // But if 'nth' is set to 'regular', it's a sign that you're applying
// a different style to the rest of the string. e.g. 'nth:regular,fg:dim' // a different style to the rest of the string. e.g. 'nth:regular,fg:dim'
// In this case, we still need to apply it to clear the style. // In this case, we still need to apply it to clear the style.
colBase = colBase.WithAttr(t.nthAttr) fgAttr := tui.ColNormal.Attr()
nthAttrFinal := fgAttr.Merge(t.nthAttr).Merge(nthOverlay)
colBase = colBase.WithNewAttr(nthAttrFinal)
} }
if !wholeCovered && t.nthAttr > 0 { if !wholeCovered && t.nthAttr > 0 {
var tokens []Token var tokens []Token
@@ -3702,7 +3715,7 @@ func (t *Terminal) printHighlighted(result Result, colBase tui.ColorPair, colMat
sort.Sort(ByOrder(nthOffsets)) sort.Sort(ByOrder(nthOffsets))
} }
} }
allOffsets := result.colorOffsets(charOffsets, nthOffsets, t.theme, colBase, colMatch, t.nthAttr, hidden) allOffsets := result.colorOffsets(charOffsets, nthOffsets, t.theme, colBase, colMatch, t.nthAttr, nthOverlay, hidden)
// Determine split offset for horizontal scrolling with freeze // Determine split offset for horizontal scrolling with freeze
splitOffset1 := -1 splitOffset1 := -1
+18 -6
View File
@@ -447,6 +447,12 @@ func (p ColorPair) WithAttr(attr Attr) ColorPair {
return dup return dup
} }
func (p ColorPair) WithNewAttr(attr Attr) ColorPair {
dup := p
dup.attr = attr
return dup
}
func (p ColorPair) WithFg(fg ColorAttr) ColorPair { func (p ColorPair) WithFg(fg ColorAttr) ColorPair {
dup := p dup := p
fgPair := ColorPair{fg.Color, colUndefined, colUndefined, fg.Attr} fgPair := ColorPair{fg.Color, colUndefined, colUndefined, fg.Attr}
@@ -520,6 +526,8 @@ type ColorTheme struct {
ListLabel ColorAttr ListLabel ColorAttr
ListBorder ColorAttr ListBorder ColorAttr
GapLine ColorAttr GapLine ColorAttr
NthCurrentAttr Attr // raw current-fg attr (before fg merge) for nth overlay
NthSelectedAttr Attr // raw selected-fg attr (before ListFg inherit) for nth overlay
} }
type Event struct { type Event struct {
@@ -1199,13 +1207,19 @@ func InitTheme(theme *ColorTheme, baseTheme *ColorTheme, boldify bool, forceBlac
match.Attr = Underline match.Attr = Underline
} }
theme.Match = o(baseTheme.Match, match) theme.Match = o(baseTheme.Match, match)
// Inherit from 'fg', so that we don't have to write 'current-fg:dim' // These colors are not defined in the base themes.
// Resolve ListFg/ListBg early so Current and Selected can inherit from them.
theme.ListFg = o(theme.Fg, theme.ListFg)
theme.ListBg = o(theme.Bg, theme.ListBg)
// Inherit from 'list-fg', so that we don't have to write 'current-fg:dim'
// e.g. fzf --delimiter / --nth -1 --color fg:dim,nth:regular // e.g. fzf --delimiter / --nth -1 --color fg:dim,nth:regular
current := theme.Current current := theme.Current
if !baseTheme.Colored && current.IsUndefined() { if !baseTheme.Colored && current.IsUndefined() {
current.Attr |= Reverse current.Attr |= Reverse
} }
theme.Current = theme.Fg.Merge(o(baseTheme.Current, current)) resolvedCurrent := o(baseTheme.Current, current)
theme.NthCurrentAttr = resolvedCurrent.Attr
theme.Current = theme.ListFg.Merge(resolvedCurrent)
currentMatch := theme.CurrentMatch currentMatch := theme.CurrentMatch
if !baseTheme.Colored && currentMatch.IsUndefined() { if !baseTheme.Colored && currentMatch.IsUndefined() {
currentMatch.Attr |= Reverse | Underline currentMatch.Attr |= Reverse | Underline
@@ -1230,10 +1244,8 @@ func InitTheme(theme *ColorTheme, baseTheme *ColorTheme, boldify bool, forceBlac
scrollbarDefined := theme.Scrollbar != undefined scrollbarDefined := theme.Scrollbar != undefined
previewBorderDefined := theme.PreviewBorder != undefined previewBorderDefined := theme.PreviewBorder != undefined
// These colors are not defined in the base themes theme.NthSelectedAttr = theme.SelectedFg.Attr
theme.ListFg = o(theme.Fg, theme.ListFg) theme.SelectedFg = theme.ListFg.Merge(theme.SelectedFg)
theme.ListBg = o(theme.Bg, theme.ListBg)
theme.SelectedFg = o(theme.ListFg, theme.SelectedFg)
theme.SelectedBg = o(theme.ListBg, theme.SelectedBg) theme.SelectedBg = o(theme.ListBg, theme.SelectedBg)
theme.SelectedMatch = o(theme.Match, theme.SelectedMatch) theme.SelectedMatch = o(theme.Match, theme.SelectedMatch)