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

Reformatted codebase #325

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Reformatted codebase #325

merged 1 commit into from
Dec 22, 2021

Conversation

varaki
Copy link
Contributor

@varaki varaki commented Dec 21, 2021

All python scripts are now reformatted with 'black' using line length
of 100.

All python scripts are now reformatted with 'black' using line length
of 100.
@AdnanHodzic
Copy link
Owner

Thanks for your contribution!

I'm curious how and why did you settle on 100 instead of i.e: 80?

@varaki
Copy link
Contributor Author

varaki commented Dec 21, 2021

Thanks for your contribution!

I'm curious how and why did you settle on 100 instead of i.e: 80?

80 is just too low, 100 is a better option I think, could be 160 as well, then we can say it's twice as much of what PEP-8 recommends.
After all we are not forced to use 80x25 terminals like in the '90s. 😃

@AdnanHodzic
Copy link
Owner

Makes sense and it looks good.

Was thinking if we're doing this with code as part of this PR I would also change the output to be 100 instead of 80 characters long, i.e:

------------------------------ Current CPU stats ------------------------------
------------------ Running auto-cpufreq daemon install script ------------------

all of these are <= 80 but then most of the space would be empty and it won't look good if terminal is scaled below 100 characters, so let's keep as it is unless you have a better idea.

@AdnanHodzic AdnanHodzic merged commit 3121e2f into AdnanHodzic:master Dec 22, 2021
@varaki
Copy link
Contributor Author

varaki commented Dec 22, 2021

Makes sense and it looks good.

Was thinking if we're doing this with code as part of this PR I would also change the output to be 100 instead of 80 characters long, i.e:

------------------------------ Current CPU stats ------------------------------
------------------ Running auto-cpufreq daemon install script ------------------

all of these are <= 80 but then most of the space would be empty and it won't look good if terminal is scaled below 100 characters, so let's keep as it is unless you have a better idea.

I think we can use a function, that dynamically prints out messages in the above format considering the terminal size.
I've quickly created an example, try it out, it works fine.
You can control the inner padding as well as the decor character(s) to use:

#!/usr/bin/env python3

import os

def dynamic_print(message, decor="-", padding=1):
    """
    Dinamically print messages according to available terminal width
    If message is equals to or longer than the terminal width, print the message as is

    Parameters:
        message (str): The message to print
        decor (str):   Decorator character(s)
        padding (int): Padding size (how many whitespaces between decor and message)

    Returns:
        str: Formatted message
    """
    term_width = os.get_terminal_size().columns
    message_size = len(message)

    if message_size >= term_width:
        print(message)
        return

    # calculate decor size per side
    decor_size = int((term_width - (message_size + (padding * 2))) / 2)
    decor = f"{int(decor_size / len(decor)) * decor}"
    pad = " " * padding

    # print the formatted message
    print(f"{decor}{pad}{message}{pad}{decor}")

if __name__ == "__main__":
    dynamic_print("Current CPU stats")
    dynamic_print("Running auto-cpufreq daemon install script")

I'll check if there's room for improvement for this, update the scripts to use this and I'll submit this as a separate PR.

@AdnanHodzic
Copy link
Owner

AdnanHodzic commented Dec 22, 2021

I think we can use a function, that dynamically prints out messages in the above format considering the terminal size.

This is exactly what I was thinking of and what would be ideal solution but didn't express it as an idea 🙂

However, code sample from above didn't dynamically scale anything, and - were printed out in multiple lines.

@varaki
Copy link
Contributor Author

varaki commented Dec 22, 2021

However, code sample from above didn't dynamically scale anything, and - were printed out in multiple lines.

What do you mean? I don't get it.

I tried it on my work laptop (gnome + gnome terminal), when I posted the example here and it worked.
Now I'm on my home laptop (swaywm + alacritty), also works fine.
Already printed out lines can't be scaled, so if you run the command in a larger sized terminal, then resize it to a smaller size, then the already printed out lines will be wrapped of course. But if you re-run the script after you resized your terminal text will be aligned to that size.

Here's a demo of the command (I only removed the second function call dynamic_print("Running auto-cpufreq daemon install script")) run with a oneliner which runs the example every second, for more info:
https://user-images.githubusercontent.com/23281360/147129736-511a3f2a-6002-41bc-b071-31f7ea338a2d.mp4

As you can see, printouts change with every update if you modify the terminal size.

Another demo with the example.py untouched:
https://user-images.githubusercontent.com/23281360/147131116-0d7475ef-0759-43df-b45f-c1827688f5e5.mp4
It also works here, maybe that it prints out two messages (each of them are decorated), that's what caused the confusion for you...
Maybe you thought that the second message's decorator characters are the continuation of the first one... :)

@varaki varaki deleted the codeformat branch December 23, 2021 09:35
@AdnanHodzic
Copy link
Owner

Already printed out lines can't be scaled, so if you run the command in a larger sized terminal, then resize it to a smaller size, then the already printed out lines will be wrapped of course.

This the perfect summary right here. Since auto-cpufreq output is re-outputted every 5 seconds I think it would be great if output could be scaled like this. If you have time to implement this as part of auto-cpufreq and create a PR I'll gladly review it!

@varaki
Copy link
Contributor Author

varaki commented Dec 23, 2021

Already printed out lines can't be scaled, so if you run the command in a larger sized terminal, then resize it to a smaller size, then the already printed out lines will be wrapped of course.

This the perfect summary right here. Since auto-cpufreq output is re-outputted every 5 seconds I think it would be great if output could be scaled like this. If you have time to implement this as part of auto-cpufreq and create a PR I'll gladly review it!

Sure, I'm planning to do that. 🙂

@AdnanHodzic
Copy link
Owner

Awesome, looking forward to that PR then 🙂

@AdnanHodzic
Copy link
Owner

Changes live as part of v1.9.0 release.

@AdnanHodzic AdnanHodzic mentioned this pull request Nov 25, 2022
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.

None yet

2 participants