Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds JSON output support to the dump commands (dump-anlz, dump-pdb, and dump-setting) in the rekordcrate CLI tool. The changes enable users to serialize parsed data structures to JSON format for easier inspection and processing with tools like jq.
Changes:
- Added
--formatflag (-f) to dump commands supporting bothDebug(default) andJsonoutput formats - Implemented
Serializederives across data structures insetting.rs,anlz.rs,pdb/mod.rs,pdb/ext.rs,pdb/string.rs,pdb/offset_array.rs,pdb/bitfields.rs, andutil.rs - Created custom serialization helpers:
serialize_as_hexfor byte slices,serialize_null_stringfor NullString, andserialize_null_wide_stringfor NullWideString - Added
serde_jsonandfaster-hexdependencies to Cargo.toml
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Added DumpFormat enum and format argument handling for all dump commands, with JSON/Debug output selection |
| src/util.rs | Added serialize_as_hex helper function and Serialize derives for ColorIndex and FileType |
| src/setting.rs | Added Serialize derives to all setting structs and enums, with custom serialization for NullString fields and hex formatting for byte arrays |
| src/anlz.rs | Added Serialize derives to all analysis structs and enums, with custom serialization for wide strings and bitfield types |
| src/pdb/mod.rs | Added Serialize derives to PDB data structures including headers, pages, tables, rows, and ID types |
| src/pdb/ext.rs | Added Serialize derives to extended database types (tags, categories, track tags) |
| src/pdb/string.rs | Implemented custom Serialize for DeviceSQLString type |
| src/pdb/offset_array.rs | Added Serialize derives and custom implementation for OffsetArray |
| src/pdb/bitfields.rs | Implemented custom Serialize for PackedRowCounts bitfield |
| Cargo.toml | Added serde_json v1.0 and faster-hex v0.9 dependencies |
| Cargo.lock | Updated with new dependency entries |
| README.md | Updated documentation to describe JSON format option with usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[derive(Debug, PartialEq, Eq, Serialize)] | ||
| #[br(import(header: Header))] | ||
| pub struct Unknown { | ||
| /// Unknown header data. |
There was a problem hiding this comment.
The header_data and content_data fields in the Unknown struct should use serialize_as_hex to be consistent with the PR's approach of serializing byte slices as hex strings. Currently, they will serialize as arrays of numbers, which contradicts the goal stated in the PR description of making output easier to read.
| DumpFormat::Json => println!( | ||
| "{}", | ||
| serde_json::to_string_pretty(&anlz).expect("failed to serialize ANLZ") | ||
| ), |
There was a problem hiding this comment.
Using expect() to panic on serialization failure prevents proper error handling. Consider using ? operator or map_err() to convert the serde error to a RekordcrateError for consistent error propagation.
| DumpFormat::Json => println!( | |
| "{}", | |
| serde_json::to_string_pretty(&anlz).expect("failed to serialize ANLZ") | |
| ), | |
| DumpFormat::Json => { | |
| let json = serde_json::to_string_pretty(&anlz)?; | |
| println!("{}", json); | |
| } |
| println!( | ||
| "{}", | ||
| serde_json::to_string_pretty(&dump).expect("failed to serialize PDB output") | ||
| ); |
There was a problem hiding this comment.
Using expect() to panic on serialization failure prevents proper error handling. Consider using ? operator or map_err() to convert the serde error to a RekordcrateError for consistent error propagation.
| println!( | |
| "{}", | |
| serde_json::to_string_pretty(&dump).expect("failed to serialize PDB output") | |
| ); | |
| let json = serde_json::to_string_pretty(&dump)?; | |
| println!("{json}"); |
| DumpFormat::Json => println!( | ||
| "{}", | ||
| serde_json::to_string_pretty(&setting).expect("failed to serialize Setting") | ||
| ), |
There was a problem hiding this comment.
Using expect() to panic on serialization failure prevents proper error handling. Consider using ? operator or map_err() to convert the serde error to a RekordcrateError for consistent error propagation.
| DumpFormat::Json => println!( | |
| "{}", | |
| serde_json::to_string_pretty(&setting).expect("failed to serialize Setting") | |
| ), | |
| DumpFormat::Json => println!("{}", serde_json::to_string_pretty(&setting)?), |
| database exports (i.e. `PIONEER/rekordbox/export.pdb` files): | ||
| The included command line tool `rekordcrate` can be used to inspect various files. | ||
|
|
||
| By default, it will output `Debug` trait output. pass `--format json` (or `-f json`) to any `dump-*` command to get JSON output instead. |
There was a problem hiding this comment.
The sentence should start with a capital letter: "Pass" instead of "pass".
| By default, it will output `Debug` trait output. pass `--format json` (or `-f json`) to any `dump-*` command to get JSON output instead. | |
| By default, it will output `Debug` trait output. Pass `--format json` (or `-f json`) to any `dump-*` command to get JSON output instead. |
| serializer.serialize_str(&nws.to_string()) | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line should be removed for consistency with the rest of the codebase.
| } | ||
|
|
||
| impl ANLZ { | ||
| // TODO - this doesn't seem to work or be used ? |
There was a problem hiding this comment.
This TODO comment appears to be unrelated to the JSON serialization changes in this PR. If this is a genuine concern about the parse_sections function, it should be addressed in a separate issue or PR. If it's not a real issue, the comment should be removed to avoid confusion.
| // TODO - this doesn't seem to work or be used ? |
when using the
dump-*commands to inspect files, i found theDebugtrait output a bit difficult to work with, so this adds JSON support to make it easier to read & inspect with tools likejq.this is a bit opinionated in serializing byte slices as hex strings rather then arrays of
Number, but i think this makes the output significantly easier to read & browse though.some manual implementations of
Serialize, like forWaveformPreviewColumn, are because the default serialization resulted in a hard to read format.as an example, a settings dump in json format:
{ "brand": "PIONEER", "software": "rekordbox", "version": "0.001", "data": { "MySetting": { "unknown1": "0x7856341202000000", "on_air_display": "On", "lcd_brightness": "Two", "quantize": "On", "auto_cue_level": "Memory", "language": "English", "unknown2": 1, "jog_ring_brightness": "Bright", "jog_ring_indicator": "On", "slip_flashing": "On", "unknown3": "0x010101", "disc_slot_illumination": "Bright", "eject_lock": "Unlock", "sync": "Off", "play_mode": "Continue", "quantize_beat_value": "FullBeat", "hotcue_autoload": "On", "hotcue_color": "Off", "unknown4": 0, "needle_lock": "Lock", "unknown5": 0, "time_mode": "Remain", "jog_mode": "Vinyl", "auto_cue": "Off", "master_tempo": "On", "tempo_range": "TenPercent", "phase_meter": "Type2", "unknown6": 0 } } }