Message digests #453
Message digests #453
Conversation
NeQuissimus
force-pushed
the
message_digests
branch
2 times, most recently
from
975f3d9 to
d3740c6
Compare
Codecov Report
@@ Coverage Diff @@
## master #453 +/- ##
==========================================
+ Coverage 95.7% 95.74% +0.04%
==========================================
Files 60 61 +1
Lines 675 682 +7
Branches 14 12 -2
==========================================
+ Hits 646 653 +7
Misses 29 29
Continue to review full report at Codecov.
|
|
@fthomas I don't understand that build error. I import |
|
Wow, this is strange. I've no idea what could be causing this. |
|
|
||
| /** Module for type representing message digests. */ | ||
| object digests { | ||
| import string.HexStringSpec |
There was a problem hiding this comment.
@NeQuissimus The build error on 2.10 can be fixed by moving this import to the top-level imports.
There was a problem hiding this comment.
I hadn't had time to look at this, thx :)
How did you figure that out? :D
There was a problem hiding this comment.
I moved and commented stuff out until it compiled. One of my special powers! ;-)
|
@fthomas Can you kick Travis once more? Things build with 2.10 locally just fine now... |
|
The build is green now. 👍 |
There was a problem hiding this comment.
@NeQuissimus Sorry it took me so long to review your PR.
I've some questions about HexStringSpec but the rest looks good to me. It would be also nice if you could add an entry to the release notes in notes/0.9.0.markdown.
| @@ -17,6 +17,11 @@ object string { | |||
| type TrimmedString = String Refined MatchesRegex[W.`"""^(?!\\s).*(?<!\\s)"""`.T] | |||
|
|
|||
| object TrimmedString extends RefinedTypeOps[TrimmedString, String] | |||
|
|
|||
| /** A `String` representing a hexadecimal number */ | |||
| type HexStringSpec = MatchesRegex[W.`"""[0-9a-f]+"""`.T] | |||
There was a problem hiding this comment.
@NeQuissimus Should we also allow upper case letters A-F? What about an optional 0x prefix? I think that a prefix would be fine for HexString but may be inappropriate for the message digests types. Maybe HexStringSpec should be private to digests to sidestep these potential conflicts?
There was a problem hiding this comment.
The prefix is not really much more than an artifact to identify a hexadecimal number to humans or when the information of it being a hex number is not available. I think that is exactly the information we are adding with the refined type.
I will make the change to allow for upper-case letters (but not mixed-upper/lower).
NeQuissimus
force-pushed
the
message_digests
branch
from
fdc5bca to
fd9b2f2
Compare
| @@ -17,6 +17,11 @@ object string { | |||
| type TrimmedString = String Refined MatchesRegex[W.`"""^(?!\\s).*(?<!\\s)"""`.T] | |||
|
|
|||
| object TrimmedString extends RefinedTypeOps[TrimmedString, String] | |||
|
|
|||
| /** A `String` representing a hexadecimal number */ | |||
| type HexStringSpec = MatchesRegex[W.`"""([0-9a-f]+)|([0-9A-F]+)"""`.T] | |||
There was a problem hiding this comment.
It seems that this does not work with Scala.js (there is a test failure), but fortunately you've experience with such problems. 😺
There was a problem hiding this comment.
Oh god... I shall stop using coreJVM/test and dive into regex' in JS once again :D
|
@fthomas Can you kick the build, some 503 thing just happened on Travis. |
HexStringto string typesMD5,SHA1,SHA224,SHA256,SHA384,SHA512