diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5c65a6..c2560cb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,19 @@ CHANGELOG - Only `change-nth` and `change-with-nth` rotate through `|`-separated values - Fixed the bound command being replaced with the output. e.g. With `transform-nth(echo '1|2')`, the next key press ran `2|1` as a shell command. - A `--with-nth` template in the output can now contain `|`. e.g. `{2}|{1}` was cut to `{2}`. +- An invalid value for `change-nth` and `transform-nth` no longer restores the default `--nth`. It is ignored, as with `change-with-nth` and `transform-with-nth`. +- `change-nth` and `change-with-nth` now apply the value after the current one in the `|`-separated list + - A value equal to the current one is skipped, e.g. the first value when it is the initial `--nth` + - Invalid values in the list are skipped + - The rotation follows the current value even when another binding changed it, or when the action is printed by `transform` (unless the list contains the same value more than once) + - You can list the values in their natural order, starting with the initial one + ```sh + # Previously, you would have to write: + ls -al | fzf --with-nth .. --bind 'space:change-with-nth:1|2|..' + + # Now you can write: + ls -al | fzf --with-nth .. --bind 'space:change-with-nth:..|1|2' + ``` - nushell - The completion integration now requires nushell 0.116.1 or above, where an interactive completer can take the terminal through the `@interactive` attribute (#4934) (@sim590) diff --git a/man/man1/fzf.1 b/man/man1/fzf.1 index a9996262..8a8b2e58 100644 --- a/man/man1/fzf.1 +++ b/man/man1/fzf.1 @@ -2060,8 +2060,8 @@ A key or an event can be bound to one or more of the following actions. \fBchange\-list\-label(...)\fR (change \fB\-\-list\-label\fR to the given string) \fBchange\-multi\fR (enable multi-select mode with no limit) \fBchange\-multi(...)\fR (enable multi-select mode with a limit or disable it with 0) - \fBchange\-nth(...)\fR (change \fB\-\-nth\fR option; rotate through the multiple options separated by '|') - \fBchange\-with\-nth(...)\fR (change \fB\-\-with\-nth\fR option; rotate through the multiple options separated by '|') + \fBchange\-nth(...)\fR (change \fB\-\-nth\fR option; rotate through the multiple options separated by '|', starting from the one after the current option) + \fBchange\-with\-nth(...)\fR (change \fB\-\-with\-nth\fR option; rotate through the multiple options separated by '|', starting from the one after the current option) \fBchange\-pointer(...)\fR (change \fB\-\-pointer\fR option) \fBchange\-preview(...)\fR (change \fB\-\-preview\fR option) \fBchange\-preview\-label(...)\fR (change \fB\-\-preview\-label\fR to the given string) diff --git a/src/terminal.go b/src/terminal.go index 39e8bb01..6056d731 100644 --- a/src/terminal.go +++ b/src/terminal.go @@ -6304,6 +6304,43 @@ func (t *Terminal) startTimers(ctx context.Context) { } } +// nextValue returns the first valid, non-current value after the current one +// in a '|'-separated list, and the list rotated to end with it. +// isValid can be nil. +func nextValue(expr string, isCurrent func(string) bool, isValid func(string) bool) (string, string) { + valid := func(value string) bool { + return isValid == nil || isValid(value) + } + // An invalid value is never current, even if it resolves to the current one + current := func(value string) bool { + return valid(value) && isCurrent(value) + } + values := strings.Split(expr, "|") + start := 0 + // Last value is the one applied by the previous call + if !current(values[len(values)-1]) { + for i, value := range values { + if current(value) { + start = i + 1 + break + } + } + } + for i := range values { + pick := (start + i) % len(values) + if value := values[pick]; valid(value) && !current(value) { + return value, strings.Join(slices.Concat(values[pick+1:], values[:pick+1]), "|") + } + } + // Nothing to pick + for _, value := range values { + if valid(value) { + return value, expr + } + } + return values[0], expr +} + // Loop is called to start Terminal I/O func (t *Terminal) Loop() error { // prof := profile.Start(profile.ProfilePath("/tmp/")) @@ -7304,25 +7341,38 @@ func (t *Terminal) Loop() error { req(reqList, reqInfo) case actChangeNth, actTransformNth, actBgTransformNth: capture(true, func(expr string) { + empty := len(expr) == 0 + // Empty value means all fields if it is the whole + // expression, otherwise the default --nth + resolve := func(value string) ([]Range, bool) { + if nth, err := splitNth(value); err == nil || empty { + return nth, true + } else if len(value) == 0 { + return t.nth, true + } + return nil, false + } + // Empty or a single '..' means all fields + allFields := func(nth []Range) bool { + return len(nth) == 0 || len(nth) == 1 && nth[0].IsFull() + } + isCurrent := func(nth []Range) bool { + return compareRanges(t.nthCurrent, nth) || allFields(t.nthCurrent) && allFields(nth) + } // Only change-nth cycles through '|'-separated values - tokens := []string{expr} if a.t == actChangeNth { - tokens = strings.Split(expr, "|") + expr, a.a = nextValue(expr, func(value string) bool { + nth, _ := resolve(value) + return isCurrent(nth) + }, func(value string) bool { + _, ok := resolve(value) + return ok + }) } - if nth, err := splitNth(tokens[0]); err == nil || len(expr) == 0 { - // Changed + if nth, ok := resolve(expr); ok && !isCurrent(nth) { newNth = &nth - } else { - // The default - newNth = &t.nth - } - // Cycle - if len(tokens) > 1 { - a.a = strings.Join(append(tokens[1:], tokens[0]), "|") - } - if !compareRanges(t.nthCurrent, *newNth) { changed = true - t.nthCurrent = *newNth + t.nthCurrent = nth t.forceRerenderList() } }) @@ -7331,18 +7381,22 @@ func (t *Terminal) Loop() error { break Action } capture(true, func(expr string) { - tokens := []string{expr} + resolve := func(value string) string { + // Empty value restores the default --with-nth + if len(value) == 0 { + return t.withNthDefault + } + return value + } if a.t == actChangeWithNth { - tokens = strings.Split(expr, "|") - } - withNthExpr := tokens[0] - if len(tokens) > 1 { - a.a = strings.Join(append(tokens[1:], tokens[0]), "|") - } - // Empty value restores the default --with-nth - if len(withNthExpr) == 0 { - withNthExpr = t.withNthDefault + expr, a.a = nextValue(expr, func(value string) bool { + return resolve(value) == t.withNthExpr + }, func(value string) bool { + _, err := nthTransformer(resolve(value)) + return err == nil + }) } + withNthExpr := resolve(expr) if withNthExpr != t.withNthExpr { if factory, err := nthTransformer(withNthExpr); err == nil { newWithNth = &withNthSpec{fn: factory(t.delimiter)} diff --git a/src/terminal_test.go b/src/terminal_test.go index 409baab5..063da21e 100644 --- a/src/terminal_test.go +++ b/src/terminal_test.go @@ -927,3 +927,56 @@ func TestSplitOnIND(t *testing.T) { } } } + +func TestNextValue(t *testing.T) { + tests := []struct { + expr string + current string + want string + rotated string + }{ + {"a", "x", "a", "a"}, + {"a", "a", "a", "a"}, + {"a|b|c", "x", "a", "b|c|a"}, + {"a|b|c", "a", "b", "c|a|b"}, + {"a|b|c", "b", "c", "a|b|c"}, + {"a|b|c", "c", "a", "b|c|a"}, + {"a|a|b", "a", "b", "a|a|b"}, + {"a|b|b", "b", "a", "b|b|a"}, + {"a|a", "a", "a", "a|a"}, + {"a|b|", "", "a", "b||a"}, + {"|a", "", "a", "|a"}, + {"a|bad|b", "a", "b", "a|bad|b"}, + {"bad|a", "x", "a", "bad|a"}, + {"a|bad", "a", "a", "a|bad"}, + // An invalid value is never current + {"a|bad|b", "bad", "a", "bad|b|a"}, + // Nothing to pick: the first valid value + {"bad|a", "a", "a", "bad|a"}, + {"bad", "x", "bad", "bad"}, + // The last value is checked first + {"b|a|c|a", "a", "b", "a|c|a|b"}, + } + isValid := func(v string) bool { return v != "bad" } + for _, tc := range tests { + got, rotated := nextValue(tc.expr, func(v string) bool { return v == tc.current }, isValid) + if got != tc.want || rotated != tc.rotated { + t.Errorf("nextValue(%q) with current %q: got (%q, %q), want (%q, %q)", + tc.expr, tc.current, got, rotated, tc.want, tc.rotated) + } + } +} + +func TestNextValueRepeated(t *testing.T) { + // Presses with a rotated list reach every position, even with a repeated value + expr := "1|2|1|3" + current := "1" + got := []string{} + for range 6 { + current, expr = nextValue(expr, func(v string) bool { return v == current }, nil) + got = append(got, current) + } + if want := "2 1 3 1 2 1"; strings.Join(got, " ") != want { + t.Errorf("got %q, want %q", strings.Join(got, " "), want) + } +} diff --git a/test/test_core.rb b/test/test_core.rb index daae0509..86af78fd 100644 --- a/test/test_core.rb +++ b/test/test_core.rb @@ -2342,6 +2342,94 @@ class TestCore < TestInteractive end end + def test_change_nth_next_value + tmux.send_keys %(echo 'a b c d' | #{FZF} --nth 1 --bind 'space:change-nth(1|2|3|4),x:change-nth(3),result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[1]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('[2]>') } + # Changed by another binding. The next value is the one after '3'. + tmux.send_keys :x + tmux.until { |lines| assert lines.any_include?('[3]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('[4]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('[1]>') } + end + + def test_change_nth_all_fields_is_current + # '--nth ..' is stored as an empty list, which must match '..' + tmux.send_keys %(echo 'a b c' | #{FZF} --nth .. --bind 'space:change-nth(..|1|2)' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('[1]>') } + end + + def test_change_nth_repeated_value + tmux.send_keys %(echo 'a b c' | #{FZF} --nth 1 --bind 'space:change-nth(1|2|1|3)' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[1]>') } + %w[2 1 3 1 2].each do |nth| + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?("[#{nth}]>") } + end + end + + def test_change_nth_skips_invalid_value + tmux.send_keys %(echo 'a b c' | #{FZF} --nth 2 --bind 'space:change-nth(1|x|3)' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[2]>') } + %w[1 3 1].each do |nth| + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?("[#{nth}]>") } + end + end + + def test_change_nth_only_invalid_values_left + # 'x' is invalid and '3' is current, so nothing changes + tmux.send_keys %(echo 'a b c' | #{FZF} --nth 1 --bind 'a:change-nth(3),space:change-nth(x|3)+transform-prompt:echo "S[$FZF_NTH]> "' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[1]>') } + tmux.send_keys :a + tmux.until { |lines| assert lines.any_include?('[3]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('S[3]>') } + end + + def test_change_nth_invalid_value_ignored + tmux.send_keys %(echo 'a b c' | #{FZF} --nth 1 --bind 'a:change-nth(3),space:change-nth(bad)+transform-prompt:echo "S[$FZF_NTH]> "' --bind 'b:transform-nth(echo bad)+transform-prompt:echo "T[$FZF_NTH]> "' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[1]>') } + tmux.send_keys :a + tmux.until { |lines| assert lines.any_include?('[3]>') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('S[3]>') } + tmux.send_keys :b + tmux.until { |lines| assert lines.any_include?('T[3]>') } + end + + def test_change_nth_from_transform + # A new action is created on each run, so the next value is found from the current state + tmux.send_keys %(echo 'a b c' | #{FZF} --nth 1 --bind 'space:transform:echo "change-nth(1|2|3)"' --bind 'result:transform-prompt:echo "[$FZF_NTH]> "'), :Enter + tmux.until { |lines| assert lines.any_include?('[1]>') } + %w[2 3 1].each do |nth| + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?("[#{nth}]>") } + end + end + + def test_change_with_nth_skips_default_value + tmux.send_keys %(echo 'a b c' | #{FZF} --with-nth 1 --bind 'space:change-with-nth(|2)'), :Enter + tmux.until { |lines| assert lines.any_include?('> a') } + # Empty value is the default (field 1), which is current + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('> b') } + end + + def test_change_with_nth_skips_invalid_value + tmux.send_keys %(echo 'a b c' | #{FZF} --with-nth 1 --bind 'space:change-with-nth(1|foo|2)'), :Enter + tmux.until { |lines| assert lines.any_include?('> a') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('> b') } + tmux.send_keys :Space + tmux.until { |lines| assert lines.any_include?('> a') } + end + def test_transform_with_nth_search input = [ 'alpha bravo charlie',