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

extend unknown color error message #147

Merged
merged 2 commits into from
Aug 10, 2020
Merged

extend unknown color error message #147

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ade9817
extend unkown color error message
SnowMB Aug 4, 2020
cd212b8
make sure only the input and not the padded colors are given in the e…
SnowMB Aug 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/wireviz/wv_colors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def get_color_hex(input, pad=False):
try:
output = [_color_hex[input[i:i + 2]] for i in range(0, len(input), 2)]
except KeyError:
print("Unknown color specified")
print(f"Unknown color specified: {input}")
Copy link
Collaborator

@kvid kvid Aug 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Consider also something like print(f'Unknown color specified: {bytes(input, "utf-8")}') to catch any unprintable characters. Edit: (or white-space characters that need quotes to be seen)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that, but I'm not sure if I like the byte indicator around the color string. Unknown color specified: b'OGYW' But I will include it if you want 😄

Copy link
Collaborator

@kvid kvid Aug 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

  1. I agree, and it's easy to strip out the leading byte indicator by using str(bytes(input, "utf-8"))[1:] instead if that is the only thing you don't like.
  2. At the cost of an extra code line and an extra if-else, it is also possible (if you want, but not required IMHO - a matter of taste, I guess) to avoid the quotes when they are not strictly needed:
quoted = str(bytes(input, "utf-8"))[1:] # Skip byte indicator, but keep quotes
print(f'Unknown color specified: {input if input.strip() == quoted[1:-1] else quoted}')

However, I don't insist. It's just a suggestion in case someone find it useful. 😄

Copy link
Collaborator

@formatc1702 formatc1702 Aug 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe I'm out of the loop, but why don't we stick to the suggested

print(f'Unknown color specified: {input}')

?
Are we really worried people will try to use emojis and non-printable characters as colors? 😛

For stray whitespace characters,

print(f'Unknown color specified: "{input}"')

should be enough to catch them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any of them are OK with me. I just wanted to menton ideas that have been useful in other projects, but you decide what is needed in this project. My reply was not intended as a fight for my suggestion, but simply a correction of the draw-back that was commented.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, just trying to understand the motivation for the idea. I think for now it's safe to keep it as-is :)

output = [color_default]
return output

Expand Down