WillPower3309 / awesome-dotfiles

Dotfiles for awesome people using the awesomewm linux environment
1.17k stars 68 forks source link

[Feature Request] Some scripts are incompatible with the fish shell #33

Open e-sam opened 3 years ago

e-sam commented 3 years ago

Summary

As we have discovered in #31, some of the scripts are not working, if the default shell is fish. Unfortunately, this also affects the network widget and possibly other areas, that I haven't noticed yet. Especially, in the network widget, the differences between fish and bash are so big, that I doubt, it will be possible to write a version, that runs in both shells. Therefore my proposal would be, to have two versions of the script and use the SHELL environment variable, to choose the script corresponding to the users login shell.

Example

An example of the big differences between bash and fish I mentioned earlier:

# Bash
if [[ -n "${$wireless}" && -d "${net}${wireless}" ]]; 
then
  ...
fi

# Fish
if test -n "$wireless" && test -d "$net$wireless"
  ...
end

Points to discuss

Once we have come to a conclusion about the above points, I am willing to create a pull request, containing the network manager and everything else, I discover until then, in a version working with fish.

WillPower3309 commented 3 years ago

Thank you for the generous offer and detailed write up! I'm going to spend some time this evening researching this to see if there's a shell agnostic way to achieve the functionality we want here, and come up with an implementation plan accordingly, I'll keep this issue updated on the progress.

e-sam commented 3 years ago

I think I have found a pretty neat solution. We could pipe the scripts to bash, this would mean you need to have bash installed to make the scripts work, but it doesn't have to be your default shell.

Examples:

echo 'pgrep -u $USER -x %s > /dev/null || (%s)' | bash -
echo 'a longer script' | bash -
WillPower3309 commented 3 years ago

I think I have found a pretty neat solution. We could pipe the scripts to bash, this would mean you need to have bash installed to make the scripts work, but it doesn't have to be your default shell.

Examples:

echo 'pgrep -u $USER -x %s > /dev/null || (%s)' | bash -
echo 'a longer script' | bash -

Thats a super elegant solution! I was having a hard time finding a fix so thank you for the idea, I think that is the route we should go down to fix it. I'll be implementing this tonight

e-sam commented 3 years ago

Great! I'm glad I could help. Thanks again for the nice themes

WillPower3309 commented 3 years ago

commit 02c706b fixes the startup issues, however fixing the network widget will be more time intensive. It's a mess of lua string formatting and single and double quotes clashing when trying to add echo ' command ', so I will be trying to tackle that beast over the next few days. Pull requests are welcome if anyone can finish before then but for now the startup behavior is fixed

WillPower3309 commented 3 years ago

Got a very important interview late next week so ill be putting this off until after then, apologies to anyone waiting. The startup is working now, just need to finish the network widget

aarondill commented 10 months ago

@WillPower3309 if you're still interested in fixing this, instead of piping commands to bash, you could set awful.util.shell to bash, which would force the awful.spawn.*_with_shell functions to spawn a bash shell instead of fish, avoiding the requirement to manually pipe to bash each time (and the interim fish shell), as well as making quoting easier.

WillPower3309 commented 10 months ago

@WillPower3309 if you're still interested in fixing this, instead of piping commands to bash, you could set awful.util.shell to bash, which would force the awful.spawn.*_with_shell functions to spawn a bash shell instead of fish, avoiding the requirement to manually pipe to bash each time (and the interim fish shell), as well as making quoting easier.

That's great to know, thanks! I'll see if I can get that fixed up