Skip to content
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

Another refactor #9

Merged
merged 9 commits into from
Sep 27, 2020
Merged

Another refactor #9

merged 9 commits into from
Sep 27, 2020

Conversation

zallison
Copy link
Contributor

I wasn't completely happy with my first refactor, it left too much chart stuff in disks.py and vice versa.

charts.py, disks.py, and just a bit in cli.py have been reworked. Overall it's a MUCH better structure. From here it should be easy to add any other charts, specifically whatever psutil can grab.

charts.py now has proper setters and getters

I'm torn on how to handle colors/attributes. Right now the setters for colors don't use the old enum structure and assumes the incoming is text and converts it via fg(color) which, of course, means only foreground colors work, but all 256 colors work. I was hoping I could check to see it was a string, but unfortunately fg(color) returns a string. I'm hoping someone has a better idea of how the color setters should/could be implemented to account for accepting plain text and converting it (e.g. from the command line) and existing formatting strings (e.g. bg(color) or attr(attribute))

The only other outstanding issue is that "details" doesn't seem to be getting passed on from the cli.

At the bottom of each module I added a if __name__ == "__main__" .... section that runs a quick sanity check. charts.py prints a test chart and disks.py prints charts of everything with details

So it's not "done" just yet, but it needs some more eyes on it before going into master. There are no other branches available though.

@bexxmodd bexxmodd merged commit 255be27 into bexxmodd:master Sep 27, 2020
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