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

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

Merged
merged 5 commits into from
Feb 15, 2024

Conversation

jfmontanaro
Copy link
Contributor

Hi! I'm back again with more Xonsh stuff. Turns out you can't get rid of me that easily! 😈

This PR contains 3 somewhat-unrelated changes, which I bundled together because it seemed easier than splitting them into separate PRs and then trying to keep them all in sync with any changes. As always, though, happy to change the approach if it would be better otherwise! :)

Summery of changes:

  • Added Xonsh-detection to atuin import auto
  • Fixed Xonsh importers to respect the presence of $HISTFILE in the env
  • Fixed an issue with up-arrow keybinding clobbering Xonsh's built-in behavior when choosing items from autosuggest menu

Detecting Xonsh in atuin import auto

The only weird thing about this is that Xonsh does not by default do anything to $SHELL so we can't rely on that to know whether we are in Xonsh. Instead I'm looking at $XONSH_HISTORY_FILE, which should (if I understand correctly) always be set in an Xonsh session unless the user has specifically decided to unset it. Conveniently, we can also determine whether to use the JSON or SQLite importer by looking at the suffix of $XONSH_HISTORY_FILE.

Rsepcting $HISTFILE in Xonsh importers

While working on updating the docs to describe the behavior of the Xonsh importers as discussed in my previous PR, I discovered the whole thing with $HISTFILE, which I hadn't been aware of. So I updated the Xonsh importers to respect that.

One note (which I will also include in the docs once this is all sorted out) $HISTFILE means something slightly different when Xonsh is running in its default JSON mode. In that case, because the history is stored in a bunch of different files rather than a single one, $HISTFILE should be set to the parent directory of those JSON files: by default, ~/.local/share/xonsh/history_json/.

Disabling the up-arrow binding in Xonsh when choosing an autocomplete suggestion

Much like in #1025, there's a situation in Xonsh when we don't want to be overriding the default behavior of the up-arrow. When you hit Tab and there are multiple possible completions for a command, Xonsh will bring up a menu that allows you to choose between them. You navigate this menu with the arrow keys, but unfortunately Atuin's up-arrow binding is still active when you're navigating this menu, so any time you hit Up you're dumped into an Atuin session that you probably weren't looking for. Fortunately Xonsh (actually the underlying prompt_toolkit instance) exposes enough of its state that we can detect when there's an active auto-complete and disable the binding in that case.

While I was at it I also slightly modified the Xonsh script to not even register its handlers if it was called with --disable-up-arrow or --disable-ctrl-r. I think this is ever-so-slightly preferable because even if those handlers were always cancelled by the filter, that's still an extra check that the shell doesn't have to do.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Thank you. Appreciate you following up with improvements 🙏

@ellie ellie merged commit 4512cd5 into atuinsh:main Feb 15, 2024
15 checks passed
ellie pushed a commit to atuinsh/docs that referenced this pull request Feb 21, 2024
Documentation for Xonsh importers, as discussed in atuinsh/atuin#1678
and atuinsh/atuin#1711.

I also noticed that the TOC wasn't generating properly on the page, so I
changed the h1's to h2's and that seems to have fixed it. This does
slightly change the appearance of the page, so I'm happy to revert that
if you'd prefer to keep it the way it was.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants