-
Notifications
You must be signed in to change notification settings - Fork 494
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
add sha1/sha256 to default renderer #164
Conversation
Having the sha1 & sha256 hashes by default would actually be pretty useful I think. This change would add those.
Ha, interesting. Initially, we've decided to minimize the output for the default mode and only include the other hash values in the more verbose outputs. Does using these modes work for you or do you consider these values to be crucial here? |
@@ -21,6 +21,8 @@ def width(s, character_count): | |||
def render_meta(doc, ostream): | |||
rows = [ | |||
(width("md5", 22), width(doc["meta"]["sample"]["md5"], 82)), | |||
(width("sha1", 22), width(doc["meta"]["sample"]["sha1"], 82)), | |||
(width("sha256", 22), width(doc["meta"]["sample"]["sha256"], 82)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only need to use width
on the first row, to pad out the column, but there's no way you would have known this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an artifact of us using tabulate
but not wanting to fork/extend it with layout options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mr-tz I would personally use the default view as general meta data for notes as well as other things, and we are just adding 2 rows to the output. However that is just my use case so I can fork it and add this if I want :) and others wont be affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I'm fine with the additional fields if you can update as per Willi's suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in 45fbd49
thanks @threathive ! |
Having the sha1 & sha256 hashes by default would actually be pretty useful I think. This change would add those.