-
-
Notifications
You must be signed in to change notification settings - Fork 686
Add PowerShell module #2543
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
base: main
Are you sure you want to change the base?
Add PowerShell module #2543
Conversation
I've tested it and everything works well. Now I'm a happy user. Thank you for your work! |
Thank you @ltrzesniewski and lzybkr, appreciate the work ❤️ Confirming Successful Test on
About TestsDetails on test steps and commands run at https://github.com/justunsix/rust-tests/blob/main/atuin-tests/pr2543/README.md
Error not related to PRSharing an error found during testing in case others find it during their testing, though it is a local issue and not related to the PR itself. The error also occurs when testing with atuin/main in nushell in the same environment, so error
Error: migration 20230531212437 was previously applied but has been modified
>>
>> Location:
>> C:\Users\user1\usr\reference\atuin\crates\atuin-client\src\record\sqlite_store.rs:61:9
Error:: The term 'Error:' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
|
@justunsix thanks for testing and the detailed report! Sorry, I should have mentioned the database issue in the OP, I'll add a warning right now. 😬 |
fix as explained at atuinsh/atuin#2543 (comment)
cbefeb6
to
59c6f95
Compare
I found an issue with encoding and pushed a fix: I took the opportunity to add support for an The compiler was also complaining about an elided lifetime, but not every time for some reason, so I made it explicit. |
Re-tested after the push and confirming successful tests on:
Additions from the earlier testing:
Comments:
<TabItem label="powershell">
Add
```shell
atuin init powershell | Out-String | Invoke-Expression
```
to your $PROFILE file.
If you use a prompt that has more than one line, also add
```shell
$env:ATUIN_POWERSHELL_PROMPT_OFFSET=-X
```
to your $PROFILE file.
Replace X with the number of lines in your prompt, but take away 1.
For example, if your prompt has 2 lines, the environment variable would be -1.
</TabItem> |
Amazing, cannot wait! Thanks for the hard work. |
Atuin is launched but nothing appears on screen. I don't see anything relevant in the changelog. I guess I'll have to take a look. |
Awh man :( What part breaks? |
FWIW, it still works on Linux with pwsh 7.5.0 |
This is caused by PowerShell/PowerShell#20853 - looks like I'd have to manually copy Atuin's stdout to PowerShell's stdout through the script. Ugh. That's cursed. I'll need to think a bit more about it and maybe find a better solution. In any case, it doesn't look like a simple change, so I'm converting this PR to draft for now. 😞 |
ac9ae46
to
21bb83a
Compare
I worked around the issue by adding a hidden command line option which tells Atuin where to write the result instead of going through stderr. I hope that's ok for you. We may also see if the issue I reported here gets fixed and drop the second commit if you prefer stderr. |
Pulled from atuinsh#2543 Fixes interactive mode in fish where the terminal wasn't being displayed properly. fixes atuinsh#1289 Co-authored-by: Lucas Trzesniewski
Pulled from atuinsh#2543 Fixes interactive mode in fish where the terminal wasn't being displayed properly. fixes atuinsh#1289 Co-authored-by: Lucas Trzesniewski <[email protected]>
Pulled from atuinsh#2543 Fixes interactive mode in fish where the terminal wasn't being displayed properly. fixes atuinsh#1289 Co-authored-by: Lucas Trzesniewski <[email protected]>
a73924b
to
17b7319
Compare
I was a bit bored so I added dotfiles support since it was missing 😅 A few notes:
|
17b7319
to
5cccb33
Compare
Thank you for all that! Appreciate you keeping this up to date. I'm happy with the hidden command flag + have no strong preference for stderr. A couple of thoughts
Thanks again for all of your work here! <3 |
Drive-by question as a PowerShell user not that familiar with Atuinsh's architecture:
What gets written to this file/location and would it leak history/commands to disk? |
@mickey1700 Run Get-Module and post what version PSReadLine is. |
@mickey1700 some delay is expected, unfortunately, as PSReadLine polls for commands every 300ms. It shouldn't be as long as 1 second though, but it's noticeable. |
If I understand correctly atuin is starting a new powershell process and maybe if your profile is large it takes longer? Just a guess I could be totally wrong |
@the-mentor nope, powershell runs atuin, and work continues in the same powershell process when atuin exits. I'm pretty sure this is due to how PSReadLine polls for commands, but I didn't spend a lot of time on this. I thought maybe an issue could be opened about this in PSReadLine about shortening this delay. |
Yeah, I was going to bring that up, but an older version of the module might have other delays. |
I have lots of powershell experience so when I get a chance I'll try to see if I can look the code and test it out. |
I'm on the latest stable version: 2.3.6. Are you using a beta version and not seeing the issue on your end?
Thanks - I definitely expected some delay, similar to what can be observed with
Good thought! I tested with the @ltrzesniewski Let me know if there’s anything else I can try to help pinpoint the cause. |
I have a delay, I just asked because "1 second" seemed a little long to me. |
Fair enough. I measured the delay during a random invocation — it took around 800 ms to return to the shell. Sometimes it's faster, other times a bit slower. |
This PR runs up to 4 PSReadLine commands after Atuin exits: |
I've tested this on PowerShell 7.5.1 on WSL, and I found a couple of bugs (and a fix for one of them). The two I don't have fixes for are fairly generic, so I'm not sure they would show up for other platforms. I'm also not sure if any of these bugs are the actual issue that was reported to PowerShell, but it seems at least related, so I quoted the previous comment just in case. Bug 1I have $suggestion = (Get-Content -Raw $resultFile).Trim() Fix 1I know it's not very DRY, but if the [Microsoft.PowerShell.PSConsoleReadLine]::Insert($suggestion.Substring(17).Trim()) and: [Microsoft.PowerShell.PSConsoleReadLine]::Insert($suggestion.Trim()) (respectively); The same error also occurs when it's checking for the if ($null -eq $suggestion) { $suggestion = '' } # <- convert $suggestion to empty string instead of $null
if ( $suggestion.StartsWith('__atuin_accept__:')) or for PowerShell 7 only, you can change the line to: if ( ${ $suggestion }?.StartsWith('__atuin_accept__:') ) # <- PS 7 syntax only I prefer not to say "the error goes away" after these changes, but the scriptblock does handle the Bug 2Accepting the line from Atuin's history (whether with I don't know if this is intended or not, but it's also not a major problem, more like an interesting quirk. I'm not familiar enough with Bug 3Pressing Again, this is not a major problem, but it is a slight bother having to hit Bug 4PowerShell history in a Linux or Linux-based environment gets added to the same database as the other shells. Not sure if this is fixable, but it would be nice to have the two shells handled separately, if that's possible. |
Hello @Vacant0mens, thank you very much for testing and providing a helpful report! However, the behavior you are experiencing seems to come from using an Atuin PowerShell module which is incompatible with the Could you please run If it is something else, you can install the latest version from this page with: cargo install --git https://github.com/ltrzesniewski/atuin.git --branch powershell-pr
atuin init powershell | Out-String | Invoke-Expression After installing it with cargo, All right, let's take a closer look. Bug 1 This is very surprising to me, as PowerShell shouldn't execute bindings while Atuin is running. Could you show me the command you used to bind I also tested the following: It beeps when I press Escape in the shell (the terminal shows a 🔔 icon in the tab at least), but not when I exit Atuin. The line you mention: $suggestion = (Get-Content -Raw $resultFile).Trim() comes from a previous version of this PR. The current version has the following: $suggestion = (Get-Content -Raw $resultFile -Encoding UTF8 | Out-String).Trim() The BTW, I can't use the PowerShell 7 syntax, as I want for this feature to work on the PowerShell 5 version which is shipped with Windows. Bug 2 This is something which was very frustrating, and which I have fixed some time ago. Here's the behavior I get: Bug 3 Pressing enter should execute the selected line. At least it works for me. I suppose the Bug 4 This is default Atuin behavior, but you can use a separate Atuin configuration in your PowerShell session if that's the behavior you'd like to have. You can set a separate |
Ah, darn. My bad. 🤦 That said, for a version of atuin that wasn't built with/for the PowerShell additions and for using an older scriptblock, it worked surprisingly well all things considered! 🎉 I went back and installed the beta version from your branch as you suggested and used the Good job with this PR! Can't wait for this to get merged and released! 👋 |
I've also managed to install it just fine on Windows with the same methods. Otherwise it works great! I haven't had a chance to test it on a Debian machine yet (other than WSL), but I will try to sometime this week. |
That may just be a different config, see if it is related to #2249 |
I am syncing my PowerShell and WSL |
@lapo-luchini This should not depend on this PR. I suppose you're either calling the same Atuin does the following: if You may check this with commands such as |
Mhh, no, definitely separated folders:
vs
|
This might be normal tough, as I see that |
Used this a workaround:
|
ac08165
to
52beca9
Compare
I rebased the PR on the v18.7.1 release and fixed an issue with quote characters on PS 5.1 /cc @rhinosfly @ajn142 |
This adds PowerShell support by invoking the following expression: atuin init powershell | Out-String | Invoke-Expression Co-authored-by: Jason Shirk <[email protected]>
PowerShell handling of native command lines depends on many things which are difficult to control, such as the OS, the shell version, or even variables such as $PSNativeCommandArgumentPassing. See the about_Parsing help page for more details. Going through environment variables should make everything consistent.
52beca9
to
1e24dd8
Compare
Well, there has been a new Atuin release today, so I rebased the PR on v18.8.0, and added support for command chaining (#2834). |
This adds PowerShell support 🎉
I built this script around @lzybkr's prototype, so I added him as co-author (I hope that's ok). I wouldn't know where to start without his contribution.
I'm not a PowerShell expert, so this was a nice opportunity to learn some stuff. I think it's ok, but I would appreciate if someone more knowledgeable in the matter could review this though.
It would be nice if other PowerShell users could test this with their configs and report any issues. I wouldn't be surprised if there are some remaining bugs or missing features.
Fixes #84
Installation
If you'd like to test this, you can install the
atuin
from this PR by running:Then, add the following to your PowerShell profile file (whose path is in
$PROFILE
) and restart the shell:This requires
atuin
to be in the path and the PSReadLine module to be installed, which is the case by default.Tests
I tested this on the following:
I also tested this with and without my custom Oh My Posh prompt. It works fine in both cases, except that since my OMP config contains
"newline": true
, my prompt is multiline and shifts downwards by a single line on eachatuin search -i
invocation. This can be adjusted with the$env:ATUIN_POWERSHELL_PROMPT_OFFSET
environment variable (e.g. I set mine to-1
to account for the additional prompt line).Checks