Skip to content

Fix nth attr merge order to respect precedence hierarchy#4697

Merged
junegunn merged 4 commits into
masterfrom
devel
Mar 2, 2026
Merged

Fix nth attr merge order to respect precedence hierarchy#4697
junegunn merged 4 commits into
masterfrom
devel

Conversation

@junegunn
Copy link
Copy Markdown
Owner

@junegunn junegunn commented Mar 1, 2026

Test case

printf "foo bar baz\nfoo bar baz\nfoo bar baz" |
  fzf --nth 3 --color fg:dim,nth:regular,current-fg:underline,hl+:strikethrough,selected-fg:italic \
      --multi --query z --bind result:select+up+select

Note

The behavior of selected-fg has changed. Previously, selected-fg:italic would not inherit attributes from fg, so --color fg:dim,selected-fg:italic would only apply italic to selected lines without dim. Now selected-fg inherits from fg (via list-fg), consistent with how current-fg works. If you want to clear inherited attributes, prefix with regular: selected-fg:regular:italic.

junegunn added 2 commits March 1, 2026 19:07
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
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.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes nth attribute merging so that styling precedence is respected (e.g., ensuring current-fg / selected-fg attributes can override nth, even when nth:regular clears attributes).

Changes:

  • Add support for “nth overlay” attributes sourced from raw current-fg / selected-fg attrs and thread them through rendering.
  • Adjust theme initialization order so list-fg/list-bg are resolved earlier and current inherits from list-fg.
  • Extend and update tests to cover the precedence case (fg < nth < current-fg/selected-fg).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/tui/tui.go Adds WithNewAttr and stores raw current/selected attrs on the theme for nth overlay logic; resolves list colors earlier.
src/terminal.go Plumbs nth overlay attrs through printHighlighted and applies correct precedence when nth:regular is involved.
src/result.go Updates colorOffsets signature and applies nth overlay when producing per-span color pairs.
src/result_test.go Updates call sites and adds tests validating nth overlay precedence behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/result.go
Comment thread src/terminal.go
junegunn added 2 commits March 1, 2026 20:19
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.
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.
@junegunn junegunn merged commit b4e5857 into master Mar 2, 2026
8 checks passed
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Mar 3, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [junegunn/fzf](https://github.com/junegunn/fzf) | minor | `v0.68.0` → `v0.70.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>junegunn/fzf (junegunn/fzf)</summary>

### [`v0.70.0`](https://github.com/junegunn/fzf/releases/tag/v0.70.0): 0.70.0

[Compare Source](junegunn/fzf@v0.68.0...v0.70.0)

#### Demo: `change-with-nth`

<https://github.com/user-attachments/assets/40abe7e6-8c98-4a13-85b7-17e69372ef9e>

#### Summary

- Added `change-with-nth` action for dynamically changing the `--with-nth` option.
  - Requires `--with-nth` to be set initially.
  - Multiple options separated by `|` can be given to cycle through.
  ```sh
  echo -e "a b c\nd e f\ng h i" | fzf --with-nth .. \
    --bind 'space:change-with-nth(1|2|3|1,3|2,3|)'
  ```
- Added `change-header-lines` action for dynamically changing the `--header-lines` option
- Performance improvements (1.3x to 1.9x faster filtering depending on query)
  ```
  === query: 'l' ===
    [all]   baseline:   168.87ms  current:    95.21ms  (1.77x)  matches: 5069891 (94.78%)
    [1T]    baseline:  1652.22ms  current:   841.40ms  (1.96x)  matches: 5069891 (94.78%)

  === query: 'lin' ===
    [all]   baseline:   343.27ms  current:   252.59ms  (1.36x)  matches: 3516507 (65.74%)
    [1T]    baseline:  3199.89ms  current:  2230.64ms  (1.43x)  matches: 3516507 (65.74%)

  === query: 'linux' ===
    [all]   baseline:    85.47ms  current:    63.72ms  (1.34x)  matches: 307229 (5.74%)
    [1T]    baseline:   774.64ms  current:   589.32ms  (1.31x)  matches: 307229 (5.74%)

  === query: 'linuxlinux' ===
    [all]   baseline:    55.13ms  current:    35.67ms  (1.55x)  matches: 12230 (0.23%)
    [1T]    baseline:   461.99ms  current:   332.38ms  (1.39x)  matches: 12230 (0.23%)

  === query: 'linuxlinuxlinux' ===
    [all]   baseline:    51.77ms  current:    32.53ms  (1.59x)  matches: 865 (0.02%)
    [1T]    baseline:   409.99ms  current:   296.33ms  (1.38x)  matches: 865 (0.02%)
  ```
- Fixed `nth` attribute merge order to respect precedence hierarchy ([#&#8203;4697](junegunn/fzf#4697))
- bash: Replaced `printf` with builtin `printf` to bypass local indirections ([#&#8203;4684](junegunn/fzf#4684)) ([@&#8203;DarrenBishop](https://github.com/DarrenBishop))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My40Ni42IiwidXBkYXRlZEluVmVyIjoiNDMuNDYuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Go code test Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants