Skip to content

Conversation

@nobShinjo
Copy link
Contributor

@nobShinjo nobShinjo commented Jan 12, 2026

Summary

  • Remove trailing commas from the last element in PowerShell @() arrays when generating completions
  • Apply to top-level commands, flags, and subcommands arrays
  • Add a regression test to ensure no trailing comma is emitted

Testing

  • pnpm exec vitest run test/core/completions/generators/powershell-generator.test.ts
    • Test Files 1 passed (1)
    • Tests 30 passed (30)
    • Start at 14:15:17
    • Duration 854ms (transform 94ms, setup 0ms, collect 72ms, tests 12ms, environment 0ms, prepare 177ms)

Related

Summary by CodeRabbit

Bug Fixes

  • Fixed PowerShell completion scripts to generate valid syntax by eliminating trailing commas from array literals in generated completions.

Tests

  • Added comprehensive test coverage to validate proper formatting of PowerShell arrays without trailing commas.

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

@nobShinjo nobShinjo requested a review from TabishB as a code owner January 12, 2026 05:19
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

This PR fixes a PowerShell parser error by introducing a private helper method to strip trailing commas from array literals in generated completion scripts. The fix ensures syntactically valid PowerShell arrays without trailing commas that previously caused "Missing expression after ','" exceptions.

Changes

Cohort / File(s) Summary
PowerShell Generator Implementation
src/core/completions/generators/powershell-generator.ts
Added private helper method stripTrailingCommaFromLastLine() to remove trailing commas from the last line of PowerShell array literals. Method invoked after constructing command lines, flag entries, subcommand entries, and additional flag lines to ensure valid array syntax.
Test Coverage
test/core/completions/generators/powershell-generator.test.ts
Added test cases validating that generated PowerShell arrays do not contain trailing commas before closing parentheses. Tests ensure the fix prevents the parser error experienced in issue #482.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A comma at the end does cause such fright,
PowerShell arrays need their syntax right!
No trailing marks in @() we'll place,
Completion scripts now generate with grace! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing trailing commas from PowerShell array entries to fix syntax errors.
Linked Issues check ✅ Passed The PR successfully addresses issue #482 by implementing a helper method to strip trailing commas from array entries in PowerShell generation and adding regression tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to resolving issue #482: adding the stripTrailingCommaFromLastLine helper method and comprehensive tests for PowerShell array generation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

@vibe-kanban-cloud
Copy link

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
test/core/completions/generators/powershell-generator.test.ts (1)

447-473: Good regression test; consider extending coverage.

The test validates the fix for the reported issue with proper cross-platform line ending handling. However, the subcommand get has no flags, so the generateArgumentCompletion code path (line 159 in the generator) isn't directly exercised.

🔧 Optional: Add flags to the subcommand for fuller coverage
 subcommands: [
   {
     name: 'get',
     description: 'Get a config value',
-    flags: [],
+    flags: [
+      {
+        name: 'verbose',
+        description: 'Verbose output',
+      },
+    ],
   },
 ],
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36078b1 and 670ccb4.

📒 Files selected for processing (2)
  • src/core/completions/generators/powershell-generator.ts
  • test/core/completions/generators/powershell-generator.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/core/completions/generators/powershell-generator.test.ts (2)
src/core/completions/types.ts (1)
  • CommandDefinition (36-72)
scripts/postinstall.js (2)
  • script (81-81)
  • generator (80-80)
🔇 Additional comments (6)
src/core/completions/generators/powershell-generator.ts (5)

11-14: Clean implementation of the helper method.

The guard for empty arrays and the regex pattern correctly handle the trailing comma removal. The in-place mutation is appropriate here since all call sites own their arrays.


25-29: Correct placement for top-level commands.

The helper is called after all command entries are added, properly stripping the trailing comma from the last entry before joining.


87-98: Correct handling for parent command flags.

The helper is correctly placed after the flag entries loop, and the cmd.flags.length > 0 guard ensures entries exist.


110-114: Correct handling for subcommand entries.

Properly strips trailing comma from the last subcommand entry within the guarded block.


149-160: Correct handling for flags in argument completion.

This covers the code path for commands without subcommands, ensuring their flag arrays also have trailing commas stripped.

test/core/completions/generators/powershell-generator.test.ts (1)

1-3: LGTM.

Standard import formatting.

Copy link
Contributor

@TabishB TabishB left a comment

Choose a reason for hiding this comment

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

@nobShinjo Thank you for this fix 🫶

@TabishB TabishB merged commit 07dd634 into Fission-AI:main Jan 13, 2026
7 checks passed
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.

Windows Powershell Completions Exception: Missing expression after ','.

2 participants