fix(xonsh): Add xonsh to auto import, respect $HISTFILE in xonsh import, and fix issue with up-arrow keybinding in xonsh (#1711)

* add xonsh to `atuin import auto`

* respect $HISTFILE in xonsh importers

* disable up-arrow binding in xonsh when completion menu is active

* include xonsh logic in the same conditional as other shells

* format and fix clippy lints

4 files changed, 48 insertions, 22 deletions

diff --git a/atuin-client/src/import/xonsh.rs b/atuin-client/src/import/xonsh.rs
index 2269212f..8a37c715 100644
--- a/atuin-client/src/import/xonsh.rs
+++ b/atuin-client/src/import/xonsh.rs
@@ -10,7 +10,7 @@ use time::OffsetDateTime;
 use uuid::timestamp::{context::NoContext, Timestamp};
 use uuid::Uuid;
 
-use super::{Importer, Loader};
+use super::{get_histpath, Importer, Loader};
 use crate::history::History;
 
 // Note: both HistoryFile and HistoryData have other keys present in the JSON, we don't
@@ -41,7 +41,7 @@ pub struct Xonsh {
     hostname: String,
 }
 
-fn get_hist_dir(xonsh_data_dir: Option<String>) -> Result<PathBuf> {
+fn xonsh_hist_dir(xonsh_data_dir: Option<String>) -> Result<PathBuf> {
     // if running within xonsh, this will be available
     if let Some(d) = xonsh_data_dir {
         let mut path = PathBuf::from(d);
@@ -107,7 +107,9 @@ impl Importer for Xonsh {
     const NAME: &'static str = "xonsh";
 
     async fn new() -> Result<Self> {
-        let hist_dir = get_hist_dir(env::var("XONSH_DATA_DIR").ok())?;
+        // wrap xonsh-specific path resolver in general one so that it respects $HISTPATH
+        let xonsh_data_dir = env::var("XONSH_DATA_DIR").ok();
+        let hist_dir = get_histpath(|| xonsh_hist_dir(xonsh_data_dir))?;
         let sessions = load_sessions(&hist_dir)?;
         let hostname = get_hostname();
         Ok(Xonsh { sessions, hostname })
@@ -167,7 +169,7 @@ mod tests {
 
     #[test]
     fn test_hist_dir_xonsh() {
-        let hist_dir = get_hist_dir(Some("/home/user/xonsh_data".to_string())).unwrap();
+        let hist_dir = xonsh_hist_dir(Some("/home/user/xonsh_data".to_string())).unwrap();
         assert_eq!(
             hist_dir,
             PathBuf::from("/home/user/xonsh_data/history_json")
diff --git a/atuin-client/src/import/xonsh_sqlite.rs b/atuin-client/src/import/xonsh_sqlite.rs
index 8310c375..de59d477 100644
--- a/atuin-client/src/import/xonsh_sqlite.rs
+++ b/atuin-client/src/import/xonsh_sqlite.rs
@@ -10,7 +10,7 @@ use time::OffsetDateTime;
 use uuid::timestamp::{context::NoContext, Timestamp};
 use uuid::Uuid;
 
-use super::{Importer, Loader};
+use super::{get_histpath, Importer, Loader};
 use crate::history::History;
 
 #[derive(Debug, FromRow)]
@@ -57,7 +57,7 @@ impl HistDbEntry {
     }
 }
 
-fn get_db_path(xonsh_data_dir: Option<String>) -> Result<PathBuf> {
+fn xonsh_db_path(xonsh_data_dir: Option<String>) -> Result<PathBuf> {
     // if running within xonsh, this will be available
     if let Some(d) = xonsh_data_dir {
         let mut path = PathBuf::from(d);
@@ -98,7 +98,9 @@ impl Importer for XonshSqlite {
     const NAME: &'static str = "xonsh_sqlite";
 
     async fn new() -> Result<Self> {
-        let db_path = get_db_path(env::var("XONSH_DATA_DIR").ok())?;
+        // wrap xonsh-specific path resolver in general one so that it respects $HISTPATH
+        let xonsh_data_dir = env::var("XONSH_DATA_DIR").ok();
+        let db_path = get_histpath(|| xonsh_db_path(xonsh_data_dir))?;
         let connection_str = db_path.to_str().ok_or_else(|| {
             eyre!(
                 "Invalid path for SQLite database: {}",
@@ -151,7 +153,7 @@ mod tests {
 
     #[test]
     fn test_db_path_xonsh() {
-        let db_path = get_db_path(Some("/home/user/xonsh_data".to_string())).unwrap();
+        let db_path = xonsh_db_path(Some("/home/user/xonsh_data".to_string())).unwrap();
         assert_eq!(
             db_path,
             PathBuf::from("/home/user/xonsh_data/xonsh-history.sqlite")
diff --git a/atuin/src/command/client/import.rs b/atuin/src/command/client/import.rs
index 4ab9ba61..35595b9b 100644
--- a/atuin/src/command/client/import.rs
+++ b/atuin/src/command/client/import.rs
@@ -59,8 +59,18 @@ impl Cmd {
                     return Ok(());
                 }
 
+                // $XONSH_HISTORY_BACKEND isn't always set, but $XONSH_HISTORY_FILE is
+                let xonsh_histfile =
+                    env::var("XONSH_HISTORY_FILE").unwrap_or_else(|_| String::new());
                 let shell = env::var("SHELL").unwrap_or_else(|_| String::from("NO_SHELL"));
-                if shell.ends_with("/zsh") {
+
+                if xonsh_histfile.to_lowercase().ends_with(".json") {
+                    println!("Detected Xonsh",);
+                    import::<Xonsh, DB>(db).await
+                } else if xonsh_histfile.to_lowercase().ends_with(".sqlite") {
+                    println!("Detected Xonsh (SQLite backend)");
+                    import::<XonshSqlite, DB>(db).await
+                } else if shell.ends_with("/zsh") {
                     if ZshHistDb::histpath().is_ok() {
                         println!(
                             "Detected Zsh-HistDb, using :{}",
diff --git a/atuin/src/shell/atuin.xsh b/atuin/src/shell/atuin.xsh
index 5df26b26..486d7784 100644
--- a/atuin/src/shell/atuin.xsh
+++ b/atuin/src/shell/atuin.xsh
@@ -1,8 +1,11 @@
 import subprocess
+
+from prompt_toolkit.application.current import get_app
+from prompt_toolkit.filters import Condition
 from prompt_toolkit.keys import Keys
 
-$ATUIN_SESSION=$(atuin uuid).rstrip('\n')
 
+$ATUIN_SESSION=$(atuin uuid).rstrip('\n')
 
 @events.on_precommand
 def _atuin_precommand(cmd: str):
@@ -52,16 +55,25 @@ def _search(event, extra_args: list[str]):
 
 @events.on_ptk_create
 def _custom_keybindings(bindings, **kw):
-    @bindings.add(Keys.ControlR, filter=_ATUIN_BIND_CTRL_R)
-    def r_search(event):
-        _search(event, extra_args=[])
-
-    @bindings.add(Keys.Up, filter=_ATUIN_BIND_UP_ARROW)
-    def up_search(event):
-        # Only trigger if the buffer is a single line
-        if not '\n' in buffer.text:
+    if _ATUIN_BIND_CTRL_R:
+        @bindings.add(Keys.ControlR)
+        def r_search(event):
+            _search(event, extra_args=[])
+
+    if _ATUIN_BIND_UP_ARROW:
+        @Condition
+        def should_search():
+            buffer = get_app().current_buffer
+            # disable keybind when there is an active completion, so
+            # that up arrow can be used to navigate completion menu
+            if buffer.complete_state is not None:
+                return False
+            # similarly, disable when buffer text contains multiple lines
+            if '\n' in buffer.text:
+                return False
+
+            return True
+
+        @bindings.add(Keys.Up, filter=should_search)
+        def up_search(event):
             _search(event, extra_args=["--shell-up-key-binding"])
-            return
-        
-        # Run the default behavior for up arrow
-        event.current_buffer.auto_up(count=event.arg)