Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
e-sam opened this issue Jan 1, 2021 · 8 comments
Open

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

e-sam opened this issue Jan 1, 2021 · 8 comments
Labels
enhancement New feature or request

Comments

@e-sam
Copy link

e-sam commented Jan 1, 2021 •
edited
Loading

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

  • Is there really no way to write a script that runs in both shells?
  • Should we try, to auto detect the users current shell, e.g. by using the SHELL environment variable, or rather have a variable, in rc.lua, where the user can change the shell manually?
  • Should we keep the scripts inside the lua files, or move them to dedicated shell files, to make things a bit cleaner?
    • Downside of this could be, that you have to make the scripts executable after cloning the repository

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
Copy link
Owner

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
Copy link
Author

e-sam commented Jan 2, 2021 •
edited
Loading

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
Copy link
Owner

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

@WillPower3309 WillPower3309 added the enhancement New feature or request label Jan 2, 2021
@e-sam
Copy link
Author

e-sam commented Jan 2, 2021

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

@WillPower3309
Copy link
Owner

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
Copy link
Owner

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
Copy link

@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
Copy link
Owner

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@WillPower3309 @e-sam @aarondill