search

elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.53k stars 297 forks source link

`store:next-cmd` and `store:prev-cmd` are not documented #1770

Open krader1961 opened 4 months ago

krader1961 commented 4 months ago

While working on my flat-file replacement for the Elvish daemon and BoltDB I noticed the store:next-cmd and store:prev-cmd commands are not documented.

P.S., I only noticed this because I was exploring when the type Store interface method NextCmd (defined in pkg/store/storedefs/storedefs.go) was invoked. AFAICT it is only called by the store:next-cmd implementation. Which begs the question whether either command should even exist given that the user can simply get a list of all commands using store:cmds and iterate through the list.

xiaq commented 4 months ago

The methods are used in history walking mode to avoid loading the entire history into memory.

krader1961 commented 4 months ago

The methods are used in history walking mode to avoid loading the entire history into memory.

That is true for the implementation underlying store:prev-cmd but not store:next-cmd, AFAICT. The CLI store code calls the PrevCmd method when it has not already cached the previous command relative to the current command history cursor. AFAICT it never calls the NextCmd method.

Only the store:next-cmd implementation calls the NextCmd method, AFAICT. Also, both methods are essentially implementation details of the CLI command history traversal mechanism to skip identical commands. Which might be why the commands in the store module that invokes them didn't get documented (obviously it could also be a simple oversight).

Regardless of the reason these commands are not documented I don't think they should be documented and the commands should be removed. Anyone wanting to examine the command history can use the store:cmds and store:cmd commands. It is not obvious what the value is in exposing the internal PrevCmd and NextCmd methods.