knqyf263 / pet

Simple command-line snippet manager
MIT License
4.45k stars 224 forks source link

Windows PowerShell Support #140

Open smetroid opened 4 years ago

smetroid commented 4 years ago

Hello,

Great tool, use it on my MacOS and Linux environment, now I am trying to use it with PowerShell/Windows. I can create new commands entries, however when it comes to pet exec the default shell is dos(cmd.exe) and not PowerShell. Still good enough to save the the crazy PowerShell commands I have to remember, however it would be nice to have it execute the command as well.

Thanks in advance. If you can guide me through where to make the changes/updates, I can make changes, test, and make a PR if anyone could else would be interested.

-- Enrique

RamiAwar commented 7 months ago

Hey Enrique! I can look into this for you and guide you through it :)

Right now we have a file called util_windows.go (and util_unix.go), these define the run function which is used to actually execute the command in question. We'd want to somehow create an alternative run that uses powershell I'm guessing.

We'd either have to:

  1. Change the run signature to also accept the 'shell type' from somewhere. Maybe we can get this from the shell itself (some way to detect if we're running in powershell) or somewhere else idk.
  2. Create a global config flag that decides which shell to use. That means you're limited to using either powershell or cmd until you change your global pet config.

I think the global config option is way easier to implement and less disruptive. I'd go for that as a first step if I were you!

And we might have to update the 'selection' as well potentially, not sure about that one.

smetroid commented 7 months ago

Thanks for your response on this @RamiAwar ... I've moved away from PowerShell at the moment, but I am sure this would still be a nice feature for some other people who are working with PowerShell.

I can close this ticket if I need to, or I can leave it open to be referenced in a PR.

RamiAwar commented 7 months ago

I see, yeah no worries! Yeah let's keep it open, I think it's easy to implement and I'll probably get to it later!

cderv commented 6 days ago

I was trying to use pet on Windows, as I found windows bundle in the release. Is this supposed to work in Powershell ? or only meant for other use in Windows (like Git Bash) ?

Asking as I found issues with some commands. Thanks!

RamiAwar commented 6 days ago

@cderv Salut! It's actually meant to work with any shell, we just haven't added Powershell support in an automatic way yet. Right now you could customize it I think by modifying this variable and replacing cmd with the powershell binary. https://github.com/knqyf263/pet/blob/f88549cbb1e7eb4853bf7929b659e083076bfc92/cmd/util_windows.go#L18

I haven't tested on Windows in a while tho, maybe you can try it out and let me know if it works! Then we can document it.

cderv commented 6 days ago

I haven't tested on Windows in a while tho, maybe you can try it out and let me know if it works! Then we can document it.

I am trying to use it on Windows. So let me open issues for problem I am finding. I am not (yet) knowledgeable on GO so opening issues will also help me understand where to look.

Or is it better here to open discussion that you then convert to issues ? let me know the guidelines.

RamiAwar commented 5 days ago

@cderv Feel free to open issues if you think they're different from this one! I'll review them separately.

Sorry I didn't mean to suggest you had to modify the source code - I just meant you should try modifying your Pet global settings -> Conf.General.Cmd and set that to the powershell binary (probably just PowerShell) and then maybe Pet will find and use it.

I might have access to a windows machine tonight, might just test this out myself!

cderv commented 5 days ago

Sorry I didn't mean to suggest you had to modify the source code - I just meant you should try modifying your Pet global settings -> Conf.General.Cmd and set that to the powershell binary (probably just PowerShell) and then maybe Pet will find and use it.

Thanks ! I deduced that looking at source code. pet configure was part of the problem (see #314), so once I found the config file, I tried the setting Cmd = "pwsh"and it does not work because it would require special handling for passing command instruction like Cmd https://github.com/knqyf263/pet/blob/f88549cbb1e7eb4853bf7929b659e083076bfc92/cmd/util_windows.go#L17-L23

Example of failure

❯ pet edit
The argument 'nvim-qt +0 C:\Users\chris\AppData\Roaming\pet\snippet.toml' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

Usage: pwsh[.exe] [-Login] [[-File] <filePath> [args]]
                  [-Command { - | <script-block> [-args <arg-array>]
                                | <string> [<CommandParameters>] } ]
                  [-CommandWithArgs <string> [<CommandParameters>]
                  [-ConfigurationName <string>] [-ConfigurationFile <filePath>]
                  [-CustomPipeName <string>] [-EncodedCommand <Base64EncodedCommand>]
                  [-ExecutionPolicy <ExecutionPolicy>] [-InputFormat {Text | XML}]
                  [-Interactive] [-MTA] [-NoExit] [-NoLogo] [-NonInteractive] [-NoProfile]
                  [-NoProfileLoadTime] [-OutputFormat {Text | XML}]
                  [-SettingsFile <filePath>] [-SSHServerMode] [-STA]
                  [-Version] [-WindowStyle <style>]
                  [-WorkingDirectory <directoryPath>]

       pwsh[.exe] -h | -Help | -? | /?

PowerShell Online Help https://aka.ms/powershell-docs

All parameters are case-insensitive.
exit status 64

Command would need to be

pwsh -Command "nvim-qt +0 C:\Users\chris\AppData\Roaming\pet\snippet.toml"

like CMD /C is used https://github.com/knqyf263/pet/blob/f88549cbb1e7eb4853bf7929b659e083076bfc92/cmd/util_windows.go#L21-L22

So possibly some switch statement to do the right thing when pwsh is set in config. Possibly, detecting the Shell in which pet is called could help know if CMD or PWSH should be used. Which would probably be the clever thing to do so that pet exec can do the right thing depending on which shell pet is used.

Hope this helps