Refactored how firecore tools handles printing
#69
Conversation
There is now a common flag for all `tools` subcommand `output` that can be one of: text, json, jsonl, protojson or protojsonl. Commands that had flags defined have been removed, this should be backward compatible. The text printer is now more clever and is able to print details about most blocks.
| if err != nil { | ||
| fmt.Printf("❌ Unable to print block %s: %s\n", block.AsRef(), err) | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| if printDetails == PrintFull { | ||
| printer, err := print2.GetOutputPrinter(globalToolsCheckCmd, chain.BlockFileDescriptor()) |
There was a problem hiding this comment.
I plan to remove all printing support from check.
| One of: text, json, jsonl, protojson, protojsonl | ||
| `)) | ||
|
|
||
| flags.String("bytes-encoding", "hex", "Encoding for bytes fields when printing in 'text', 'json' or 'jsonl' --output, either 'hex', 'base58' or 'base64'") |
There was a problem hiding this comment.
idea for this:
maybe LONG byte arrays should always be printed in hex, and this bytes-encoding flag should always be for byte arrays below 64 bytes or something...
One of the issue is that base58 is excruciatingly slow when dealing with long byte arrays
There was a problem hiding this comment.
Seems rather hard to understand if encoding switches based on length IMO. And since it's for printing, not sure it's a big thing with the performance, we are not in Solana ;)
|
|
||
| if printer == "text" { | ||
| // Supports the `transactions` flag defined on `firecore tools print` sub-command, | ||
| // we should move it to a proper `text` sub-option like `output-text-details` or something |
There was a problem hiding this comment.
maybe just a different output mode:
short
full
or summary vs fulltext
There was a problem hiding this comment.
Yeah let's discuss that part, I created a generic block printer using Golang protoreflect, we could maybe extend it!
| // Encode encodes the given bytes using the specified encoding. | ||
| func Encode(bytesEncoding string, bytes []byte) string { | ||
| switch { | ||
| case strings.EqualFold(bytesEncoding, "base58"): |
There was a problem hiding this comment.
here we could switch based on the length, as mentionned earlier.. just an idea but I think it would make sense
| @@ -73,9 +73,7 @@ func NewToolsCompareBlocksCmd[B firecore.Block](chain *firecore.Chain[B]) *cobra | |||
|
|
|||
| flags := cmd.PersistentFlags() | |||
| flags.Bool("diff", false, "When activated, difference is displayed for each block with a difference") | |||
| flags.String("bytes-encoding", "hex", "Encoding for bytes fields, either 'hex' or 'base58'") | |||
There was a problem hiding this comment.
Here you replaced with the global bytes-encoding that supports more values (like base64), but the implementation in tools_compare_blocks.go does NOT support anything else than hex or base58, watch out there needs to be a few changes later in this file, in func Compare(...)
There is now a common flag for all
toolssubcommandoutputthat can be one of: text, json, jsonl, protojson or protojsonl.Commands that had flags defined have been removed, this should be backward compatible.
The text printer is now more clever and is able to print details about most blocks.