-
Notifications
You must be signed in to change notification settings - Fork 652
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
Issue-2748 - Add ByteBuffer Hex init & write #2837
Issue-2748 - Add ByteBuffer Hex init & write #2837
Conversation
28cf942
to
4621870
Compare
2a7cede
to
3e2b725
Compare
fde9321
to
e1fb499
Compare
@weissi please take a look when you find the time |
1e60ede
to
1d2ba6b
Compare
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.
Ok awesome, this is getting a lot closer. Only a few little notes here.
Sources/NIOCore/ByteBuffer-hex.swift
Outdated
public struct HexDecodingError: Error { | ||
let kind: HexDecodingErrorKind | ||
|
||
public enum HexDecodingErrorKind { |
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.
This enum
needs to be private, or we can't safely use this type.
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.
Are you suggesting internal? Private would mean this would be impossible (to my understanding) to initialise (atleast the way I have done so).
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.
Private should be fine, you'd use public static let
s on the parent to access the fields:
public struct HexDecodingError: Error {
private let kind: Kind
private enum Kind {
case invalidHexLength
case invalidCharacter
}
public static let invalidHexLength = HexDecodingError(kind: .invalidHexLength)
public static let invalidCharacter = HexDecodingError(kind: .invalidCharacter)
}
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.
thanks will find time to implement.
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.
Made HexDecodingError equatable. Makes sense to be able to see what kind of error you have and if it is not by making kind public, then I think the alternative is the equatable comformance. It also helps with tests. LMK what you think.
70052c4
to
eea3a60
Compare
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.
Fantastic, this is looking really good. A pair of very minor textual tweaks and then I'm happy. Thanks for sticking with this!
Doing a bit of travelling. Actually flying to London for the swift conference. Will find time to push the changes. |
9e08a5b
to
8e1244d
Compare
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.
Awesome, I'm happy with this! Well done getting through the gauntlet again @ali-ahsan-ali, it's really good that you were able to stick with it. Much appreciated ✨ .
Add ability for below two functions
ByteBuffer(hexEncodedBytes: "68 65 6c 6c 6f 20 77 6f 72 6c 64 0a")
buffer.writeHexEncodedBytes("68656c6c6f20776f726c640a")
Motivation:
#2748