Conversation
This allows multibase codes to be specified by the name rather than just the prefix char.
ghost
assigned
kevina
force-pushed
the
kevina/cid-fmt-enhan
branch
from
6b1990b to
c4bfcd0
Compare
|
Note: It adds a new However, I am also thinking that this Thoughts? |
|
Edit: No longer necessary. |
|
Edit: Not really a requirement. |
|
With the new |
|
@Stebalien I would like to get this in about the same time we get #53 in. |
|
|
||
| // Extract the encoding from a Cid. If Decode on the same string did | ||
| // not return an error neither will this function. | ||
| func ExtractEncoding(v string) (mbase.Encoding, error) { |
There was a problem hiding this comment.
What about providing a "decode" function that also returns the encoding (e.g., DecodeWithEncoding)? Decode could then be modified to just call that internally.
(not a strong opinion, I just feel it would be slightly more useful/efficient as one usually wants to do both at the same time)
There was a problem hiding this comment.
I did that at first. The ExtractEncoding is useful when you just want the encoding and it is fairly efferent and should not allocate any heap memory so the cost of the extra function call should be negligible.
|
|
||
| // Format formats a cid according to the format specificer as | ||
| // documented in the FormatRef constant | ||
| func Format(fmtStr string, base mb.Encoding, cid *Cid) (string, error) { |
There was a problem hiding this comment.
Thoughts on mirroring (mostly) fmt?
func Fprint(w io.Writer, base mb.Encoding, cid *Cid)
func Fprintln(w io.Writer, base mb.Encoding, cid *Cid) (int, error)
func Fprintf(w io.Writer, format string, base mb.Encoding, cid *Cid) (int, error)
func Print(base mb.Encoding, cid *Cid) (int, error)
func Println(base mb.Encoding, cid *Cid) (int, error)
func Printf(format string, base mb.Encoding, cid *Cid) (int, error)
func Sprint(base mb.Encoding, cid *Cid) (string, error) // should this return an error?
func Sprintln(base mb.Encoding, cid *Cid) (string, error)
func Sprintf(format string, base mb.Encoding, cid *Cid) (string, error)This may not really be worth it. We can always add that later.
There was a problem hiding this comment.
I think that is an overkill. :) A better model might be the time package. I could make Format a method, but I also think something like this should not be port of the core Cid functionally and possible, at a later date, moved to an external package.
| %h multihash name | ||
| %H multihash code | ||
| %L hash digest length | ||
| %m multihash encoded in base %b (with multibase prefix) |
There was a problem hiding this comment.
It would be kind of nice to support %(base)mwhere the (base) part is optional. That would, e.g., allow cid-fmt '%(base16)d' "$MYCID" to get a hex hash digest.
However, we can punt on that and figure it out later (just thought I'd note it down). We'd still want to be able to pass the default base to the format function (even if it's 0x0).
format.go
Outdated
| // documented in the FormatRef constant | ||
| func Format(fmtStr string, base mb.Encoding, cid *Cid) (string, error) { | ||
| p := cid.Prefix() | ||
| out := new(bytes.Buffer) |
There was a problem hiding this comment.
No need to allocate here. We can just put this on the stack. It even has a small internal 64byte buffer to avoid allocations when formatting small things.
| func hashToString(num uint64) string { | ||
| name, ok := mh.Codes[num] | ||
| if !ok { | ||
| return fmt.Sprintf("hash?%d", num) |
There was a problem hiding this comment.
Personally, for performance, I'd rewrite these helpers to write directly to the buffer (e.g., fmtHash(b bytes.Buffer, num uint64)). However, we can always revisit this if it becomes a performance issue (it won't be for the moment).
There was a problem hiding this comment.
In most cases a string from the look table is returned. Sprintf is only for the rare corner case when an encoding was used that is not in the table.
|
LGTM. I have a few small suggestions but I'd be happy merging as is. |
2 participants
Refactor and enhance cid-fmt and extract most of it from the main package so the functionality can be used as a library.
Closes #41.