fix(_mnn): remove false claims from help text - #117
Conversation
Run as _mnn, or through any name other than en_ or em_, the help said that the command copies an em dash and gave a bare invocation as an example, but that invocation exits 1. Each inline conditional in usage() tested only for en_, so every other name fell through to the em-dash text. usage() now picks its text by name. en_ and em_ each describe their own dash and point to the other command. Every other name gets a description of the shared implementation that lists both commands, with no example. The note about invoking the script as en_ or em_ is gone from their own help, where it told the reader to do what they had already done. The tests now check that every line of help that shows a dash names the command that copies it, in place of asserting the note's wording. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper |
PR Summary by QodoCorrect
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The functional changes align with the PR description and the added tests cover the corrected help-text behavior, with only a minor test-helper readability nit noted.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR fixes _mnn --help output so it no longer falsely claims that invoking the shared implementation under an arbitrary name will copy a specific dash; help text is now selected based on SCRIPT_NAME and provides accurate, name-appropriate guidance.
Changes:
- Refactors
_mnnusage()to use acaseonSCRIPT_NAME, emitting distinct help foren_,em_, and a “shared implementation” help for all other names. - Removes the misleading “must be invoked as…” note from
en_/em_help while keeping direct-invocation error behavior intact. - Updates and expands
tests/_mnn.batsto validate correct help attribution and prevent future regressions.
| File | Description |
|---|---|
_mnn |
Reworks usage() to generate accurate help text based on the invoked name (en_, em_, or other). |
tests/_mnn.bats |
Adds targeted help-text assertions and removes assertions that enforced the previously-misleading note. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
assert_dashes_attributed_to_commands appended the em-dash scan to the en-dash scan with stray+=$(...). Command substitution strips trailing newlines, so when both scans found stray lines, the last en-dash line and the first em-dash line ran together in the failure message. Both scans now run in one command substitution, which keeps one stray line per line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The help-text selection logic is corrected cleanly and the added/updated tests directly cover the previously incorrect behavior without introducing observable regressions.
Review effort: Lite
Findings: None

Run as
_mnn,--helpdescribed a command that doesn't work:Each inline conditional in
usage()tested only foren_, so every name other thanen_, including a symlink under any other name, fell through to the em-dash text.usage()now picks its text by name:en_andem_each describe their own dash and point to the other command. The note about invoking the script asen_orem_is gone from their help, since it told the reader to do what they had already done.Examples:section, because no example would work.Tests
Two
_mnnhelp tests asserted the wording of the misdirected note; those assertions are removed. Six tests are added, and all six failed before the fix. The central one checks that every line of help that shows a dash (glyph or code point) also names the command that copies it, so the help can't say that the invoked name copies one. Against the old code, it flagged exactly the two false lines above.Verification
make test: 332 of 332 pass (Linux and Windows clipboard tests skip on macOS).tests/_mnn.batsunder/bin/bash3.2.57: 43 of 43 pass.make run-pre-commit: clean.🤖 Generated with Claude Code