Skip to content

Conversation

@sgoedecke
Copy link
Collaborator

The --var flag was defined as a StringSlice, which treats commas as separators between multiple values. When users passed --var city="paris, milan", it was being split into two separate values:

  • "paris" (invalid format, no =)
  • " milan" (invalid format, no =)

This PR updates the type to StringArray and adds a test.

@sgoedecke sgoedecke marked this pull request as ready for review July 30, 2025 22:06
@sgoedecke sgoedecke requested a review from a team as a code owner July 30, 2025 22:07
Copilot AI review requested due to automatic review settings July 30, 2025 22:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the --var flag incorrectly split values containing commas into separate arguments, causing parsing errors when users passed template variables with comma-separated values.

  • Changes the flag type from StringSlice to StringArray to prevent automatic comma splitting
  • Adds comprehensive test coverage for the comma-containing values scenario

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cmd/run/run.go Updates flag definition and getter from StringSlice to StringArray
cmd/run/run_test.go Adds test case verifying comma-containing values are preserved correctly

Copy link
Member

@chadfawcett chadfawcett left a comment

Choose a reason for hiding this comment

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

I'm currently debugging why my multiline string isn't working as a variable. This very well may be the culprit.

@sgoedecke sgoedecke merged commit 210db6f into main Jul 30, 2025
5 checks passed
@sgoedecke sgoedecke deleted the sgoedecke/fixup-vars-with-commas branch July 30, 2025 22:12
@pelikhan
Copy link
Contributor

@sgoedecke can you move it to a helper file that I can use as well with the "generate" command?

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.

5 participants