Skip to content

CmdPal: Rename idappUserModelId in LaunchProfileCommand to remove ambiguity#46481

Merged
michaeljolley merged 2 commits intoniels9001/cmdpal-terminal-dock-pinningfrom
copilot/sub-pr-46372
Mar 24, 2026
Merged

CmdPal: Rename idappUserModelId in LaunchProfileCommand to remove ambiguity#46481
michaeljolley merged 2 commits intoniels9001/cmdpal-terminal-dock-pinningfrom
copilot/sub-pr-46372

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

Summary of the Pull Request

LaunchProfileCommand had a constructor parameter id and backing field _id that held the Terminal AppUserModelId used for COM activation, which was ambiguous given the base class Id property (set to a generated command identifier via MakeId). Renames the parameter and field throughout to appUserModelId/_appUserModelId.

PR Checklist

  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Addresses review feedback on #46372. Changes in LaunchProfileCommand.cs:

  • _id field → _appUserModelId
  • Constructor parameter idappUserModelId
  • Private Launch(string id, ...)Launch(string appUserModelId, ...)

Before, both the activation ID and the command identity used id-derived names, making it easy to accidentally pass the wrong value. The MakeId static method already used appUserModelId as its parameter; this aligns the rest of the class with that convention.

// Before
private readonly string _id;
internal LaunchProfileCommand(string id, ...) { this._id = id; this.Id = MakeId(id, profile); }

// After
private readonly string _appUserModelId;
internal LaunchProfileCommand(string appUserModelId, ...) { this._appUserModelId = appUserModelId; this.Id = MakeId(appUserModelId, profile); }

Validation Steps Performed

  • Code review: no issues flagged.
  • Caller site (ProfilesListPage.cs) already passes profile.Terminal.AppUserModelId — no functional change.

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI changed the title [WIP] [WIP] Address feedback on CmdPal dock pinning and profile icons implementation CmdPal: Rename idappUserModelId in LaunchProfileCommand to remove ambiguity Mar 24, 2026
Copilot AI requested a review from michaeljolley March 24, 2026 19:44
@michaeljolley michaeljolley marked this pull request as ready for review March 24, 2026 19:52
@michaeljolley michaeljolley merged commit 9250c82 into niels9001/cmdpal-terminal-dock-pinning Mar 24, 2026
12 checks passed
@michaeljolley michaeljolley deleted the copilot/sub-pr-46372 branch March 24, 2026 20: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