Chore: Importing from OneNote: Add debug tool for inspecting .one files#15084
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Inspect Binary
participant FS as File System
participant Parser as Parser
participant Out as Stdout/Stderr
User->>CLI: run inspect input.one [--onestore|--section]
CLI->>CLI: parse arguments -> Config
alt invalid config
CLI->>Out: print usage to stderr (exit 1)
else valid config
CLI->>FS: read file bytes
alt read error
CLI->>Out: print error + usage to stderr (exit 2)
else read success
CLI->>Out: print "Reading ..." to stderr
CLI->>Parser: parse_onestore_raw(data) or parse_section_from_data(data)
alt parse error
Parser-->>CLI: Err
CLI->>Out: print "Parse error: ..." to stderr (exit 3)
else parse success
Parser-->>CLI: OneStore or Section
CLI->>Out: print debug-formatted result (exit 0)
end
end
end
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/onenote-converter/README.md (1)
98-100: Tighten CLI examples: add fence languages and consistent run context.Both fenced blocks should declare
bash(MD040), and examples should use the same working-directory assumption to avoid ambiguity.Proposed README patch
-``` -bash$ cargo run -- ./test-data/ink.one --onestore -``` +```bash +cd parser/ +cargo run -- ./test-data/ink.one --onestore +``` -``` -bash$ cd parser/ -bash$ cargo run -- ./test-data/ink.one --section -``` +```bash +cd parser/ +cargo run -- ./test-data/ink.one --section +```Also applies to: 103-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/onenote-converter/README.md` around lines 98 - 100, Update the README.md CLI examples to use fenced code blocks with the bash language and make the working-directory consistent by adding an explicit cd parser/ before running cargo; specifically change the two examples that run cargo run ./test-data/ink.one --onestore and cargo run ./test-data/ink.one --section so each is wrapped in ```bash fences and includes the cd parser/ line prior to the cargo run command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/onenote-converter/parser/src/bin/inspect.rs`:
- Line 56: Update the CLI usage string so it lists all accepted flags
consistently: change the eprintln! call that currently prints "Usage:
{program_name} <input_file> [--section]" to include both "--onestore" and
"--section" (e.g., "[--onestore] [--section]") to match the argument parsing
logic that checks for the "--onestore" and "--section" flags; ensure the change
is made where the usage is printed (the eprintln! invocation) so users see the
correct flag options.
---
Nitpick comments:
In `@packages/onenote-converter/README.md`:
- Around line 98-100: Update the README.md CLI examples to use fenced code
blocks with the bash language and make the working-directory consistent by
adding an explicit cd parser/ before running cargo; specifically change the two
examples that run cargo run ./test-data/ink.one --onestore and cargo run
./test-data/ink.one --section so each is wrapped in ```bash fences and includes
the cd parser/ line prior to the cargo run command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 827744f8-f441-4455-bbd4-9ae037ade867
📒 Files selected for processing (12)
packages/onenote-converter/README.mdpackages/onenote-converter/parser-utils/src/debug.rspackages/onenote-converter/parser-utils/src/errors.rspackages/onenote-converter/parser-utils/src/lib.rspackages/onenote-converter/parser/Cargo.tomlpackages/onenote-converter/parser/src/bin/inspect.rspackages/onenote-converter/parser/src/local_onestore/objects/object_group_list.rspackages/onenote-converter/parser/src/onenote/mod.rspackages/onenote-converter/parser/src/onestore/mod.rspackages/onenote-converter/parser/src/onestore/object.rspackages/onenote-converter/parser/src/shared/prop_set.rspackages/tools/cspell/dictionary4.txt
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/onenote-converter/README.md`:
- Around line 98-105: Add the missing fenced code block language tags so both
command examples are tagged as bash: update the two triple-backtick blocks that
contain "bash$ cargo run -- ./test-data/ink.one --onestore" and "bash$ cargo run
-- ./test-data/ink.one --section" to start with ```bash (i.e., replace ``` with
```bash for those blocks) to satisfy markdownlint MD040.
- Around line 97-105: Update the README examples to make the cargo invocation
explicit for the inspect binary: replace the ambiguous "cargo run --
./test-data/..." calls with "cargo run --bin inspect -- ./test-data/..." for
both the --onestore and --section examples, and if this is a workspace where the
inspect binary lives in a different crate, add the appropriate -p <crate>
selector (e.g., the parser crate) so the command is copy/paste reliable; ensure
both example lines referencing the inspect binary are updated consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1390c987-ac51-4601-82b5-d368108a3cd1
📒 Files selected for processing (1)
packages/onenote-converter/README.md
| // Marks the end of a signature block. Ignored. | ||
| // See https://learn.microsoft.com/en-us/openspecs/office_file_formats/ms-onestore/0fa4c886-011a-4c19-9651-9a69e43a19c6 | ||
| iterator.next(); | ||
| log!("Ignoring DataSignatureGroupDefinitionFND"); |
There was a problem hiding this comment.
This change removes an unnecessary log statement that cluttered the tool's output.
|
|
||
| // According to MS-ONESTORE 2.1.12, revision_role *should* always be 0x1 | ||
| if data.base.revision_role != 0x1 { | ||
| // TODO: Find a test .one file that uses this and implement: | ||
| log_warn!( | ||
| "TO-DO: Apply the new role and context to the revision (role {:x})", | ||
| data.base.revision_role | ||
| ); | ||
| } |
There was a problem hiding this comment.
This change removes an unnecessary warning statement that cluttered the tool's output. The warning is now only emitted if the assigned revision role is different from the expected value documented in MS-ONESTORE 2.1.12.
|
|
||
| impl Debug for PropertySet { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| fn format_value(value: &PropertyValue) -> String { |
There was a problem hiding this comment.
This change includes the string representation of some Vec()s in the debug output (Vecs sometimes store Strings). This is useful when searching for a particular string in the lower-level --onestore debug output.
| Ok(value.to_string().unwrap()) | ||
| value.to_string().map_err(|err| err.into()) |
There was a problem hiding this comment.
This change prevents the UTF-16 decoder from panicing when encountering invalid Unicode. It instead returns an error that can be handled by the caller. This allows the debug tool to safely include string representations of certain fields that might not include valid UTF-16.
Problem
With the current
onenote-converter/logic, it's difficult to inspect the content of a.onefile. Manually inspecting the parsed.onefile can be useful when debugging OneNote import failures.Solution
Add a tool to
onenote-converter/to display the structures parsed from a.onefile.This is helpful, for example, to compare the OneNote file structures parsed by Joplin to the Onetastic debug XML posted by @juliusgodo in #13549. (Onetastic is a OneNote extension).
Sample output