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

@@ -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<HistorySessionId> {
+        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<HistorySessionId> {
+        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;
                         }

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<HistoryItem>;
 
     /// retrieves the next unused session id
-    fn next_session_id(&mut self) -> Result<HistorySessionId>;
 
     /// count the results of a query
     fn count(&self, query: SearchQuery) -> Result<i64>;
@@ -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,

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<HistorySessionId> {
-        // doesn't support separating out different sessions
-        Ok(HistorySessionId::new(0))
-    }
 }
 
 impl FileBackedHistory {

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<HistorySessionId> {
-        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