Skip to content

Conversation

@arepala-uml
Copy link
Contributor

@arepala-uml arepala-uml commented Nov 30, 2025

  1. Replaced the custom flag-based CLI with a Cobra root command. Usage/help and shell parsing will come from Cobra.
  2. Modified config/plugin flag registration to pflag.
  3. Updated the README help snippet to match the new adder --help output.

Closes #164


Summary by cubic

Switched the CLI to a Cobra root command for consistent parsing and help output. Migrated all config/plugin flags to pflag and updated README to match the new adder --help.

  • Refactors

    • Replaced custom FlagSet with Cobra (rootCmd, RunE).
    • Bound config and plugin options via pflag (BindFlags, PopulateCmdlineOptions).
    • Returned errors instead of exiting in runtime paths for cleaner failure handling.
    • Kept existing flag names; adds -h/--help and --version.
  • Dependencies

    • Added github.com/spf13/cobra and github.com/spf13/pflag.

Written for commit a1cd436. Summary will update automatically on new commits.

Summary by CodeRabbit

  • Documentation

    • Updated command-line help references and formatting for clarity and consistency.
  • New Features

    • Enhanced command-line interface with improved help output formatting and structured error handling.
  • Refactor

    • Improved startup initialization and configuration binding for more robust error handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 30, 2025

📝 Walkthrough

Walkthrough

This pull request introduces a Cobra-based CLI framework to replace the standard library flag package. The changes refactor cmd/adder/main.go to implement a rootCmd that delegates to a new run() function, replace flag-based argument parsing with pflag throughout the codebase, migrate startup logic into the run() function with error propagation, and add two new dependencies (cobra v1.8.1 and pflag v1.0.6). Config and plugin interfaces are updated to accept pflag.FlagSet instead of flag.FlagSet. README formatting is normalized for JSON examples and help references.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • cmd/adder/main.go: Significant structural refactoring with Cobra integration, error propagation logic replacing os.Exit calls, background error handling, and graceful shutdown implementation require careful verification
  • internal/config/config.go: Method signature change from ParseCmdlineArgs() to BindFlags() with altered control flow (parsing responsibility shifts externally); verify all call sites are updated correctly
  • Plugin interface changes (plugin/option.go, plugin/register.go): Public method signatures updated to use pflag.FlagSet; ensure all implementations and callers are compatible
  • New dependencies: Verify cobra and pflag versions are stable and compatible with existing codebase
  • Error handling paths: Ensure errors propagate correctly through the new run() function and root command execution

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: replacing the existing CLI with Cobra for command-line handling.
Linked Issues check ✅ Passed The PR implements the core objective from #164: replacing command-line handling with Cobra to improve usage output and enable shell completion.
Out of Scope Changes check ✅ Passed All changes are scoped to Cobra integration: CLI refactoring, pflag migration, README updates, and necessary dependency additions. No unrelated modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cobra_commandline_handle

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arepala-uml arepala-uml changed the title enhancement(main): Use cobra for commandline handling feat(main): Use cobra for commandline handling Nov 30, 2025
Signed-off-by: Akhil Repala <arepala@blinklabs.io>
@arepala-uml arepala-uml force-pushed the cobra_commandline_handle branch from 29d73ed to a1cd436 Compare November 30, 2025 19:38
@arepala-uml arepala-uml marked this pull request as ready for review November 30, 2025 21:13
@arepala-uml arepala-uml requested review from a team as code owners November 30, 2025 21:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

194-210: Inconsistent help flag references in filtering section.

This section still references -h/-help and shows old-style flag output format, while the updated help section at line 111-128 now uses --help and Cobra's output format. Consider updating this section for consistency.

-You can get a list of all available filter options by using the `-h`/`-help`
+You can get a list of all available filter options by using the `--help`
 flag.

 ```bash
-$ ./adder -h
-Usage of adder:
+$ ./adder --help
+Usage:
+  adder [flags]
+
+Flags:
 ...
-  -filter-address string
+      --filter-address string
         specifies address to filter on
-  -filter-asset string
+      --filter-asset string
         specifies the asset fingerprint (asset1xxx) to filter on
-  -filter-policy string
+      --filter-policy string
         specifies asset policy ID to filter on
-  -filter-type string
+      --filter-type string
         specifies event type to filter on
 ...

</blockquote></details>

</blockquote></details>
🧹 Nitpick comments (2)
README.md (1)

61-99: Inconsistent JSON indentation in transaction example.

The block and rollback examples were updated to use 2-space indentation, but the transaction example still uses 4-space indentation. Consider normalizing for consistency across all examples.

cmd/adder/main.go (1)

143-159: Consider returning error instead of calling os.Exit in goroutine.

This os.Exit(1) in the goroutine (line 157) is inconsistent with the PR's goal of replacing exit-based error handling with error propagation. It bypasses graceful shutdown and Cobra's error handling.

Consider using an error channel pattern or making the debug listener startup synchronous so errors can be returned:

 	// Start debug listener
 	if cfg.Debug.ListenPort > 0 {
 		logger.Info(fmt.Sprintf(
 			"starting debug listener on %s:%d",
 			cfg.Debug.ListenAddress,
 			cfg.Debug.ListenPort,
 		))
+		debugger := &http.Server{
+			Addr: fmt.Sprintf(
+				"%s:%d",
+				cfg.Debug.ListenAddress,
+				cfg.Debug.ListenPort,
+			),
+			ReadHeaderTimeout: 60 * time.Second,
+		}
 		go func() {
-			debugger := &http.Server{
-				Addr: fmt.Sprintf(
-					"%s:%d",
-					cfg.Debug.ListenAddress,
-					cfg.Debug.ListenPort,
-				),
-				ReadHeaderTimeout: 60 * time.Second,
-			}
 			err := debugger.ListenAndServe()
-			if err != nil {
-				logger.Error(
-					fmt.Sprintf("failed to start debug listener: %s", err),
-				)
-				os.Exit(1)
+			if err != nil && err != http.ErrServerClosed {
+				logger.Error(fmt.Sprintf("debug listener error: %s", err))
 			}
 		}()
 	}

Alternatively, if immediate failure is critical, start the listener synchronously before the goroutine to catch binding errors early.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03d287d and a1cd436.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • README.md (5 hunks)
  • cmd/adder/main.go (8 hunks)
  • go.mod (1 hunks)
  • internal/config/config.go (3 hunks)
  • plugin/option.go (2 hunks)
  • plugin/register.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/adder/main.go (4)
internal/config/config.go (1)
  • GetConfig (173-175)
internal/version/version.go (2)
  • Version (23-23)
  • GetVersionString (27-33)
plugin/register.go (5)
  • GetPlugins (103-111)
  • PluginTypeInput (26-26)
  • PluginTypeOutput (27-27)
  • ProcessConfig (86-101)
  • ProcessEnvVars (69-84)
plugin/plugin.go (1)
  • Plugin (21-27)
🔇 Additional comments (8)
plugin/register.go (1)

17-21: LGTM - clean pflag migration.

The import and function signature changes are minimal and correct. The pflag library maintains API compatibility with the standard flag package for these operations, so no logic changes are needed.

Also applies to: 58-67

plugin/option.go (1)

22-24: LGTM - straightforward pflag migration.

The migration correctly updates the import and method signature. The pflag library's StringVar, BoolVar, IntVar, and UintVar methods are API-compatible with the standard library, so the flag registration logic remains unchanged.

Also applies to: 45-88

internal/config/config.go (1)

154-170: LGTM - clean refactor to external flag binding.

The BindFlags method correctly delegates flag registration to the provided pflag.FlagSet, allowing Cobra to handle parsing. This is the appropriate pattern for Cobra integration. The --version flag works correctly: it's registered in BindFlags, parsed by Cobra, and checked in run() at line 87 to print the version string.

cmd/adder/main.go (5)

41-52: LGTM!

The Cobra command setup is well-structured:

  • SilenceUsage: true prevents printing usage on every error, which is appropriate for a daemon-style application.
  • cobra.NoArgs correctly enforces that no positional arguments are accepted.
  • Delegating to run() via RunE enables clean error propagation.

81-84: LGTM!

Using panic for flag binding errors in init() is acceptable here since these are configuration errors that indicate a programming mistake rather than a runtime condition.


86-134: LGTM!

The refactored error handling using fmt.Errorf with %w for error wrapping is clean and idiomatic. Early returns for --version and list commands are appropriate.


213-221: LGTM!

The non-blocking error consumer pattern is appropriate here. The comment clearly documents the intentional behavior of logging pipeline errors without terminating, which allows the application to continue operating through transient errors.


237-241: LGTM!

The main() function correctly follows the standard Cobra pattern: execute the root command and exit with status 1 on error. Cobra handles error printing internally.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

@arepala-uml arepala-uml merged commit 6e5bb98 into main Dec 3, 2025
12 checks passed
@arepala-uml arepala-uml deleted the cobra_commandline_handle branch December 3, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use cobra for commandline handling

3 participants