-
-
Notifications
You must be signed in to change notification settings - Fork 595
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 support #1375
Add xonsh support #1375
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was quick. I guess xonsh has no mechanism for altering the ctrl-R keybindings?
This PR is blocked while i wait for a reply from the xonsh community.
I couldn't find any at the very least |
We already need to do that for nushell, so it's annoying but not that odd - https://atuin.sh/docs/advanced-install#nushell I think the keybindings can be set using the |
In that case, i'll use a temp file for now. |
What's the status of this PR? I have been working getting atuin to work with my xonsh install and wound up with something fairly similar, except that I'm using I'd be happy to contribute changes to this PR, or submit a new one if @Matthieu-LAURENT39 would prefer. |
I didn't have time to work on this PR lately, so feel free to add improvements to it! Otherwise i was planning on continuing it next week or the week after that, there isn't much left to be done. |
Followup question for the atuin maintainers: I'm working on history importers for xonsh to go with this (there are multiple importers because, like zsh and nushell, xonsh has an alternate mode that stores history in a sqlite db.) I have two questions:
|
We don't rely on it! It's purely for index performance. I'd prefer if you could generate a new UUIDv7 if possible, though if not the Xonsh ID is also OK
This would be reasonable! If we don't have the information then there is little that can be done. |
Summary of changes: * Added duration to postcommand hook * Switched main search operation to use `subproccess.run()` rather than running as an xonsh shell command - this a) allows us to capture stderr without needing a temporary file and b) avoids a weird broken-buffer state that results from running a fullscreen TUI and then programmatically editing the buffer * Added support for immediately executing chosen command via `__atuin_accept__:` (like bash/zsh/fish)
a084eaf
to
ac58059
Compare
@jfmontanaro do you have anything else you want to add, or should i mark the PR as ready for review? |
I think you can go ahead and mark it ready. I am working on functionality for importing xonsh history, but I think I'll just open another PR for that later. |
I'm adding support for |
In the end, i went with what it seems to do for bash, aka only check once during the initial sourcing, and not checking again afterwards. Anyways, that's all i wanted to add, this should be ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understand of Xonsh (not much), looks great! Thank you both 🙏
Would one of you also be able to update the docs please? 🙏
Seeing as this is your first time contributing, if you would like a holographic contributors-only Atuin sticker, then please fill out this form!
We do also have a Discord if you'd like to ask any questions, or just fancy hanging out!
I have some free time, i'll try to make a PR to add some documentation 👍 |
This adds xonsh-related informations to the docs. Related PR: atuinsh/atuin#1375
Guys from this PR - you rock! Thank you! 🎉 |
This PR aims to add support for Xonsh to atuin.