NayamAmarshe / please

🙏 Please CLI - Minimalistic New Tab Page CLI Tool with a greeting, date and time, inspirational quotes and your personal tasks and to-do list
MIT License
582 stars 26 forks source link

fix: make line render properly #26

Closed ChemDevv closed 2 years ago

ChemDevv commented 2 years ago

This pull request was made in response to an issue where the line would not print with the user's name properly

Previously, like the OP's terminal, the line would print very shortly without the user's name. These changes fixed that issue locally, and now the line properly renders every time - please try running poetry build and installing the whl file to test.

I believe console.rule is a better function to use for printing that line rather than nesting it in centerprint; it reduces complexity and returns the same result.

NayamAmarshe commented 2 years ago

So the issue is in Rich's Rule module, right?

ChemDevv commented 2 years ago

So the issue is in Rich's Rule module, right?

Yeah, I think so. I think the way it is in main like this:

center_print(Rule(date_text, style="#FFBF00"))

where the Rule function was being called as an argument for the center_print function was causing weird errors and issues, resulting in the mentioned issue where sometimes the user's name wasn't printed with a very short line.

Changing that line of the code to

console.rule(date_text, align="center", style="#FFBF00")

is a lot easier to maintain, and it returns expected behavior. There really isn't a need to call center_print on the user's name when the console.rule module does the same thing by calling one function.

After I built and tested the changes, I no longer had the same errors described in the linked issue.

ChemDevv commented 2 years ago

I also changed the logic of checking if the user wants that line from

try:
            if config["disable_line"]:
                center_print(date_text)
            else:
                pass
        except:
            center_print(Rule(date_text, style="#FFBF00"))

to

if "disable_line" in config.keys() and config["disable_line"] == True:
            center_print(date_text)
        else:
            console.rule(date_text, align="center", style="#FFBF00")

The logic is the same as before, but it simplifies the code. Checking to see if the user does not have the disable option in their config file by looking for a key error is replaced by the else statement.

NayamAmarshe commented 2 years ago

I also changed the logic of checking if the user wants that line from

try:
            if config["disable_line"]:
                center_print(date_text)
            else:
                pass
        except:
            center_print(Rule(date_text, style="#FFBF00"))

to

if "disable_line" in config.keys() and config["disable_line"] == True:
            center_print(date_text)
        else:
            console.rule(date_text, align="center", style="#FFBF00")

The logic is the same as before, but it simplifies the code. Checking to see if the user does not have the disable option in their config file by looking for a key error is replaced by the else statement.

Yours is definitely better. I think we'll have to change all the instances of try except with this method.