Skip to content

Parameter Expansion Updates#290

Merged
UdeshyaDhungana merged 4 commits into
mainfrom
parameter-expansion-updates
Apr 24, 2026
Merged

Parameter Expansion Updates#290
UdeshyaDhungana merged 4 commits into
mainfrom
parameter-expansion-updates

Conversation

@UdeshyaDhungana

@UdeshyaDhungana UdeshyaDhungana commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Note

Medium Risk
Updates multiple parameter-expansion stages and assertions, which can change pass/fail outcomes across different shells and user implementations. Risk is moderate because it alters the tester’s canonical expectations for identifier validity and expansion behavior.

Overview
Parameter expansion stage tests are updated to be stricter and more representative. Missing-variable printing via declare -p is renamed/clarified as DeclarePrintMissingVariableTestCase and now hard-fails if invoked with an invalid identifier.

Identifier validation coverage is expanded. Stage pex4 adds an additional invalid assignment case (hyphens in variable names) and DeclareAssignmentTestCase broadens zsh-compatible fallback patterns to distinguish digit-prefixed vs mid-string invalid characters.

Argument expansion scenarios are strengthened. Stages pex5pex7 now assert expansions embedded within words (both bare $VAR and braced ${VAR} forms), braced expansions at word boundaries with literal suffixes, and mixed missing-variable expansions, with fixture expectations updated accordingly.

Reviewed by Cursor Bugbot for commit b565635. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Panic message references old renamed type name
    • Updated the panic message to reference the correct type name DeclarePrintMissingVariableTestCase instead of the old name DeclarePrintErrorTestCase.

Create PR

Or push these changes by commenting:

@cursor push adee86d828
Preview (adee86d828)
diff --git a/internal/test_cases/declare_print_missing_variable_test_case.go b/internal/test_cases/declare_print_missing_variable_test_case.go
--- a/internal/test_cases/declare_print_missing_variable_test_case.go
+++ b/internal/test_cases/declare_print_missing_variable_test_case.go
@@ -22,7 +22,7 @@
 	}
 
 	if !isValidIdentifier(t.Variable) {
-		panic(fmt.Sprintf("Codecrafters Internal Error - DeclarePrintErrorTestCase called on invalid identifier %s", t.Variable))
+		panic(fmt.Sprintf("Codecrafters Internal Error - DeclarePrintMissingVariableTestCase called on invalid identifier %s", t.Variable))
 	}
 
 	testCase := CommandResponseTestCase{

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 991d4f4. Configure here.

Comment thread internal/test_cases/declare_print_missing_variable_test_case.go Outdated
@UdeshyaDhungana UdeshyaDhungana merged commit c29388c into main Apr 24, 2026
7 checks passed
@UdeshyaDhungana UdeshyaDhungana deleted the parameter-expansion-updates branch April 24, 2026 11:17
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.

1 participant