From 3fa87ef1e2f23e8929f65ea3389971c9d4b3a927 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Sun, 18 Sep 2022 17:04:42 -0500 Subject: [PATCH] change history session_id to a systematically generated number (#481) * change history session_id to a systematically generated number * added env var, removed comments * tweak test * add public function to get the history session id * switch from reedline commit date to unix_epoch --- src/engine.rs | 33 ++++++++++++++++++++++----------- src/history/base.rs | 5 +++-- src/history/file_backed.rs | 8 +------- src/history/sqlite_backed.rs | 21 +++------------------ 4 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index bbad801..c207d92 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,8 +3,6 @@ use crate::{ history::SearchFilter, menu_functions::{parse_selection_char, ParseAction}, }; - -use crate::result::{ReedlineError, ReedlineErrorVariants}; #[cfg(feature = "external_printer")] use { crate::external_printer::ExternalPrinter, @@ -25,6 +23,7 @@ use { }, painting::{Painter, PromptLines}, prompt::{PromptEditMode, PromptHistorySearchStatus}, + result::{ReedlineError, ReedlineErrorVariants}, utils::text_manipulation, EditCommand, ExampleHighlighter, Highlighter, LineBuffer, Menu, MenuEvent, Prompt, PromptHistorySearch, ReedlineMenu, Signal, UndoBehavior, ValidationResult, Validator, @@ -34,7 +33,9 @@ use { event::{Event, KeyCode, KeyEvent, KeyModifiers}, terminal, Result, }, - std::{borrow::Borrow, fs::File, io, io::Write, process::Command, time::Duration}, + std::{ + borrow::Borrow, fs::File, io, io::Write, process::Command, time::Duration, time::SystemTime, + }, }; // The POLL_WAIT is used to specify for how long the POLL should wait for @@ -155,6 +156,7 @@ impl Reedline { let hinter = None; let validator = None; let edit_mode = Box::new(Emacs::default()); + let hist_session_id = Self::create_history_session_id(); Reedline { editor: Editor::default(), @@ -162,7 +164,7 @@ impl Reedline { history_cursor: HistoryCursor::new(HistoryNavigationQuery::Normal( LineBuffer::default(), )), - history_session_id: None, + history_session_id: hist_session_id, history_last_run_id: None, input_mode: InputMode::Regular, painter, @@ -182,6 +184,21 @@ impl Reedline { } } + /// Get a new history session id based on the current time and the first commit datetime of reedline + fn create_history_session_id() -> Option { + let nanos = match SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) { + Ok(n) => n.as_nanos() as i64, + Err(_) => 0, + }; + + Some(HistorySessionId::new(nanos)) + } + + /// Return the previously generated history session id + pub fn get_history_session_id(&self) -> Option { + self.history_session_id + } + /// A builder to include a [`Hinter`] in your instance of the Reedline engine /// # Example /// ```rust @@ -856,13 +873,7 @@ impl Reedline { let buf = self.editor.get_buffer(); if !buf.is_empty() { let mut entry = HistoryItem::from_command_line(buf); - // todo: in theory there's a race condition here because another shell might get the next session id at the same time - entry.session_id = - Some(*self.history_session_id.get_or_insert_with(|| { - self.history - .next_session_id() - .expect("todo: error handling") - })); + entry.session_id = self.history_session_id; let entry = self.history.save(entry).expect("todo: error handling"); self.history_last_run_id = entry.id; } diff --git a/src/history/base.rs b/src/history/base.rs index c001b29..8229820 100644 --- a/src/history/base.rs +++ b/src/history/base.rs @@ -2,7 +2,7 @@ use chrono::Utc; use crate::{core_editor::LineBuffer, HistoryItem, Result}; -use super::{HistoryItemId, HistorySessionId}; +use super::HistoryItemId; /// Browsing modes for a [`History`] #[derive(Debug, Clone, PartialEq, Eq)] @@ -149,7 +149,6 @@ pub trait History: Send { fn load(&self, id: HistoryItemId) -> Result; /// retrieves the next unused session id - fn next_session_id(&mut self) -> Result; /// count the results of a query fn count(&self, query: SearchQuery) -> Result; @@ -179,6 +178,8 @@ mod test { #[cfg(not(feature = "sqlite"))] const IS_FILE_BASED: bool = true; + use crate::HistorySessionId; + fn create_item(session: i64, cwd: &str, cmd: &str, exit_status: i64) -> HistoryItem { HistoryItem { id: None, diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index be6edba..167e867 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -1,6 +1,5 @@ use super::{ - base::CommandLineSearch, History, HistoryItem, HistoryItemId, HistorySessionId, - SearchDirection, SearchQuery, + base::CommandLineSearch, History, HistoryItem, HistoryItemId, SearchDirection, SearchQuery, }; use crate::{ result::{ReedlineError, ReedlineErrorVariants}, @@ -256,11 +255,6 @@ impl History for FileBackedHistory { } Ok(()) } - - fn next_session_id(&mut self) -> Result { - // doesn't support separating out different sessions - Ok(HistorySessionId::new(0)) - } } impl FileBackedHistory { diff --git a/src/history/sqlite_backed.rs b/src/history/sqlite_backed.rs index 73d91be..ea57e9f 100644 --- a/src/history/sqlite_backed.rs +++ b/src/history/sqlite_backed.rs @@ -1,6 +1,3 @@ -use chrono::{TimeZone, Utc}; -use rusqlite::{named_params, params, Connection, ToSql}; - use super::{ base::{CommandLineSearch, SearchDirection, SearchQuery}, History, HistoryItem, HistoryItemId, HistorySessionId, @@ -9,10 +6,10 @@ use crate::{ result::{ReedlineError, ReedlineErrorVariants}, Result, }; - -const SQLITE_APPLICATION_ID: i32 = 1151497937; - +use chrono::{TimeZone, Utc}; +use rusqlite::{named_params, params, Connection, ToSql}; use std::{path::PathBuf, time::Duration}; +const SQLITE_APPLICATION_ID: i32 = 1151497937; /// A history that stores the values to an SQLite database. /// In addition to storing the command, the history can store an additional arbitrary HistoryEntryContext, @@ -156,18 +153,6 @@ impl History for SqliteBackedHistory { // no-op (todo?) Ok(()) } - - fn next_session_id(&mut self) -> Result { - Ok(HistorySessionId::new( - self.db - .query_row( - "select coalesce(max(session_id), 0) + 1 from history", - params![], - |r| r.get(0), - ) - .map_err(map_sqlite_err)?, - )) - } } fn map_sqlite_err(err: rusqlite::Error) -> ReedlineError { // TODO: better error mapping