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

Wraparound when exceeding history (max_file_size) does not work as expected #76

Closed
zwieberl opened this issue Oct 26, 2017 · 3 comments
Closed
Labels

Comments

@zwieberl
Copy link

Maybe this is meant this way, but if the history-file has exactly max_file_size lines and another is pushed, the file is cleared completely and the newly pushed is its only entry.
The in memory-history works as expected (drops the oldest, but keeps the rest).

How to reproduce (quick and dirty):

$ touch test
$ for x in $(seq 1000); do echo $x >> test; done
$ cat test

This gives 1000 entries.

Compile and run:

extern crate liner;
use liner::Context;

fn main() {
    let mut con = Context::new();
    con.history.set_file_name(Some("test".to_string()));
    con.history.load_history();
    let command = "blabla";
    con.history.push(command.into());
    con.history.commit_history();
}

and then

$ cat test

again. Result only "blabla" will be in the history-file.

@zwieberl
Copy link
Author

zwieberl commented Oct 27, 2017

diff --git a/src/history.rs b/src/history.rs
index 8545959..c5a23be 100644
--- a/src/history.rs
+++ b/src/history.rs
@@ -242,7 +242,7 @@ fn write_to_disk(max_file_size: usize, new_item: &Buffer, file_name: &str) -> io
                             total_read += 1;
                             if byte == b'\n' {
                                 stored += 1;
-                                if stored == max_file_size {
+                                if stored == (stored + cmds_read - max_file_size) {
                                     seek_point = total_read;
                                     break
                                 }

This seems to solve the problem.

@MovingtoMars
Copy link
Owner

That patch does fix the functionality when using liner, but for some reason it adds a lot of nulls to the start of the history file.

I'll have a better look tomorrow.

@MovingtoMars
Copy link
Owner

This should be fixed as of f5e00be

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

No branches or pull requests

2 participants