Merge branch 'history_refactor' into 'master'

Rip apart History and refactor the API

See merge request redox-os/liner!7

examples/comments.rs

@@ -20,17 +20,18 @@ fn highlight_dodo(s: &str) -> String {
 fn main() {
     let mut con = Context::new();
 
-    let history_file = args().nth(1);
-    match history_file {
-        Some(ref file_name) => println!("History file: {}", file_name),
-        None => println!("No history file"),
-    }
+    let history_file = match args().nth(1) {
+        Some(file_name) => {
+            println!("History file: {}", file_name);
+            file_name
+        }
+        None => {
+            eprintln!("No history file provided. Ending example early.");
+            return;
+        }
+    };
 
-    con.history.set_file_name(history_file);
-    // We set the file name, then check if we set it, and if we set it properly, we load it in
-    if con.history.file_name().is_some() {
-        con.history.load_history().unwrap();
-    }
+    con.history.set_file_name_and_load_history(history_file).unwrap();
 
     loop {
         // Reads the line, the first arg is the prompt, the second arg is a function called on every bit of text leaving liner, and the third is called on every key press
@@ -94,7 +95,7 @@ fn main() {
                     _ => {}
                 }
 
-                // If we typed nothing, don't continue down to pushing to history 
+                // If we typed nothing, don't continue down to pushing to history
                 if res.is_empty() {
                     break;
                 }
@@ -115,7 +116,6 @@ fn main() {
                     _ => {
                         // Ensure that all writes to the history file
                         // are written before exiting due to error.
-                        con.history.commit_history();
                         panic!("error: {:?}", e)
                     },
                 }
@@ -126,5 +126,5 @@ fn main() {
     }
 
     // Ensure that all writes to the history file are written before exiting.
-    con.history.commit_history();
+    con.history.commit_to_file();
 }

examples/nocomments.rs

@@ -19,16 +19,18 @@ fn highlight_dodo(s: &str) -> String {
 fn main() {
     let mut con = Context::new();
 
-    let history_file = args().nth(1);
-    match history_file {
-        Some(ref file_name) => println!("History file: {}", file_name),
-        None => println!("No history file"),
-    }
+    let history_file = match args().nth(1) {
+        Some(file_name) => {
+            println!("History file: {}", file_name);
+            file_name
+        }
+        None => {
+            eprintln!("No history file provided. Ending example early.");
+            return;
+        }
+    };
 
-    con.history.set_file_name(history_file);
-    if con.history.file_name().is_some() {
-        con.history.load_history().unwrap();
-    }
+    con.history.set_file_name_and_load_history(history_file).unwrap();
 
     loop {
         let res = con.read_line("[prompt]$ ",
@@ -91,15 +93,12 @@ fn main() {
                     _ => {
                         // Ensure that all writes to the history file
                         // are written before exiting.
-                        con.history.commit_history();
                         panic!("error: {:?}", e)
                     },
                 }
             }
         }
-
     }
-
     // Ensure that all writes to the history file are written before exiting.
-    con.history.commit_history();
-}
+    con.history.commit_to_file();
+}
\ No newline at end of file

src/context.rs

@@ -73,7 +73,7 @@ impl Context {
         &mut self,
         prompt: P,
         f: Option<ColorClosure>,
-        mut handler: &mut EventHandler<RawTerminal<Stdout>>,
+        handler: &mut EventHandler<RawTerminal<Stdout>>,
     ) -> io::Result<String> {
         self.read_line_with_init_buffer(prompt, handler, f, Buffer::new())
     }
@@ -92,7 +92,7 @@ impl Context {
     pub fn read_line_with_init_buffer<P: Into<String>, B: Into<Buffer>>(
         &mut self,
         prompt: P,
-        mut handler: &mut EventHandler<RawTerminal<Stdout>>,
+        handler: &mut EventHandler<RawTerminal<Stdout>>,
         f: Option<ColorClosure>,
         buffer: B,
     ) -> io::Result<String> {
@@ -105,13 +105,13 @@ impl Context {
             }
         };
 
-        self.revert_all_history();
+        //self.revert_all_history();
         res
     }
 
     fn handle_keys<'a, T, W: Write, M: KeyMap<'a, W, T>>(
         mut keymap: M,
-        mut handler: &mut EventHandler<W>,
+        handler: &mut EventHandler<W>,
     ) -> io::Result<String>
     where
         String: From<M>,

src/editor.rs

@@ -112,8 +112,8 @@ macro_rules! cur_buf {
 
 impl<'a, W: Write> Editor<'a, W> {
     pub fn new<P: Into<String>>(
-        out: W, 
-        prompt: P, 
+        out: W,
+        prompt: P,
         f: Option<ColorClosure>,
         context: &'a mut Context
     ) -> io::Result<Self> {
@@ -601,7 +601,7 @@ impl<'a, W: Write> Editor<'a, W> {
         if self.show_autosuggestions {
             {
                 let autosuggestion = self.current_autosuggestion().cloned();
-                let mut buf = self.current_buffer_mut();
+                let buf = self.current_buffer_mut();
                 if let Some(x) = autosuggestion {
                     buf.insert_from_buffer(&x);
                 }

src/history.rs

 use super::*;
 
-use std::collections::{vec_deque, VecDeque};
-use std::io::{BufRead, BufReader, Error, ErrorKind};
-use std::fs::{File, OpenOptions};
-use std::io::{self, Read, Seek, SeekFrom, Write};
-use std::iter::IntoIterator;
-use std::ops::Index;
-use std::ops::IndexMut;
-use std::sync::Arc;
-use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
-use std::sync::mpsc::{channel, Sender};
-use std::thread::{sleep, spawn, JoinHandle};
-use std::time::Duration;
-
-use bytecount::count;
+use std::{
+    collections::{vec_deque, VecDeque},
+    io::{BufRead, BufReader, BufWriter},
+    fs::File,
+    io::{self, Seek, SeekFrom, Write},
+    iter::IntoIterator,
+    ops::Index,
+    ops::IndexMut,
+    path::Path,
+    //time::Duration,
+};
 
 const DEFAULT_MAX_SIZE: usize = 1000;
 
@@ -26,68 +23,60 @@ pub struct History {
     file_name: Option<String>,
     /// Maximal number of buffers stored in the memory
     /// TODO: just make this public?
-    max_size: usize,
+    max_buffers_size: usize,
     /// Maximal number of lines stored in the file
     // TODO: just make this public?
-    max_file_size: Arc<AtomicUsize>,
-    /// Handle to the background thread managing writes to the history file
-    bg_handle: Option<JoinHandle<()>>,
-    /// Signals the background thread to stop when dropping the struct
-    bg_stop: Arc<AtomicBool>,
-    /// Sends commands to write to the history file
-    sender: Sender<(Buffer, String)>,
-
+    max_file_size: usize,
     // TODO set from environment variable?
     pub append_duplicate_entries: bool,
 }
 
 impl History {
-    /// It's important to execute this function before exiting your program, as it will
-    /// ensure that all history data has been written to the disk.
-    pub fn commit_history(&mut self) {
-        // Signal the background thread to stop
-        self.bg_stop.store(true, Ordering::Relaxed);
-        // Wait for the background thread to stop
-        if let Some(handle) = self.bg_handle.take() {
-            let _ = handle.join();
-        }
-    }
-
     /// Create new History structure.
     pub fn new() -> History {
-        let max_file_size = Arc::new(AtomicUsize::new(DEFAULT_MAX_SIZE));
-        let bg_stop = Arc::new(AtomicBool::new(false));
-        let (sender, receiver) = channel();
-
-        let stop_signal = bg_stop.clone();
-        let max_size = max_file_size.clone();
         History {
             buffers: VecDeque::with_capacity(DEFAULT_MAX_SIZE),
             file_name: None,
-            sender: sender,
-            bg_handle: Some(spawn(move || {
-                while !stop_signal.load(Ordering::Relaxed) {
-                    if let Ok((command, filepath)) = receiver.try_recv() {
-                        let max_file_size = max_size.load(Ordering::Relaxed);
-                        let _ = write_to_disk(max_file_size, &command, &filepath);
-                    }
-                    sleep(Duration::from_millis(100));
-                }
-
-                // Deplete the receiver of commands to write, before exiting the thread.
-                while let Ok((command, filepath)) = receiver.try_recv() {
-                    let max_file_size = max_size.load(Ordering::Relaxed);
-                    let _ = write_to_disk(max_file_size, &command, &filepath);
-                }
-            })),
-            bg_stop: bg_stop,
-            max_size: DEFAULT_MAX_SIZE,
-            max_file_size: max_file_size,
+            max_buffers_size: DEFAULT_MAX_SIZE,
+            max_file_size: DEFAULT_MAX_SIZE,
             append_duplicate_entries: false,
         }
     }
 
+    /// Set history file name and at the same time load the history.
+    pub fn set_file_name_and_load_history<P: AsRef<Path>>(&mut self, path: P) -> io::Result<String> {
+        let status;
+        let path = path.as_ref();
+        let file = if path.exists() {
+            status = format!("opening {:?}", path);
+            File::open(path)?
+        } else {
+            status = format!("creating {:?}", path);
+            File::create(path)?
+        };
+        let reader = BufReader::new(file);
+        for line in reader.lines() {
+            match line {
+                Ok(line) => self.buffers.push_back(Buffer::from(line)),
+                Err(_) => break,
+            }
+        }
+        self.file_name = path.to_str().map(|s| s.to_owned());
+        Ok(status)
+    }
+
+    /// Set maximal number of buffers stored in memory
+    pub fn set_max_buffers_size(&mut self, size: usize) {
+        self.max_buffers_size = size;
+    }
+
+    /// Set maximal number of entries in history file
+    pub fn set_max_file_size(&mut self, size: usize) {
+        self.max_file_size = size;
+    }
+
     /// Number of items in history.
+    #[inline(always)]
     pub fn len(&self) -> usize {
         self.buffers.len()
     }
@@ -96,10 +85,6 @@ impl History {
     /// size has been met. If writing to the disk is enabled, this function will be used for
     /// logging history to the designated history file.
     pub fn push(&mut self, new_item: Buffer) -> io::Result<()> {
-        self.file_name.as_ref().map(|name| {
-            let _ = self.sender.send((new_item.clone(), name.to_owned()));
-        });
-
         // buffers[0] is the oldest entry
         // the new entry goes to the end
         if !self.append_duplicate_entries
@@ -109,12 +94,20 @@ impl History {
         }
 
         self.buffers.push_back(new_item);
-        while self.buffers.len() > self.max_size {
+        while self.buffers.len() > self.max_buffers_size {
             self.buffers.pop_front();
         }
         Ok(())
     }
 
+    /// Removes duplicate entries in the history
+    pub fn remove_duplicates(&mut self, input: &str) {
+        self.buffers.retain(|buffer| {
+            let command = buffer.lines().concat();
+            command != input
+        });
+    }
+
     /// Go through the history and try to find a buffer which starts the same as the new buffer
     /// given to this function as argument.
     pub fn get_newest_match<'a, 'b>(
@@ -134,52 +127,32 @@ impl History {
     }
 
     /// Get the history file name.
+    #[inline(always)]
     pub fn file_name(&self) -> Option<&str> {
-        match self.file_name {
-            Some(ref s) => Some(&s[..]),
-            None => None,
-        }
-    }
-
-    /// Set history file name. At the same time enable history.
-    pub fn set_file_name(&mut self, name: Option<String>) {
-        self.file_name = name;
+        self.file_name.as_ref().map(|s| s.as_str())
     }
 
-    /// Set maximal number of buffers stored in memory
-    pub fn set_max_size(&mut self, size: usize) {
-        self.max_size = size;
-    }
+    pub fn commit_to_file(&mut self) {
+        if let Some(file_name) = self.file_name.clone() {
+            // Find how many bytes we need to move backwards
+            // in the file to remove all the old commands.
+            if self.buffers.len() >= self.max_file_size {
+                let pop_out = self.buffers.len() - self.max_file_size;
+                for _ in 0..pop_out {
+                    self.buffers.pop_front();
+                }
+            }
 
-    /// Set maximal number of entries in history file
-    pub fn set_max_file_size(&mut self, size: usize) {
-        self.max_file_size.store(size, Ordering::Relaxed);
-    }
+            let mut file = BufWriter::new(File::create(&file_name)
+                // It's safe to unwrap, because the file has be loaded by this time
+                .unwrap());
 
-    /// Load history from given file name
-    pub fn load_history(&mut self) -> io::Result<()> {
-        let file_name = match self.file_name.clone() {
-            Some(name) => name,
-            None => {
-                return Err(Error::new(
-                    ErrorKind::Other,
-                    "Liner: file name not specified",
-                ))
-            }
-        };
-        let file = try!(OpenOptions::new().read(true).open(file_name));
-        let reader = BufReader::new(file);
-        for line in reader.lines() {
-            match line {
-                Ok(line) => self.buffers.push_back(Buffer::from(line)),
-                Err(_) => break,
+            // Write the commands to the history file.
+            for command in self.buffers.iter().cloned() {
+                let _ = file.write_all(&String::from(command).as_bytes());
+                let _ = file.write_all(b"\n");
             }
         }
-        Ok(())
-    }
-
-    fn buffers_ref(&self) -> &VecDeque<Buffer> {
-        &self.buffers
     }
 }
 
@@ -188,7 +161,7 @@ impl<'a> IntoIterator for &'a History {
     type IntoIter = vec_deque::Iter<'a, Buffer>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.buffers_ref().into_iter()
+        self.buffers.iter()
     }
 }
 
@@ -205,117 +178,3 @@ impl IndexMut<usize> for History {
         &mut self.buffers[index]
     }
 }
-
-/// Perform write operation. If the history file does not exist, it will be created.
-/// This function is not part of the public interface.
-/// XXX: include more information in the file (like fish does)
-fn write_to_disk(max_file_size: usize, new_item: &Buffer, file_name: &str) -> io::Result<()> {
-    let mut file = OpenOptions::new()
-        .read(true)
-        .write(true)
-        .create(true)
-        .open(file_name)?;
-
-    // The metadata contains the length of the file
-    let file_length = file.metadata().ok().map_or(0u64, |m| m.len());
-
-    {
-        // Count number of entries in file
-        let mut num_stored = 0;
-
-        // 4K byte buffer for reading chunks of the file at once.
-        let mut buffer = [0; 4096];
-
-        // Find the total number of commands in the file
-        loop {
-            // Read 4K of bytes all at once into the buffer.
-            let read = file.read(&mut buffer)?;
-            // If EOF is found, don't seek at all.
-            if read == 0 {
-                break;
-            }
-            // Count the number of commands that were found in the current buffer.
-            let cmds_read = count(&buffer[0..read], b'\n');
-            num_stored += cmds_read;
-        }
-
-        // Find how many bytes we need to move backwards
-        // in the file to remove all the old commands.
-        if num_stored >= max_file_size {
-            let mut total_read = 0u64;
-            let mut move_dist = 0u64;
-            file.seek(SeekFrom::Start(0))?;
-
-            let mut eread = 0;
-            loop {
-                // Read 4K of bytes all at once into the buffer.
-                let read = file.read(&mut buffer)?;
-                // If EOF is found, don't seek at all.
-                if read == 0 {
-                    break;
-                }
-                // Count the number of commands that were found in the current buffer
-                let cmds_read = count(&buffer[0..read], b'\n');
-
-                if eread + cmds_read >= num_stored - max_file_size {
-                    for &byte in buffer[0..read].iter() {
-                        total_read += 1;
-                        if byte == b'\n' {
-                            if eread == num_stored - max_file_size {
-                                move_dist = total_read;
-                                break;
-                            }
-                            eread += 1;
-                        }
-                    }
-                    break;
-                } else {
-                    total_read += read as u64;
-                    eread += cmds_read;
-                }
-            }
-
-
-            // Move it all back
-            move_file_contents_backward(&mut file, move_dist)?;
-        }
-    };
-
-
-    // Seek to end for appending
-    try!(file.seek(SeekFrom::End(0)));
-    // Write the command to the history file.
-    try!(file.write_all(String::from(new_item.clone()).as_bytes()));
-    try!(file.write_all(b"\n"));
-    file.flush()?;
-
-    Ok(())
-}
-
-fn move_file_contents_backward(file: &mut File, distance: u64) -> io::Result<()> {
-    let mut total_read = 0;
-    let mut buffer = [0u8; 4096];
-
-    file.seek(SeekFrom::Start(distance))?;
-    
-    loop {
-        // Read 4K of bytes all at once into the buffer.
-        let read = file.read(&mut buffer)?;
-        total_read += read as u64;
-        // If EOF is found, don't seek at all.
-        if read == 0 {
-            break;
-        }
-
-        file.seek(SeekFrom::Current(-(read as i64 + distance as i64)))?;
-
-
-        file.write_all(&buffer[..read])?;
-        file.seek(SeekFrom::Current(distance as i64))?;
-    }
-
-    file.set_len(total_read)?;
-    file.flush()?;
-
-    Ok(())
-}

src/test.rs

@@ -74,12 +74,12 @@ fn test_history_indexing() {
 #[test]
 fn test_in_memory_history_truncating() {
     let mut h = History::new();