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

Don't panic when creating FileBackedHistory with usize::MAX capacity #701

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/demo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use reedline::CursorConfig;
#[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))]
use reedline::FileBackedHistory;

fn main() -> std::io::Result<()> {
fn main() -> reedline::Result<()> {
println!("Ctrl-D to quit");
// quick command like parameter handling
let vi_mode = matches!(std::env::args().nth(1), Some(x) if x == "--vi");
Expand Down
10 changes: 9 additions & 1 deletion src/history/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ mod test {
#[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))]
#[test]
fn history_size_zero() -> Result<()> {
let mut history = crate::FileBackedHistory::new(0);
let mut history = crate::FileBackedHistory::new(0)?;
history.save(create_item(1, "/home/me", "cd ~/Downloads", 0))?;
assert_eq!(history.count_all()?, 0);
let _ = history.sync();
Expand All @@ -440,4 +440,12 @@ mod test {

Ok(())
}

#[test]
fn create_file_backed_history() {
use crate::HISTORY_SIZE;

assert!(crate::FileBackedHistory::new(usize::MAX).is_err());
assert!(crate::FileBackedHistory::new(HISTORY_SIZE).is_ok());
}
}
27 changes: 17 additions & 10 deletions src/history/file_backed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,15 @@ impl Default for FileBackedHistory {
/// Creates an in-memory [`History`] with a maximal capacity of [`HISTORY_SIZE`].
///
/// To create a [`History`] that is synchronized with a file use [`FileBackedHistory::with_file()`]
///
/// # Panics
///
/// If `HISTORY_SIZE == usize::MAX`
fn default() -> Self {
Self::new(HISTORY_SIZE)
match Self::new(HISTORY_SIZE) {
Ok(history) => history,
Err(e) => panic!("{}", e),
}
}
}

Expand Down Expand Up @@ -286,20 +293,20 @@ impl History for FileBackedHistory {
impl FileBackedHistory {
/// Creates a new in-memory history that remembers `n <= capacity` elements
///
/// # Panics
///
/// If `capacity == usize::MAX`
pub fn new(capacity: usize) -> Self {
pub fn new(capacity: usize) -> Result<Self> {
if capacity == usize::MAX {
panic!("History capacity too large to be addressed safely");
return Err(ReedlineError(ReedlineErrorVariants::OtherHistoryError(
"History capacity too large to be addressed safely",
)));
}
FileBackedHistory {

Ok(FileBackedHistory {
capacity,
entries: VecDeque::new(),
file: None,
len_on_disk: 0,
session: None,
}
})
}

/// Creates a new history with an associated history file.
Expand All @@ -310,8 +317,8 @@ impl FileBackedHistory {
///
/// **Side effects:** creates all nested directories to the file
///
pub fn with_file(capacity: usize, file: PathBuf) -> std::io::Result<Self> {
let mut hist = Self::new(capacity);
pub fn with_file(capacity: usize, file: PathBuf) -> Result<Self> {
let mut hist = Self::new(capacity)?;
if let Some(base_dir) = file.parent() {
std::fs::create_dir_all(base_dir)?;
}
Expand Down
6 changes: 6 additions & 0 deletions src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ pub enum ReedlineErrorVariants {
#[derive(Debug)]
pub struct ReedlineError(pub ReedlineErrorVariants);

impl From<std::io::Error> for ReedlineError {
fn from(err: std::io::Error) -> Self {
Self(ReedlineErrorVariants::IOError(err))
}
}

impl Display for ReedlineError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
Expand Down