Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make edit:command-history easier, and faster, to use with tools like fzf #1053

Closed
krader1961 opened this issue Jun 21, 2020 · 8 comments
Closed

Comments

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Jun 21, 2020

See #1051 for what prompted me to open this issue.

At least one common use-case of the edit:command-history command would be easier and faster (i.e., more efficient) with some enhancements to that command:

  1. An option to output the commands in newest-to-oldest order (keeping the default oldest-to-newest).

  2. An option to eliminate duplicate commands regardless of the output order. "Duplicate" here means comparing just the "cmd" member of the default map output.

  3. An option to request just the "cmd" string be emitted rather than a map of all command attributes.

  4. Maybe: An option to limit the output to the N newest commands; without regard to the order of the output. Note that the take command only works as a substitute for this option if the option to output the commands in newest-to-oldest order is used. On the other hand this option may not make much sense when coupled with oldest-to-newest order (the default). Regardless of whether or not it is treated as the first or last N commands. Thus the "maybe" prefix since it may be better to omit this option.

Using the first three options makes it easy to do command selection using the popular fzf utility. It should also improve the time to produce a fzf menu by at least an order of magnitude.

The duplicate command elimination option might warrant more discussion. Specifically, whether elvish should always do duplicate elimination when updating the command data store. In all my years using ksh and zsh as my day-to-day interactive shells I can't ever recall a situation where retaining each individual use of a command in the history was useful. As opposed to being an annoyance when I listed the N most recent commands I ran or was stepping backward through my command history; e.g., via [up-arrow]. Note that fish always does duplicate elimination. I can't recall ever hearing anyone complain about the deduplication.

@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented Jun 27, 2020

See also #568 which asked for a way to access a deduplicated command history more than two years ago.

@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented May 5, 2021

A recent question on IRC reminded me that I, like @zzamboni, had added some functions to my rc.elv several months ago to use the fzf program rather than the builtin histlist mode but hadn't bound those functions to Ctrl-R. So I did so and was happy with fzf but not how long it took before the fzf query prompt was usable.

On my primary MacPro server edit:command-history >/dev/null completes in 55 ms. The following function takes 760 ms (~13.8 times longer):

edit:command-history | each [hist-entry]{
    print $hist-entry[cmd]"\000"
} | perl -n0e 'print unless $h{$_}++' >/dev/null

A pure Elvish solution takes 2849 ms (~3.7 and ~51.8 times longer than the perl and unmodified history solutions respectively):

seen = [&]
edit:command-history |
    each [hist-entry]{
        cmd = $hist-entry[cmd]
        if (has-key $seen $cmd) {
        continue
        }
        seen[$cmd] = $true
        print $cmd"\000"
    } >/dev/null

Implementing a couple of the proposed enhancements should dramatically improve the experience of using fzf for selecting history entries. Especially if combined with a to-lines enhancement to control the EOL character.

@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented May 15, 2021

I have a proof-of-concept change that adds a &dedup and &newest-first option; thus addressing items one and two in my original problem statement. It dramatically reduces the cost of feeding such output to an external command. On my system the following takes 151.819 ms while the solution using perl requires 728.985 ms -- a factor of 4.8 times faster. So much faster that pressing Ctrl-R no longer has any visible artifacts before I can interact with fzf.

> edit:command-history &dedup &newest-first |
    each [hist-entry]{ print $hist-entry[cmd]"\000" } > /dev/null

It's certain to be even faster if we can replace the each... print... in the above pipeline with to-lines &eol="\000" (or an equivalent capability) and edit:command-history gains a &cmd-only option to output just the command text rather than a command map.

P.S., Note that outputting all commands takes 511.519 ms on my system. Which is 3.4 times slower than outputting just the non-duplicates:

> time { edit:command-history | each [hist-entry]{ print $hist-entry[cmd]"\000" } > /dev/null }

Which, again, leads to the question of whether Elvish should even be recording each non-unique instance of a command since doing so isn't particularly useful and bloats the command history for little benefit.

krader1961 added a commit to krader1961/elvish that referenced this issue May 15, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related elves#1053
Fixes elves#568
krader1961 added a commit to krader1961/elvish that referenced this issue May 15, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related elves#1053
Fixes elves#568
krader1961 added a commit to krader1961/elvish that referenced this issue May 15, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related elves#1053
Fixes elves#568
krader1961 added a commit to krader1961/elvish that referenced this issue May 15, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related elves#1053
Fixes elves#568
krader1961 added a commit to krader1961/elvish that referenced this issue May 15, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related elves#1053
Fixes elves#568
@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented May 16, 2021

On my system the following takes 151.819 ms while the solution using perl requires 728.985 ms -- a factor of 4.8 times faster.

Also, it's important to note the solution using perl is wrong as it does not correctly preserve the order of the commands. Whereas the &dedup option correctly preserves the order by retaining the most recent, rather than the oldest, instance of a command.

@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented May 16, 2021

Implementing to-lines &null, along with the other enhancements discussed above, reduces the cost of constructing the input for the fzf program from 760 ms (for the perl based, incorrect, solution) to 34.7 ms. On my system pressing Ctrl-R now has no visible lag before I can search my command history.

krader1961 added a commit to krader1961/elvish that referenced this issue May 16, 2021
This change, combined with other changes to `edit:command-history`,
makes feeding its output into `fzf` extremely fast. Which makes it a
practical alternative to using the builtin Ctrl-R binding for searching
command history. It's also just generally useful to have efficient ways
to process "lines" that are null terminated rather than newline (or
cr-nl on Windows).

Resolves elves#1070
Related elves#1053
krader1961 added a commit to krader1961/elvish that referenced this issue May 16, 2021
This is useful when piping the output into a program like `fzf` and more
efficient than dealing with the map. This takes 180.32 ms on my system
(best of five runs):

```
time { edit:command-history &dedup &newest-first | each [hist-entry]{ print $hist-entry[cmd]"\000" } >/dev/null }
```

This option reduces the time to 156.12 ms:

```
time { edit:command-history &dedup &newest-first &cmd-only | each [cmd]{ print $cmd"\000" } >/dev/null }
```

It's not a huge difference but is 13% faster, and should be considerably
faster when combined with a hypothetical `to-lines &null` option I intend
to implement.

Related elves#1053
@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented May 16, 2021

Using both PR #1307 and PR #1308 allows a very efficient Ctrl-R binding that uses the fzf program for selection. So efficient it is indistinguishable from the builtin histlist navigation mode with respect to initialization time:

# Filter the command history through the fzf program. This is normally bound
# to Ctrl-R but can be invoked explicitly by running `history`.
fn history []{
  # If the user presses Escape to cancel the fzf operation it will exit with a
  # non-zero status. We want to ignore that exception; hence the status
  # capture. We could use an explicit `try ...except...` but this is simpler.
  _ = ?(edit:current-command = (
    edit:command-history &dedup &newest-first &cmd-only | to-lines &null |
    fzf --no-sort --read0 --layout=reverse --info=hidden --exact ^
      --query=$edit:current-command)
  )
}

edit:insert:binding[Ctrl-R] = []{ history >/dev/tty 2>&1 }

krader1961 added a commit to krader1961/elvish that referenced this issue May 17, 2021
This is useful when piping the output into a program like `fzf` and more
efficient than dealing with the map. This takes 180.32 ms on my system
(best of five runs):

```
time { edit:command-history &dedup &newest-first | each [hist-entry]{ print $hist-entry[cmd]"\000" } >/dev/null }
```

This option reduces the time to 156.12 ms:

```
time { edit:command-history &dedup &newest-first &cmd-only | each [cmd]{ print $cmd"\000" } >/dev/null }
```

It's not a huge difference but is 13% faster, and should be considerably
faster when combined with a hypothetical `to-lines &null` option I intend
to implement.

Related elves#1053
krader1961 added a commit to krader1961/elvish that referenced this issue May 17, 2021
This change, combined with other changes to `edit:command-history`,
makes feeding its output into `fzf` extremely fast. Which makes it a
practical alternative to using the builtin Ctrl-R binding for searching
command history. It's also just generally useful to have efficient ways
to process "lines" that are null terminated rather than newline (or
cr-nl on Windows).

Resolves elves#1070
Related elves#1053
xiaq added a commit that referenced this issue May 30, 2021
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.

Related #1053
Fixes #568
xiaq added a commit that referenced this issue May 30, 2021
This is useful when piping the output into a program like `fzf` and more
efficient than dealing with the map. This takes 180.32 ms on my system
(best of five runs):

```
time { edit:command-history &dedup &newest-first | each [hist-entry]{ print $hist-entry[cmd]"\000" } >/dev/null }
```

This option reduces the time to 156.12 ms:

```
time { edit:command-history &dedup &newest-first &cmd-only | each [cmd]{ print $cmd"\000" } >/dev/null }
```

It's not a huge difference but is 13% faster, and should be considerably
faster when combined with a hypothetical `to-lines &null` option I intend
to implement.

Related #1053
krader1961 added a commit to krader1961/elvish that referenced this issue Jun 8, 2021
This change makes feeding output to commands which handle NUL terminated
"lines" (e.g., `fzf -read0` or `xargs -0`) extremely fast compared to
using an explicit Elvish loop that does `print $val"\x00"`. Similarly for
handling input from commands that produce NUL terminated "lines" (e.g.,
`find . -print0`) compared to an Elvish loop using `read-upto "\x00"`.

Resolves elves#1070
Related elves#1053
xiaq added a commit that referenced this issue Jun 10, 2021
This change makes feeding output to commands which handle NUL terminated
"lines" (e.g., `fzf -read0` or `xargs -0`) extremely fast compared to
using an explicit Elvish loop that does `print $val"\x00"`. Similarly for
handling input from commands that produce NUL terminated "lines" (e.g.,
`find . -print0`) compared to an Elvish loop using `read-upto "\x00"`.

Resolves #1070
Related #1053
@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented Jun 11, 2021

For anyone who stumbles upon this issue in the future.... When writing the necessary code and responding to review feedback some of the details of this proposal were changed. This is the code I now have in my ~/.elvish/rc.elv init script:

# Filter the command history through the fzf program. This is normally bound
# to Ctrl-R.
fn history []{
  var new-cmd = (
    edit:command-history &dedup &newest-first &cmd-only |
    to-terminated "\x00" |
    try {
      fzf --no-sort --read0 --layout=reverse --info=hidden --exact ^
        --query=$edit:current-command
    } except {
      # If the user presses [Escape] to cancel the fzf operation it will exit
      # with a non-zero status. Ignore that we ran this function in that case.
      return
    }
  )
  edit:current-command = $new-cmd
}

edit:insert:binding[Ctrl-R] = []{ history >/dev/tty 2>&1 }

@krader1961
Copy link
Contributor Author

@krader1961 krader1961 commented Jul 4, 2021

Everything needed to make binding Ctrl-R to fzf is in place for the upcoming 0.16.0 release so this is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant