jorgebucaran / fisher

A plugin manager for Fish
https://git.io/fisher
MIT License
7.53k stars 257 forks source link

fisher blocks GitHub Codespaces personalized dotfile setup #742

Open vorburger opened 1 year ago

vorburger commented 1 year ago

I attempted to invoke Fisher in a bootstrap.sh for a GitHub Codespaces personalized dotfile setup.

This didn't work and blocked the GitHub Codespaces setup. I have debugged it and realized it's because of this line:

isatty || read --local --null --array stdin && set --append argv $stdin

This blocks and waits for input because during a GitHub Codespaces setup STDIN is set to an pipe. (I do not know why GitHub does this; I'll post on https://github.com/orgs/community/discussions/35527 and see if GitHub wants to provide a reason for why they do this.)

This technically isn't GitHub Codespaces specific, but would similarly block other non-interactive uses of fisher anywhere where STDIN is blocking.

I can work around this using fisher </dev/null, so I just wanted to let you know about it FYI. It seems weird to have to do this. (And took me some time to figure out.)

https://github.com/vorburger/dotfiles-reproduce-problem illustrates this.

jorgebucaran commented 1 year ago

I'm happy to change this. I just need a way ti grab stdin from the user for things like piping plugins into Fisher install, update and remove commands.

vorburger commented 1 year ago

need a way ti grab stdin from the user for things like piping plugins into Fisher install, update and remove commands.

right so just to understand the idea here is that one could do something like fisher <plugins.txt, with a file containing lines such as install jorgebucaran/nvm.fish and remove ... etc. am I understanding this correctly? (BTW that feature is not actually documented on the README.)

How about perhaps making this something triggered by an explicit flag, instead of implicit automagic?

So you would have to e.g. fisher --STDIN (or fisher -- or whatever). That seems better (to me).

jorgebucaran commented 1 year ago

That's correct. Introducing a flag would be a breaking change, though. I'm not really opposed to it, but could this be worked around on your end?

jorgebucaran commented 1 year ago

Otherwise,fisher -- could do it too. 😁

vorburger commented 1 year ago

could this be worked around on your end?

GitHub Codespaces specific, but would similarly block other non-interactive uses of fisher anywhere where STDIN is blocking.

I can work around this problem by using fisher install jorgebucaran/nvm.fish </dev/null in a GitHub Codespaces personalized dotfile setup, and already have. Just filed this issue as a suggestion, to help others avoid running into this, because it seems weird to have to do this. (And took me some time to figure out.)

Otherwise,fisher -- could do it too. grin

I asked a friend of mine about this and he suggested if you don't like --STDIN as the name of an explicit flag to "read commands from STDIN" it would typically be named - (apparently this is a "common convention" for "read from STDIN instead of from a file name) but not -- (which typically means "stop processing options and take what follows as literal arguments").

jorgebucaran commented 1 year ago

t would typically be named - (apparently this is a "common convention" for "read from STDIN instead of from a file name) but not -- (which typically means "stop processing options and take what follows as literal arguments").

Yes, indeed. Your friend is absolutely correct. 👌

jorgebucaran commented 1 year ago

We might need a breaking change to fix this. I like using Fisher's feature for testing and debugging, like running fisher ls | fisher remove. It's great for creating pipelines, but I'm not sure how popular it is. Not keen on breaking changes now, but maybe we can search Fish dotfiles using this Fisher feature and start sending out PRs. No matter what, we can't change this until Fisher 5.

Oh, and just a heads up: this feature is mentioned in https://github.com/jorgebucaran/fisher#removing-plugins, so feel free to check it out if you're interested!

keslerm commented 10 months ago

Hit this myself while trying to install/setup fisher via ansible.

jorgebucaran commented 10 months ago

Thanks, @keslerm. I've got this squarely on my radar. 👍