linrunner / TLP

TLP - Optimize Linux Laptop Battery Life
https://linrunner.de/tlp
GNU General Public License v2.0
2.51k stars 129 forks source link

New output flags for tlp-stat to improve usability with other scripts #729

Closed benjamin-voisin closed 3 months ago

benjamin-voisin commented 4 months ago

Add 3 new flags to tlp-stat : -m, -q and --version. The main goal being to facilitate the usage of tlp with other script, by making easier to parse outputs. All already existing outputs are unchanged, there are no retrocompatibility issues.

New outputs

Docs

The manpage and the help output for tlp-stat are updated accordingly to what is described above.

linrunner commented 3 months ago

Hi, I was tempted to merge this immediately. But then I noticed that in many places the indentations have slipped and only 2 blanks are used. TLP's standard is 4 blanks per level. Please correct this.

If you like, you can also update the auto-completion:

The main goal being to facilitate the usage of tlp with other script, by making easier to parse outputs.

Do you realize that the formatting can change at any time and your scraping will no longer work? I'm also curious: what are these "other scripts" - what is your use case?

benjamin-voisin commented 3 months ago

I have updated the auto-completion, and fixed the indentation to 4 spaces.

Do you realize that the formatting can change at any time and your scraping will no longer work?

That is why I think an easy way to get the version is useful, and it will always be much more futurproof than trying to parse the output with grep and sed.

I'm also curious: what are these "other scripts" - what is your use case?

I'm making a waybar module to see the current tlp status, and to be able to change

linrunner commented 3 months ago

I notice that -s -q and -m produce the same output, i.e. they are redundant. I will remove -m if you agree.

https://github.com/linrunner/TLP/tree/pr/729

benjamin-voisin commented 3 months ago

I find it kinda weird that the -s flag output two informations : system infos and status infos. To my, It would be preferable to remove status infos from -s and put it only in the -m flag. My idea is :

The main intention of my PR is to access easily the current mode (-m -q) and the version (--version). Maybe we remove the -m flag, and make -s -q output just the current status, but it feels weird to me (the fact that -s output system and status infos feels weird). I did not change -s by fear that it might break retrocompatibility, but ideally I think -s should only output system infos, and -m status infos. What do you think ?

linrunner commented 3 months ago

The scope of -s (System and Status) has been stable for a long time, I don't want to change it. I wouldn't call it "weird" but "legacy".

The main intention of my PR is to access easily the current mode and the version.

Then my suggestion is to implement just this:

benjamin-voisin commented 3 months ago

Works good for me :+1:

linrunner commented 3 months ago

I have implemented my suggestions in my branch https://github.com/linrunner/TLP/tree/pr/729. Beyond -q no longer suppresses the entire output, but only the corresponding parts of cpu and temperatures.

Please check if this still meets your requirements.

benjamin-voisin commented 3 months ago

I'v tried it and it good for me, thanks !

linrunner commented 3 months ago

Further fixes and additions were necessary -> https://github.com/linrunner/TLP/commits/pr/729/ The whole thing is merged to main now.

Thank you very much for your contribution.