chore: convert generators (generate_patt + nft_marker_gen) to arlog_*! (PR 3/4 for #90)#122
Open
maxwelljhuang wants to merge 2 commits into
Open
Conversation
Converts println!/eprintln!/eprint! in the two generator examples to the project arlog_*! macros, plus adds a [[example]] block for generate_patt in Cargo.toml. generate_patt.rs (716 lines, 34 print sites): - 16 help-banner eprintln! kept as-is (CLI UI, not log events) - 1 unknown-arg eprintln! -> arlog_w! (recoverable, parse continues) - 1 top-level Err eprintln! -> arlog_e! - 16 -> arlog_i! (success reports, setup info, debug/verbose-gated lines) - The if cfg.debug / if cfg.verbose wrappers and the --debug / --verbose CLI flags are preserved unchanged; only the inner eprintln! calls routed through arlog_i! with "Debug:"/"Verbose:" prefixes intact. Decoupling the flags from arlog levels would be a user-visible CLI change and is left to a separate PR. nft_marker_gen.rs (399 lines, 50 print sites): - 6 y/N prompt eprintln!/eprint! kept as-is (interactive UI) - 10 empty println!() spacer lines deleted (would render as bare [info] noise; original spacing was a stdout-style artifact) - 7 error-and-exit eprintln! -> arlog_e! - 2 recoverable eprintln! -> arlog_w! (per-scale failure, no-features warning), gated import follows since both sites are inside the ffi-backend cfg block - 25 -> arlog_i! including bounded enumeration loops over scales, matching the debug_labeling.rs:92-97 / load_nft.rs precedent Cargo.toml: new [[example]] block for generate_patt with required-features = ["log-helpers"], placed in the *_patt cluster. Verified --help renders plain text (no [info] prefixes) and single runs of both generators show correctly prefixed arlog output. Part 3 of 4 for webarkit#90.
Member
|
Thank you for this PR! 🙌🔝 i will review It this afternoon. |
kalwalt
reviewed
May 11, 2026
| */ | ||
|
|
||
| fn print_help_and_exit() { | ||
| eprintln!("generate_patt example — usage:"); |
Member
There was a problem hiding this comment.
You can use the arlog_rel here and in the lines after this code printing, it will not print the [info] pre text or other type of header.
kalwalt
reviewed
May 11, 2026
| use std::io::Write as _; | ||
| if !cli.yes { | ||
| eprintln!(); | ||
| eprintln!( |
Member
There was a problem hiding this comment.
As i said for the generate_patt.rs you can use arlog_rel here. Don not appyly to this line eprint!("Continue anyway? [y/N] "); i'm not sure that arlog_rel can works.
kalwalt
requested changes
May 11, 2026
Member
kalwalt
left a comment
There was a problem hiding this comment.
I left some comments in the code, if you can apply the changes requested. After that i will merge it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of 4 for #90 . Converts the two generator CLI examples to the project's arlog_*! macros. PR 3 is larger than PR 1 or PR 2 (84 print sites combined) and exercises arlog_e! and arlog_w! in examples/ for the first time, plus introduces three "do-not-convert" categories: CLI help banners, interactive prompts, and empty spacers. Calling out the contestable judgments up front so review can focus there.
Scope
Categorical decisions
CLI help banner stays as eprintln! (16 sites in generate_patt.rs
print_help_and_exit). Routing--helpoutput through arlog_i! would prefix every line with[info]and apply level filtering — wrong UX for help text, which is human UI not log events.Interactive y/N prompt stays as eprintln!/eprint! (6 sites in nft_marker_gen.rs). The prompt + read_line block needs synchronous stderr output that a log facade can't guarantee.
Empty
println!();spacer lines deleted rather than converted (10 sites in nft_marker_gen.rs). An empty arlog_i!() renders as a bare[info]with no body, which is noise. The spacing was a stdout-style artifact from the original C markerCreator. Happy to preserve them in some other form (e.g. arlog_i!("") to maintain visual structure) if you'd prefer — let me know.if cfg.debug/if cfg.verbosewrappers in generate_patt.rs preserved, inner calls converted to arlog_i! with "Debug:"/"Verbose:" prefixes intact. Two cleaner alternatives exist:Defaulting to (a) for minimum-change. Happy to flip to (b) or (c) — both are reasonable, and (c) might be the right long-term answer; let me know your preference and I'll push a fixup or open a follow-up.
Sub-classification: lines 328, 341, 347 in nft_marker_gen.rs are three per-scale outcomes in the FREAK feature loop, classified differently. 328 (arlog_i!) is success, 341 (arlog_i!) is an expected skip for small scales, 347 (arlog_w!) is an unexpected per-scale failure where the loop continues. Following the existing author's signal (println vs eprintln) for which is which.
small adjustment beyond the plan
use webarkitlib_rs::arlog_w;in nft_marker_gen.rs is gated on#[cfg(feature = "ffi-backend")]to silence an unused-import warning on the no-ffi build. Both arlog_w! call sites are already inside that cfg block; the import follows them.Verification
Skipped per pre-existing issues already documented on the issue thread: cargo build --all-features (macOS stdc++ link issue) and cargo clippy --all-targets -- -D warnings (lib-side doctest lint in arlog.rs:299).
Followups