fix(variable:get): avoid TypeError when selecting a variable interactively#126
Open
pjcdawkins wants to merge 1 commit into
Open
fix(variable:get): avoid TypeError when selecting a variable interactively#126pjcdawkins wants to merge 1 commit into
pjcdawkins wants to merge 1 commit into
Conversation
…ively When run without a name argument, the command lets the user pick a variable interactively. In that case the `name` argument is null, but it was still passed to OsUtil::escapeShellArg() when printing the "To update the variable" hint, which requires a string and threw a fatal TypeError after the variable had already been displayed. Use the selected variable's own name instead, which is populated in all code paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the legacy PHP CLI variable:get command when running interactively (no name argument provided). The command previously displayed the selected variable successfully, then fatally errored while rendering the “update” hint due to passing null into a strictly-typed shell-escaping helper.
Changes:
- Use the selected variable’s populated name (
$variable->name) instead of the optional input argument ($name) when building the trailingvariable:updatehint.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
What
Fixes a fatal
TypeErrorthrown byvariable:getwhen a variable is chosen interactively (i.e. the command is run without a name argument).Symptom
The command runs, prompts for a variable, and displays it correctly — then crashes while printing the trailing hint:
Cause
The
nameargument is optional. In the interactive path it staysnull, but it was still passed toOsUtil::escapeShellArg()(which requires astring) when rendering the "To update the variable" hint. The crash happens after the variable has already been displayed, so the command appears to succeed and then fatals.Fix
Use the selected variable's own name (
$variable->name), which is populated in every code path, instead of the raw argument.Notes
escapeShellArg() expects string, mixed givenbut currently baselines it as noise. Once this merges, that baseline entry becomes obsolete and should be dropped on rebase.🤖 Generated with Claude Code