-
Notifications
You must be signed in to change notification settings - Fork 513
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.
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
Fix cute battery by pulling out icon decisions #435
Conversation
I saw there were some linter errors, but most of them were caused by previously existing code so I'm not sure what to do about those. I did fix the couple I added. |
First of all, thank you for contributing to tmux-powerline 👍 Could you quickly describe the issue you're trying to solve? Will be easier for me to understand what you're trying to achieve 🙂 Regarding the linter errors you wrote:
The whole codebase is linted and there are just two possibilities for the linter to complain:
To ensure it is not 1., i let the workflow for the main branch run again, which reported back that everything is still fine. |
This is to fix the issue I described here cab0747#commitcomment-143145213. The changes in that commit broke the way the cute mode worked. That commit added
Looking at the linter logs, here is an example. https://github.com/erikw/tmux-powerline/actions/runs/9997569635/job/27639336899?pr=435 All that being said, I'm happy to fix any issues. I just find it weird that it's complaining about stuff I didn't even change, and especially that you have now said that you ran it on the main branch and it's fine. Let me know what you think! |
If you read the error of the linter, you'll actually understand why exactly the change you mentioned will bring up the error. The
Even though you don't introduced the subshell itself, you tried to use the var outside of the subshell (by moving the line out of the subshell) and that's what the linter is now complaining about, as you can not modify/create environment variables in a child process and have them be modified/created in the parent process as well. The solution provided by ShellCheck will remove the subshell and therefore you'll be able to use the variables as you're used to. The |
Fixed with #447 |
I don't have an osx or freebsd machine to test these changes on, but I think they will work. I've also added some code for battery info in WSL, which is the linux I'm using.
I'm open to feedback and fixes if those systems aren't working anymore.