feat(cmd): parse matrix flags by MeteorsLiu · Pull Request #111 · xgo-dev/llar

MeteorsLiu · 2026-05-21T08:44:19Z

Closes #113

codecov · 2026-05-21T08:47:51Z

Codecov Report

❌ Patch coverage is 80.95238% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.47%. Comparing base (acb4258) to head (b40d4d8).

Files with missing lines	Patch %	Lines
cmd/llar/internal/matrix_flags.go	85.96%	5 Missing and 3 partials ⚠️
cmd/llar/internal/make.go	33.33%	1 Missing and 1 partial ⚠️
cmd/llar/internal/test.go	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   80.48%   80.47%   -0.01%     
==========================================
  Files          36       37       +1     
  Lines        2034     2095      +61     
==========================================
+ Hits         1637     1686      +49     
- Misses        290      297       +7     
- Partials      107      112       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gemini-code-assist

Code Review

This pull request implements CLI matrix flag support for the llar make and llar test commands, allowing users to define build dimensions such as architecture and OS directly through command-line arguments. This is supported by a new internal parser and updated command logic, along with design documentation and tests. The review feedback identifies a logic inconsistency in the parser that prevents values starting with hyphens from being accepted for certain matrix flags. Furthermore, the MatrixStr field added to modules.Options appears to be unused in the current implementation, suggesting it may be dead code.

MeteorsLiu · 2026-05-26T08:16:11Z

/review

MeteorsLiu · 2026-05-26T08:16:20Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces dynamic matrix flag resolution for the make and test commands in llar. It extracts unknown long flags from the command-line arguments to build a custom build matrix, falling back to the host matrix combination when none are provided. The review feedback highlights two key areas for improvement in the argument parsing logic: first, using cmd.CalledAs() instead of cmd.Name() to ensure the command is correctly located in os.Args even when invoked via an alias; second, enhancing the short flag parser to iterate through all characters of a grouped short flag to prevent unknown flags from being silently ignored.

Allow passing target matrix dimensions via unknown long flags (e.g. --os linux --arch amd64) and --matrix-<key> prefix for conflicts with known flags. Uses Cobra's FParseErrWhitelist to silently consume unknown flags, then extracts them from os.Args via cmd.Flags().Lookup(). Closes xgo-dev#113

Replace hand-written arg scanner with go-flags library to handle edge cases (negative values, --, = forms) correctly via UnknownOptionHandler. Closes xgo-dev#113

Closes xgo-dev#113

MeteorsLiu · 2026-05-26T09:08:53Z

/review

MeteorsLiu · 2026-05-26T09:08:59Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces dynamic matrix flag parsing for the make and test commands using the github.com/jessevdk/go-flags library. It extracts unknown long flags as matrix dimensions while ignoring known Cobra flags. The review feedback highlights several critical and medium-severity issues: modifying the global os.Args in tests without restoring it can cause side effects, cmd.CalledAs() should fall back to cmd.Name() if empty to prevent incorrect index resolution, and redundant shorthand lookups can be simplified.

Searching os.Args for cmd.CalledAs() is fragile when the subcommand name appears as a flag value earlier in the args. Use CommandPath() word count to determine the fixed depth and slice os.Args directly. Closes xgo-dev#113

MeteorsLiu · 2026-05-26T09:54:49Z

/gemini review

MeteorsLiu · 2026-05-26T09:57:10Z

/review

gemini-code-assist

Code Review

This pull request introduces dynamic matrix flag parsing for the llar CLI tool, allowing unknown long flags to be extracted as matrix dimensions. It adds a new utility matrix_flags.go along with tests, and integrates this logic into the make and test commands. The code reviewer provided several valuable suggestions to improve argument parsing: changing cobra.ExactArgs(1) to cobra.ArbitraryArgs to prevent validation failures when flags take arguments, updating the extraction helpers to return remaining positional arguments cleanly, and adding a safety check to prevent potential panics.

fennoai

Review Summary

Good feature addition with solid test coverage. The separation of extractMatrixFlags (testable with explicit args) from resolveMatrixStr is well-designed, error messages are clear, and the FParseErrWhitelist usage is correct.

Key concerns:

Matrix values lack validation — keys are regex-checked but values flow unsanitized into filesystem paths and separator-delimited combination strings
Discarded error from parser.ParseArgs
Silent matrix dimension capture — any typo of a known flag (e.g. --vebose) silently becomes a matrix dimension with no warning

fennoai · 2026-05-26T10:02:39Z

+			val = args[0]
+			args = args[1:]
+		}
+		if val == "" {
+			matrixErr = fmt.Errorf("missing value for matrix flag --%s", option)
+			return args, nil
+		}
+		dims[key] = val


Value validation missing. Keys are validated with matrixKeyRE, but values are unconstrained beyond the empty-string check. Since matrixStr flows into filesystem paths (cache dirs, install paths) and into -/|-delimited combination strings:

A value like ../../etc could cause path traversal in cache lookups

A value containing - (the Combinations separator) could collide with legitimate multi-dim combinations

Consider adding a value regex similar to keys, or at minimum rejecting values containing /, \\, .., and |.

fennoai · 2026-05-26T10:02:39Z

+		return args, nil
+	}
+
+	parser.ParseArgs(subArgs)


The error returned by parser.ParseArgs() is discarded. While UnknownOptionHandler handles most cases via the matrixErr closure, the parser itself could return errors for other reasons (e.g., internal parse failures). Consider checking the error or adding a comment explaining why it's intentionally ignored.

Suggested change

parser.ParseArgs(subArgs)

if _, parseErr := parser.ParseArgs(subArgs); parseErr != nil && matrixErr == nil {

// go-flags may return parse errors not captured by UnknownOptionHandler

matrixErr = parseErr

}

luoliwoshang

LGTM

This comment was marked as outdated.

Sign in to view

MeteorsLiu force-pushed the feat/make-matrix-flags-goplus-main branch from c74f1fe to b28e3a2 Compare May 21, 2026 08:58

gemini-code-assist Bot reviewed May 21, 2026

View reviewed changes

Comment thread cmd/llar/internal/matrix_flags.go Outdated

Comment thread cmd/llar/internal/matrix_flags.go Outdated

Comment thread internal/modules/load.go Outdated

MeteorsLiu force-pushed the feat/make-matrix-flags-goplus-main branch 3 times, most recently from 72259dd to 6fc1bd9 Compare May 26, 2026 08:15

gemini-code-assist Bot reviewed May 26, 2026

View reviewed changes

Comment thread cmd/llar/internal/matrix_flags.go Outdated

Comment thread cmd/llar/internal/matrix_flags.go Outdated

This comment was marked as outdated.

Sign in to view

MeteorsLiu added 2 commits May 26, 2026 16:51

refactor(cmd): use go-flags UnknownOptionHandler for matrix parsing

55a2f28

Replace hand-written arg scanner with go-flags library to handle edge cases (negative values, --, = forms) correctly via UnknownOptionHandler. Closes xgo-dev#113

MeteorsLiu force-pushed the feat/make-matrix-flags-goplus-main branch from 1e02822 to 55a2f28 Compare May 26, 2026 08:53

fix(cmd): use CalledAs() for alias-safe subcommand lookup in os.Args

88bc6d9

Closes xgo-dev#113

gemini-code-assist Bot reviewed May 26, 2026

View reviewed changes

Comment thread cmd/llar/internal/matrix_flags_test.go

Comment thread cmd/llar/internal/matrix_flags_test.go

Comment thread cmd/llar/internal/matrix_flags.go Outdated

Comment thread cmd/llar/internal/matrix_flags.go Outdated

This comment was marked as outdated.

Sign in to view

MeteorsLiu force-pushed the feat/make-matrix-flags-goplus-main branch from 63aa5be to b40d4d8 Compare May 26, 2026 09:54

gemini-code-assist Bot reviewed May 26, 2026

View reviewed changes

Comment thread cmd/llar/internal/make.go

Comment thread cmd/llar/internal/test.go

Comment thread cmd/llar/internal/matrix_flags.go

Comment thread cmd/llar/internal/matrix_flags.go

Comment thread cmd/llar/internal/matrix_flags_test.go

fennoai Bot reviewed May 26, 2026

View reviewed changes

MeteorsLiu requested a review from luoliwoshang May 26, 2026 10:05

luoliwoshang approved these changes May 27, 2026

View reviewed changes

luoliwoshang merged commit 927b834 into xgo-dev:main May 27, 2026
4 checks passed

-	parser.ParseArgs(subArgs)
+	if _, parseErr := parser.ParseArgs(subArgs); parseErr != nil && matrixErr == nil {
+		// go-flags may return parse errors not captured by UnknownOptionHandler
+		matrixErr = parseErr
+	}

Conversation

MeteorsLiu commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

MeteorsLiu commented May 26, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fennoai Bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

fennoai Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

fennoai Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

luoliwoshang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MeteorsLiu commented May 21, 2026 •

edited

Loading

codecov Bot commented May 21, 2026 •

edited

Loading