Repository navigation
Tie the desktop app's types and CSV escaping to the Rust side - #538
Merged
Merged
Conversation
Types. apps/netscli-gui/src/types/netscli.ts mirrors 23 netscli-core result types by hand and nothing checked it. netscli-core gains an optional `ts` feature that derives ts-rs bindings for those types (overrides for MacAddress, IpNet and Duration; fields skipped when empty marked optional); the generated files are committed under src/types/generated, and src/types/netscli.contract.ts fails tsc unless each hand-written type has exactly the generated field names and accepts every value the core can send. CI regenerates the bindings and fails if they differ from the committed ones. netscli.ts stays hand-written: it is deliberately looser for older saved bundles and carries the docs. Checked: adding a field to a generated type, or changing a field's type, fails tsc. No drift was found today. CSV. The CLI's --csv and the desktop app's CSV export each had their own escaping. testdata/csv-escape.json holds 24 cases both are tested against. Against it the desktop export failed 5: it let control characters through (an ESC sequence from a banner went into the file as-is) and treated "\t1" and "\r5" as numbers because Number() trims whitespace, skipping the formula guard the CLI applies. Fixed to match the CLI.
Contributor
|
Site preview: https://pr-538.netscli-site-preview.pages.dev Built from 7530d0d with Production is unaffected: netscli.com is served from GitHub Pages via |
…-types # Conflicts: # CHANGELOG.md
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
1 participant
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.
Gap 4 from the audit, with a correction: I described it as "about 20 copies of the result types across the interfaces". An inventory found the CLI, TUI and MCP server already use the core structs directly through serde. The real duplicate is one hand-written TypeScript mirror of 23 core types in the desktop app (
src/types/netscli.ts), plus two CSV escaping implementations (CLI--csvand the desktop export). The other export paths (CLI Markdown, TUI transcript, JSON) are different formats, not copies.Types: checked, not replaced
netscli-coregets an optionaltsfeature that derives ts-rs bindings for the 23 result types. MacAddress, IpNet and Duration have overrides. Fields skipped when empty are marked optional, so the generated types say what the JSON really contains.apps/netscli-gui/src/types/generated/. CI (Format + Clippy job) regenerates them and fails if they differ.src/types/netscli.contract.tsfailstscunless each hand-written type has exactly the generated field names and accepts every value the core can send.netscli.tsstays hand-written on purpose. It is looser than the core where saved bundles from older versions lack fields, and it carries the docs the app's code relies on.tsc, and so does changing a field's type.CSV: one set of cases for both
testdata/csv-escape.jsonholds 24 cases. The CLI'sescapeand the desktop app'scsvEscapeare both tested against every one. The CLI passed as it was. The desktop export failed 5:"\t1"and"\r5"as numbers, becauseNumber()trims whitespace, and so skipped the formula guard the CLI applies.Fixed to match the CLI (Unicode Cc, as Rust's
is_control; decimal-literal number test). CHANGELOG under Fixed.Checked locally
cargo clippy -D warningswith and without--features ts,mdns,cargo fmttsc, eslint, all 251 tests (18 new shared-case tests)