Consider loading persistent_history_file into cmd2 history in addition to readline history

[rowdyrotifer]

@tleonhardt previously mentioned this following in #265 (when readline history was introduced)

We would need to decide if this is done for readline history, cmd2 history command history, or both. Perhaps there would be two separate options for making each of these persistent?

In the implementation of this feature (#270) only readline history support was implemented.

I think it would in fact make sense for this file to also affect the cmd2 history command, as this is the behavior that other shells exhibit with their history files. It seems more consistent and expected to have both histories linked up.

[tleonhardt]

@marklalor For the default implementation of this feature, we intentionally only supported an optionally persistent readline history. Our thought process was that we wanted the convenience of the user being able to access what they have typed in the past using <Ctrl>+r as well as the up arrow. However, we wanted the cmd2 history command to cleanly reflect only commands that have been executed within the lifetime of the current application execution.

The cmd2 history command has evolved into something logically distinct from the readline history. For example, the cmd2 history command can do things like expand aliases, macros, and shortcuts; whereas the readline history always shows you exactly what you typed as you typed it. The cmd2 history command can also be used to help generate text file scripts and transcript files for transcript regression testing.

That being said, if you feel strongly that it would be beneficial to also optionally have a persistent cmd2 history, then we could consider adding this capability after thinking through any potential unintended consequences.

@kmvanbrunt @kotfu What is your opinion on this?

[kotfu]

I'm open to exploration of this topic. The cmd2 in-memory history storage includes both exactly what was typed at the prompt, as well as the expanded command (aliases, macros, shortcuts). Say we modify cmd2 to persist these more robust memory structures instead of relying on readline persistence. We might be able to use our native persisted data to feed the readline history, which would unify these currently distinct approaches to history.

I'll be happy to experiment a bit on this one and see what we can come up with.

[tleonhardt]

@kotfu As long as we can maintain the current behavior as one option then I would be happy with having other options. I would be OK with two separate (optional) config files - the current one for readline history and a 2nd new one for cmd2 history. Though, if we could have a single file that would feel slightly better from an architectural perspective. Though if we have a single file, I'd like to make sure that we can restore either the readline history or the cmd2 history (or both) from it and that it would accurately restore both.

[kotfu]

I did a little digging, and here's what I think could work, as well as some caveats:

• readline history files (what we currently use) are just plain text with each command on a separate line
• on startup instead of telling the readline library to load this file, we would load it ourselves to populate the cmd2 history data structures, and then tell the readline library to load it
• we rely on the readline library to write the history out at exit, and it caps the history at the indicated number of commands, we continue to rely on this functionality
• when we load the history into the cmd2 History array, we add a new attribute which tracks the size of the history that was loaded.
• When the user runs the history command, we only display history elements that are "newer" than the history that was loaded. This preserves the current functionality of the history command.
• We add a new "-a" switch to the "history" command, which allows the history command to "see" commands from previous sessions that were loaded on initialization.

This preserves the format of our current history file, doesn't require a new history file, and preserves the functionality of the current history command which still only shows commands entered in the current cmd2 session.

We can then decide if we want to make an additional developer accessible setting which sets the default history behavior to include "-a" or not.

The caveat to this entire approach (and really to any serialization of python data structures to text files), is that we really need to parse every command in history when we load it on initialization. Marshaling python objects from plain text won't create instances of our Statement class, it will create strings. We need our Statement objects so that all of the current history functionality will work, including showing expanded macros, shortcuts, and aliases. To get Statement objects we will have to parse every command stored in history on initialization.

This could be a potential performance problem, because on launch we might have to parse 10,000 previous commands, or maybe even more if readline is configured to save all commands forever. We could maybe put this onto a background thread on load, but that's complicated because we will have to make History, HistoryItem, Statement, etc. thread safe.

I think it's worth a try, and maybe we impose some limit that we allow at max 1,000 commands in history, and we just add good developer documentation that explains the performance trade offs of saving large numbers of commands in history.

[kmvanbrunt]

Here are some other things to consider

• readline's history saves everything a user types whereas cmd2's history only saves valid commands and even filters out the history command. It makes sense for readline to save all input because it allows a user to up arrow and fix their input, but we don't want this stuff in cmd2's history.
• cmd2's history depends on state. If it reads from the readline file and tries to resolve aliases that only existed in the last session, then the generated Statement objects won't be correct.

One option would be to generate the persistent history files ourselves by serializing the History structures on exit. Then when the app opens, we restore the history Statement objects from the file and use this information to manually populate readline's history. This has the downside of our history file no longer being human readable and readline not having everything the user typed. If we stop filtering out the history command, then that mostly solves the missing input issue. I don't think a user would care so much if mistyped input was missing from a past session.

[kmvanbrunt]

To handle @kotfu's concern of reading 10,000 commands from a history file, we could just enforce a reasonable limit as to the amount we save. This of course changes the API as persistent_history_length would be removed. It's possible no one ever used that setting since it already had a default value.

[kmvanbrunt]

@kotfu I just realized I repeated much of what you already said. I need to read more carefully :)

[kotfu]

All history tracking depends on state, and running previous commands that did not generate an error is no guarantee they won't generate an error now. Consider:

(Cmd) alias create ls !ls -al
Alias 'ls' created
(Cmd) ls
.....
(Cmd) alias delete ls
Alias 'ls' deleted
(Cmd) history
    1  alias create ls !ls -al
    2  ls
    3  alias delete ls

If I now hit the up arrow twice, I'll get an error where I had a successfully executed command before. Same thing if I hit the up arrow once. Similarly, if I do history -r 2, I get an error instead of the list of files when I ran it originally.

The readline history is blissfully unaware of any of this, including whether the input parsed correctly, was a valid command, etc. It just stores everything you type.

I think the current behavior of the internal cmd2 history (certain commands are excluded, and only valid commands at the time they were originally executed are stored in the history) is a) desirable and b) should not be modified. If we accept these assertions (I think we should, but I'm open to discussion and debate), we have the following choices:

1. use pickle to serialize out the cmd2 History array to a 2nd history file. Add code at initialization to unpickle History from the 2nd history file.
2. re-use the readline history file to load cmd2 history on initialization. We will have to incur the parsing overhead of parsing the entire history on initialization. We will also have to silently drop items that can not be parsed, because lots of cmd2 history related code depends (rightly so) on having parsed objects in History.
3. make no change and leave the current behavior as is.

Every one of these three options shares a similar set of caveats:

• There may be items in the cmd2 history that are not valid commands if executed now. This includes commands which may have expanded previously (because of a defined shortcut or alias) but which do not expand now
• There may be items in the cmd2 history which parse/expand to a different command now than they did when they were run and added to history.
• The cmd2 history and the readline history may be different. (They will be more similar in options 1 and 2 than they are in option 3, but there will not be any guarantee they will be the same)

So if we eliminate option 3 (not suggesting we do so, but let's pretend for a minute), we are essentially weighing multiple history files vs the overhead of reparsing history. It won't take me long to implement the "reparsing history from the readline history file" code, at least enough that we can see how fast/slow it is with various sizes of history files. Once we have that data (I'll put all the code/data to run those performance tests in the branch so anyone can run it on their hardware and see the time), then we can decide.

[kmvanbrunt]

Is it possible to alter option 1 by making its output the only history file? In other words, can the history file of pickled objects be used to populate the readline history with Statement.raw from each unpickled History item we keep?

[kotfu]

Yes. I think it will require a temp file, because readline will populate history from disk, not from a file descriptor. If it would use a file descriptor we could use io.StringIO. So what we would have to:

• unpickle the History array from disk
• iterate History and write the contents of all of the HistoryItem.Statement.raw attributes to a temp file on disk
• tell the readline library to read in history from that temp file

On exit, we trim History to the maximum desired size and pickle it to disk.

[kmvanbrunt]

What if we don't call readline.read_history_file() and instead call readline.add_history() for each unpickled Statement.raw?

[tleonhardt]

I don't mind having separate history files. Readline history behavior when saving to and restoring from a file works great, is well tested, and has excellent performance.

Whatever changes we make, I want to make sure we continue to provide a great customer experience for end users. To do that, I believer it is imperative that we do not introduce any noticeable delay when starting up a cmd2 application - a 100 ms delay is probably OK, but a 500ms would be very annoying to many users.

[kotfu]

So now the potential options are:

1. Use pickle to serialize out the cmd2 History array to a 2nd history file. Add code at initialization to unpickle History from the 2nd history file.
2. Replace the contents of the current readline history file with a pickled History array. On initialization, unpickle History, and populate the in-memory readline history using readline.add_history()
3. Re-use the readline history file to load cmd2 history on initialization. We will have to incur the parsing overhead of parsing the entire history on initialization. We will also have to silently drop items that can not be parsed, because lots of cmd2 history related code depends (rightly so) on having parsed objects in History.
4. make no change and leave the current behavior as is.