fix(cli): make sentry cli --version print the version#1128
Merged
Conversation
Stricli only handles `--version` at the application proxy, so `sentry
--version` worked but `sentry cli --version` failed route resolution
("No command registered for `--version`"), and `sentry issue --version`
errored with "No flag registered". Reported in #discuss-cli.
preprocessArgv() now normalizes a top-level `--version` token (appearing
after any route group/subcommand, before any `--` escape) to a plain
`["--version"]` so the app-level handler prints it consistently. `-v` is
left untouched — it is the reserved short alias for `--verbose`. Tokens
after `--` are ignored so `sentry monitor run <slug> -- tool --version`
still forwards `--version` to the wrapped command.
Extracted preprocessArgv (version-normalize + global-flag hoist) to keep
runCli under the cognitive-complexity budget. Adds unit tests for
isVersionRequest and preprocessArgv.
Contributor
|
Contributor
Codecov Results 📊✅ Patch coverage is 81.82%. Project has 5052 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.33% 81.34% +0.01%
==========================================
Files 392 392 —
Lines 27061 27070 +9
Branches 17558 17564 +6
==========================================
+ Hits 22009 22018 +9
- Misses 5052 5052 —
- Partials 1832 1832 —Generated by Codecov Action |
Adversarial-review NIT: document that the naive token scan in isVersionRequest treats a bare `--version` as a version request even when it would otherwise be a value flag's argument (e.g. `issue list -q --version`); the `=` form passes the literal string. No behavior change.
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.
Problem
Reported by sergical in #discuss-cli:
sentry cli --versionerrors withNo command registered for \--version`, even thoughsentry --version` works.Stricli only handles
--versionat the application proxy, so it never reachesroute maps. The behavior was inconsistent across the tree:
sentry --version0.0.0-dev✅sentry cli --versionNo command registered for \--version`` ❌sentry issue --versionNo flag registered for --version❌Fix
preprocessArgv()(inargv-hoist.ts) now normalizes a top-level--versiontoken —appearing after any route group/subcommand, before any
--escape — to a plain["--version"], so the app-level version handler prints it. After the fix,sentry cli --version,sentry issue --version, andsentry issue list --versionall printthe version, consistent with
sentry --version.Notes:
-vis intentionally left alone — it's the reserved short alias for--verbose(
GLOBAL_FLAGS). Only the long--versionform is normalized. (BYK's guidance in thethread was specifically about
sentry cli --version.)--escape are ignored, sosentry monitor run <slug> -- tool --versionstill forwards
--versionto the wrapped command.--version=valueis not matched (no command defines a--versionvalue flag).preprocessArgv(version-normalize + existing global-flag hoist) sorunClistays under the cognitive-complexity budget.
Out of scope: aliasing
sentry upgrade→ CLI upgrade. Per BYK in-thread,sentry upgradeis reserved for SDK upgrade flows and must not be remapped.Tests
Unit tests added for
isVersionRequestandpreprocessArgv(route-scoped--version,nested subcommand,
-vnon-match, post---escape,--version=foonon-match). Verifiedin dev:
sentry cli --version→0.0.0-dev;sentry cli -vunchanged. typecheck + lint clean.