ss7m / paleofetch

neofetch, but written in C
MIT License
167 stars 50 forks source link

Added Support for Multiple Batteries #63

Open srithon opened 4 years ago

srithon commented 4 years ago

I made each function take in a pointer to the label so that they can modify them. This was necessary so that each battery could be given the correct name. I went fairly in-depth in my issue. There is a little bit of buffer micromanagement, but it was a really easy optimization to make so I just went for it.

srithon commented 4 years ago

I like what you did there. The ability to modify the label is quite nice, altough it might be confusing for the user, if the label that is set in the config would just be ignored or changed.

I wasn't sure about what the best way to handle this was. One way to fix this is to treat the "name" label as a format string, and then use that format string with the model name as an argument to determine the final label. This addresses your next point. With this change, you would be able to have Battery: as your defined label, and have that as the final output in the end as well. If a user wanted to have the model name of the battery as the label, they could use %s: as their defined label in the config. Since the format string is determined at compile-time, it would not pose a vulnerability.

Does this sound like a good idea?

In my opinion, it would be good, if there was a possibility to deactivate the usage of the model name as the label. I only have a single battery to show and always seeing the part number of this battery in paleofetch is not so nice for me.

See the above

I think in general, having the ability to pass function parameters in the config would be a big improvement. This would remove the duplicate functions for n-th device like for battery or gpu.

That's a great idea, and it would definitely make the code a lot less redundant. For now, we only need one argument max per function, so we would be able to get away with just adding a 4th "param" field to the config struct. However, we may need a better solution than this if we end up having more than 1 argument per function.

srithon commented 4 years ago

I went ahead and implemented the format string approach.