polybar / polybar-scripts

This is a community project. We write and collect scripts for polybar!
The Unlicense
2.46k stars 338 forks source link

Make shell scripts executable #400

Closed iptq closed 1 year ago

iptq commented 1 year ago

While packaging for Nix, I discovered the majority of these scripts aren't actually executable. This patch remedies that.

x70b1 commented 1 year ago

Thanks for this PR.

There are multiple attempts to package polybar-scripts at various distributions. I maintain none of them.

To be honest I am a bit against the idea that things in this repo should be packaged. The point is that almost no script in this repo is ready to use. The most of them need configurations or icon replacements.

I have this on the main README.md:

This repository is not an exact blueprint. I guess every script has to be customized to make your Polybar unique.

There was already the idea to make everything configurable with env vars or config files. But this is to much work for the most scripts that only contain some simple lines.

So my idea is that this repo is more a place to grab ideas, keep what you need and change what you like to.

Did I miss something?

iptq commented 1 year ago

The executable problem is orthogonal to the packaging situation; polybar will not be able to execute scripts provided directly by path if their executable bits are not set, i.e

image

This patch simply makes files executable, not adding any sort of packaging of any kind.


On the topic of customization, I disagree that projects should expect users to copy-paste lines out of them into their own configurations, rather than allowing customization via environment variables (which is easier than config files imo).

Suppose the openweather script example above uses an API that changes or needs to be updated with more information. When this repository is updated, I will have to continuously port changes over, checking that they don't clash with my own customizations. This will be required of everyone who wants to customize the behavior of their script.

On the other hand, packaging allows me to update my dependency on this package along with my operating system's other dependencies, and automatically receive updates regularly.

Again, this is just a tangent, and not related to the executable patch, but there are some further changes I am already considering to make to the weather script to allow city and units to be configured through environment variables that will certainly benefit others if I am able to contribute it back.

x70b1 commented 1 year ago

polybar will not be able to execute scripts provided directly by path if their executable bits are not set

Even if you could (you can use a shell to run them). You have to change settings and icons in the scripts to make them work. They are not build to use them as they are. Changes are required.

When this repository is updated, I will have to continuously port changes over, checking that they don't clash with my own customizations.

Thats how it is. I am not saying that this is perfect. But its a simple solution that we have.

The idea to introduce env vars is not new. The point is: Where should we stop? I have already seen so much script customizations that you always will find someone who says: I can't configure everything I want. And thats ok. So we don't try it.

Look at one of the most used scripts: updates-pacman-aurhelper Thats are 10 lines of code if you remove comments and spaces. If you now add config options like icons, colors, output styles you would increase the script size to a multiple count of lines. Is it worth it? For me not. Keep the things simple so that everyone can read them without deep knowledge of shell scripts. Everything else can build on top of this on the users machine.

iptq commented 1 year ago

Unfortunate. Thank you for your time