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

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

Open
krader1961 opened this issue Mar 4, 2024 · 2 comments
Open

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

krader1961 opened this issue Mar 4, 2024 · 2 comments

Comments

@krader1961
Copy link
Contributor

krader1961 commented Mar 4, 2024

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
Copy link
Member

xiaq commented Mar 5, 2024

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

@krader1961
Copy link
Contributor Author

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.

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

No branches or pull requests

2 participants