Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 2, 2025

  • Understand the issue: Add named parameter support to slash commands
  • Implement named parameter parsing
  • Fix: Named parameters now count toward positional numbering (per @alexec's feedback)
  • Add tests for edge cases (duplicates, reserved keys)
  • Make double quotes mandatory for named parameters (per @alexec's feedback)
  • Simplify numeric key check using strconv.Atoi (per @alexec's feedback)
  • Simplify parseBashArgsWithNamed return type to map[string]string (per @alexec's feedback)
  • Remove protection for reserved keys (per @alexec's feedback)
  • Update documentation
  • Run linter and tests - all pass
  • Run codeql - no security issues
Original prompt

We currently support positional parameteres in slash commands, I want to also support named parameters. Ideas?


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits December 2, 2025 02:14
Extended slash command parsing to support named parameters in key="value" format
alongside existing positional parameters. Named parameters can be mixed with
positional arguments and do not count toward positional numbering.

Examples:
- /fix-bug issue="PROJ-123" -> ${issue} = "PROJ-123"
- /task arg1 key="value" arg2 -> ${1}="arg1", ${2}="arg2", ${key}="value"

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copilot AI changed the title [WIP] Add support for named parameters in slash commands Add named parameter support to slash commands Dec 2, 2025
Copilot AI requested a review from alexec December 2, 2025 02:19
@alexec alexec requested a review from Copilot December 2, 2025 03:41
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 adds named parameter support to slash commands, allowing users to specify parameters using key="value" syntax in addition to existing positional parameters. The implementation extends the argument parsing logic to detect and extract named parameters while maintaining backward compatibility with positional arguments.

Key Changes

  • Added parseBashArgsWithNamed function to parse both positional and named parameters from argument strings
  • Added parseNamedParam helper to identify arguments in key=value format
  • Extended test coverage with 8 new test cases covering various named parameter scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
pkg/codingcontext/slashcommand.go Implemented named parameter parsing logic with parseBashArgsWithNamed and parseNamedParam functions; updated documentation comments to describe new syntax
pkg/codingcontext/slashcommand_test.go Added comprehensive test cases for named parameters including spaces in values, escaped quotes, mixed usage with positional args, and special characters
docs/reference/cli.md Documented named parameter syntax with examples showing usage patterns and equivalents to manual -p flag usage

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@alexec
Copy link
Contributor

alexec commented Dec 2, 2025

@copilot positional arguments should retain their numbering:

/fix-bug arg1 issue="PROJ-123" arg2 priority="high"

lts in:

  • ${1} = arg1
  • ${2} = issue="PROJ-123"
  • ${3} = arg
  • ${ARGUMENTS} = arg1 issue="PROJ-123" arg2 priority="high"
  • ${issue} = PROJ-123
  • ${priority} = high

@alexec
Copy link
Contributor

alexec commented Dec 2, 2025

@copilot make quotes mandatory. You can extract the same parser we used for -p.

Updated slash command parsing so that named parameters (key="value") are also
included as positional arguments in their original form. This addresses the
feedback that `/task arg1 issue="PROJ-123" arg2` should result in:
- ${1} = arg1
- ${2} = issue="PROJ-123"
- ${3} = arg2
- ${issue} = PROJ-123

Also added protections:
- Reserved key ARGUMENTS cannot be used as a named parameter key
- Numeric keys (1, 2, etc.) cannot be used as named parameter keys
- Duplicate named parameter keys: last value wins (documented behavior)

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 2, 2025

@copilot positional arguments should retain their numbering:

...

Fixed in ee87d79. Named parameters now count toward positional numbering. /fix-bug arg1 issue="PROJ-123" arg2 priority="high" now results in:

  • ${1} = arg1
  • ${2} = issue="PROJ-123"
  • ${3} = arg2
  • ${4} = priority="high"
  • ${ARGUMENTS} = full string
  • ${issue} = PROJ-123
  • ${priority} = high

Also added protections for reserved keys (ARGUMENTS, numeric keys).

Per @alexec's feedback, named parameters now require mandatory double quotes.
- Unquoted values like `key=value` are treated as positional arguments
- Single-quoted values like `key='value'` are also treated as positional arguments
- Only `key="value"` format is recognized as a named parameter

Updated parseNamedParamWithQuotes to validate double quote requirement.
Added tests for unquoted and single-quoted edge cases.
Updated documentation to clarify mandatory double quotes.

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 2, 2025

@copilot make quotes mandatory. You can extract the same parser we used for -p.

Fixed in 86b2502. Double quotes are now mandatory for named parameters. key=value and key='value' are now treated as positional arguments, only key="value" is recognized as a named parameter.

Replaced manual character-by-character numeric check with strconv.Atoi
for cleaner, more idiomatic Go code.

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copilot AI requested a review from alexec December 2, 2025 04:13
Changed return type from ([]string, map[string]string, error) to
(map[string]string, error). The map now contains both positional keys
("1", "2", "3", etc.) and named parameter keys, eliminating the need
for a separate allArgs slice.

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
@alexec alexec closed this Dec 2, 2025
@alexec alexec reopened this Dec 2, 2025
@alexec alexec marked this pull request as ready for review December 2, 2025 04:29
@alexec
Copy link
Contributor

alexec commented Dec 2, 2025

@copilot don't worry about protecting numeric arguments or ARGUMENTS. A user would have to misuse the system greatly for this to happen.

Per @alexec's feedback, removed the validation that prevented named
parameters from using ARGUMENTS or numeric keys. Users would have to
misuse the system greatly for this to cause problems.

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 2, 2025

@copilot don't worry about protecting numeric arguments or ARGUMENTS. A user would have to misuse the system greatly for this to happen.

Removed in a07097e. The protections for ARGUMENTS and numeric keys have been removed. Named parameters can now use any key.

@alexec alexec enabled auto-merge (squash) December 2, 2025 05:04
@alexec alexec merged commit 511ce5a into main Dec 2, 2025
1 check passed
@alexec alexec deleted the copilot/add-named-parameter-support branch December 2, 2025 05:05
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.

2 participants