Skip to content

Ensure we execute shell in bash#5

Open
aburd wants to merge 1 commit intodenisoster:masterfrom
aburd:master
Open

Ensure we execute shell in bash#5
aburd wants to merge 1 commit intodenisoster:masterfrom
aburd:master

Conversation

@aburd
Copy link

@aburd aburd commented Aug 2, 2022

Description

master branch breaks if the user is using something like fish
This PR will ensure that all cmds are made using bash

Other information

Recommendations from awesomewm
https://awesomewm.org/apidoc/libraries/awful.spawn.html#awful.spawn.with_shell

@G2G2G2G
Copy link

G2G2G2G commented Oct 11, 2022

works fine with fish, the commands are nothing special

scrot /home/user/Pictures/$(date +%F_%T).png -e 'xclip -selection c -t image/png < $f'

@aburd
Copy link
Author

aburd commented Oct 16, 2022

@G2G2G2G
I'm sorry, are you saying that scrot works fine with fish, or this plugin does?
Because I was referring to the awesomewm-screenshot plugin code as not working with fish.

@G2G2G2G
Copy link

G2G2G2G commented Oct 16, 2022

the plugin.. what I pasted is the code that this plugin runs. both work fine.
They are the same thing, if one works the other works.

@aburd
Copy link
Author

aburd commented Oct 17, 2022

@G2G2G2G
Hi thanks for your feedback.
I was using fish, and this plugin did not work for me until I introduced the changes in this PR.

Your own code example does not work for me in fish, just tested right now.

⋊> scrot /home/user/Pictures/$(date +%F_%T).png -e 'xclip -selection c -t image/png < $f'                                                                        
fish: $(...) is not supported. In fish, please use '(date)'.
scrot /home/user/Pictures/$(date +%F_%T).png -e 'xclip -selection c -t image/png < $f'
                          ^

If you are confident that this works, and it is a "just your PC" type problem, then feel free to close this.
But don't you think it would be best to execute these commands in the same shell program to made the code a bit more predictable?

@G2G2G2G
Copy link

G2G2G2G commented Oct 17, 2022

I'm not the maintainer, just some other guy that uses fish and it works fine.

This project is seemingly dead, your PR seems fine as it'll work on any system, but was just letting you know it does work on fish lol
maybe yours is an old version and whatever bug fish had, isn't fixed yet in yours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants