Conversation
Remove all --include-* flags for built-in modules (csv, json, yaml, toml, xml, fuzzy, test) from CLI and associated test cases. Simplifies CLI interface and reduces code complexity.
There was a problem hiding this comment.
Pull request overview
This PR removes the CLI flags that implicitly included built-in modules (csv/json/yaml/toml/xml/fuzzy/test), simplifying mq-run’s argument surface and query construction.
Changes:
- Removed
--json/--csv/--yaml/--toml/--xml/--fuzzy/--testflags and the corresponding query-prefix include logic frommq-run’s CLI. - Deleted integration/unit tests that depended on those flags.
- Tweaked the CLI
abouttext and removed anitertoolsimport.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/mq-run/src/cli.rs | Removes include flags + query composition for module includes; updates help/about text; deletes related unit tests. |
| crates/mq-run/tests/integration_tests.rs | Removes integration test cases that validated module parsing via the removed CLI flags. |
Comments suppressed due to low confidence (2)
crates/mq-run/tests/integration_tests.rs:128
- The integration test cases for built-in data modules (csv/json/yaml/xml parse) were removed along with the CLI flags. That drops coverage of these modules entirely. Consider reintroducing equivalent cases by removing the flags and making the query explicitly include/import the module (e.g.,
include "json" | json_parse()orimport "json" | json::json_parse(...)) so the same behaviors remain tested under the new CLI contract.
#[case::test_stream(
vec!["--unbuffered", "-I", "text", "--stream", ".h"],
"# title\n",
None
)]
crates/mq-run/src/cli.rs:1633
- The unit tests for
include_*module flags were removed, but nothing replaces that coverage for module loading/exposure. If the intent is to require explicitinclude/importin the query now, it would be good to add tests asserting the new behavior (e.g.,json_parse()fails without include/import, and succeeds with an explicit include/import) to prevent regressions.
#[test]
fn test_fmt_file_not_found() {
let cli = Cli {
input: InputArgs::default(),
| #[command( | ||
| about = "mq is a markdown processor that can filter markdown nodes by using jq-like syntax.", | ||
| about = "mq is a markdown processor that can filter markdown nodes by using jq-like syntax. \nhttps://mqlang.org", | ||
| long_about = None | ||
| )] |
There was a problem hiding this comment.
The about string includes a trailing space before the newline ("...syntax. \nhttps://..."), which will show up in --help output. Consider removing the extra space (or moving the URL to after_help/long_about) to avoid awkward formatting in the help text.
| /// Enable streaming mode for processing large files line by line | ||
| #[arg(long, default_value_t = false)] | ||
| stream: bool, | ||
|
|
||
| /// Include the built-in JSON module | ||
| #[arg(long = "json", default_value_t = false)] | ||
| include_json: bool, | ||
|
|
||
| /// Include the built-in CSV module | ||
| #[arg(long = "csv", default_value_t = false)] | ||
| include_csv: bool, | ||
|
|
||
| /// Include the built-in Fuzzy module | ||
| #[arg(long = "fuzzy", default_value_t = false)] | ||
| include_fuzzy: bool, | ||
|
|
||
| /// Include the built-in YAML module | ||
| #[arg(long = "yaml", default_value_t = false)] | ||
| include_yaml: bool, | ||
|
|
||
| /// Include the built-in TOML module | ||
| #[arg(long = "toml", default_value_t = false)] | ||
| include_toml: bool, | ||
|
|
||
| /// Include the built-in XML module | ||
| #[arg(long = "xml", default_value_t = false)] | ||
| include_xml: bool, | ||
|
|
||
| /// Include the built-in test module | ||
| #[arg(long = "test", default_value_t = false)] | ||
| include_test: bool, | ||
| } |
There was a problem hiding this comment.
This PR removes the --json/--csv/--yaml/--toml/--xml/--fuzzy/--test flags. The CLI reference docs currently still list these options (e.g., README.md and docs/books/src/reference/cli.md), so they’ll be out of date after this merges. Please update the generated CLI help/docs (and add a CHANGELOG entry if this is a breaking user-facing change).
| use clap::{Parser, Subcommand}; | ||
| use colored::Colorize; | ||
| use glob::glob; | ||
| use itertools::Itertools; | ||
| use miette::IntoDiagnostic; | ||
| use miette::miette; | ||
| use mq_lang::DefaultEngine; |
There was a problem hiding this comment.
itertools::Itertools was removed from this file and there are no remaining Itertools usages in mq-run sources. mq-run/Cargo.toml still declares an itertools dependency, which is now unused; consider removing it to keep the dependency set minimal.
No description provided.