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.

Sign up for GitHub

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

Jump to bottom

Fix #243 Added module file for text_styling in utils #254

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix #243 Added module file for text_styling in utils #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae8838d
Updated token commands in doc
ShubhamPandey28 Jan 2, 2020
d98c71c
Added pipfiles in made while using pipenv
ShubhamPandey28 Jan 11, 2020
d0fa88f
Added text_styling.py in utils
ShubhamPandey28 Jan 11, 2020
edc766a
Fixed flake8 build error in text_styling.py
ShubhamPandey28 Jan 11, 2020
9da07f3
Fixed flake8 build error in text_styling.py
ShubhamPandey28 Jan 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ venv/
ENV/
env3/

# pipenv
Pipfile
Pipfile.lock

# Spyder project settings
.spyderproject
.spyproject
Expand Down
4 changes: 2 additions & 2 deletions docs/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ <h4><b>Configure your Auth Token</b></h4>
<div class="command">
<div class="label">Run this command</div>
<div class="text">
<span id="setToken"> evalai set_token &lt;auth_token&gt; </span>
<span id="setToken"> evalai set-token &lt;auth_token&gt; </span>
<a class="copy-btn" onclick="CopyToClipboard('setToken')" data-toggle="tooltip" data-placement="top" title="copy to clipboard"><i class="fas fa-copy"></i>
</a>
</div>
Expand All @@ -97,7 +97,7 @@ <h4><b>View your Auth Token</b></h4>
<div class="command">
<div class="label">Run this command</div>
<div class="text">
<span id="getToken">evalai get_token</span>
<span id="getToken">evalai get-token</span>
<a class="copy-btn" onclick="CopyToClipboard('getToken')" data-toggle="tooltip" data-placement="top" title="copy to clipboard"><i class="fas fa-copy"></i>
</a>
</div>
Expand Down
27 changes: 27 additions & 0 deletions evalai/utils/text_styling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

from click import echo, style


def print_message(message, error=False):

style_message = ""

index = 0

for i in list(message.split("`")):

if index % 2:
style_message += style(i, fg="cyan")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice logic here. A small query here: what do you think about adding a check to see if the text is actually enclosed in backticks. That is, to check if there is a closing backtick and only applying this after that.
I'm not sure if it is easy to do here, but if the parts of the messages were in a list ls:

if len(ls) == ODD:
  # apply command style

Let me know what you think.

Another small query: I think we can add bold=True here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya sure. both of them are doable and looks nice. I will follow it in the following PR

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I will do that in following PR


elif error:
style_message += style(i, fg="red", bold=True)

else:
style_message += style(i, fg="green")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You used the error flag here to either print the error message or the success message. But there could be times when we just want to print the info with white bold text or just plain white text. Can you add that functionality here somehow? Feel free to use helper methods as mentioned in #234

Small query: bold=True?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will include it in following PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just followed the codes of the other files. most of the error messages where bold in those files so I kept it bold.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add an argument to the functions for bold text in the following PR


index += 1

echo(
style_message,
color=True
)