Skip to content

Handle double-quoted commands#81

Merged
Malcolmnixon merged 4 commits intomainfrom
double-quoted-command-fix
May 7, 2026
Merged

Handle double-quoted commands#81
Malcolmnixon merged 4 commits intomainfrom
double-quoted-command-fix

Conversation

@Malcolmnixon
Copy link
Copy Markdown
Member

Pull Request

Description

This fixes handling of commands that are double-quoted to handle spaces in path names.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code quality improvement

Related Issues

Closes #

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

  • Code builds successfully and all tests pass: pwsh ./build.ps1
  • Code produces zero warnings

Code Quality

  • New code has appropriate XML documentation comments
  • Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

  • All linters pass: pwsh ./lint.ps1

Testing

  • Added unit tests for new functionality
  • Updated existing tests if behavior changed
  • All tests follow the AAA (Arrange, Act, Assert) pattern
  • Test coverage is maintained or improved

Documentation

  • Updated README.md (if applicable)
  • Updated docs/ documentation (if applicable)
  • Added code examples for new features (if applicable)
  • Updated requirements.yaml (if applicable)

Additional Notes

Copilot AI review requested due to automatic review settings May 7, 2026 14:39
@Malcolmnixon Malcolmnixon added the bug Something isn't working label May 7, 2026
Copy link
Copy Markdown
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

Fixes execution of configured tool commands that are wrapped in double-quotes (typically to handle executable paths containing spaces) by changing how the command is passed to the OS shell.

Changes:

  • Switched ProcessStartInfo from ArgumentList to string-based Arguments for invoking cmd.exe /c and /bin/sh -c.
  • Updated inline documentation/comments to explain the quoting motivation and behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs
Comment thread src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs Outdated
Comment thread src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs Outdated
Copilot AI review requested due to automatic review settings May 7, 2026 15:11
Copy link
Copy Markdown
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

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

Comment thread src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs
Comment thread docs/user_guide/introduction.md Outdated
@Malcolmnixon
Copy link
Copy Markdown
Member Author

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026

@copilot apply changes based on the comments in this thread

Implemented the requested thread updates in commit 9fb327c: added a Windows-only regression test for a double-quoted executable path plus quoted argument in VersionMarkConfigTests, and updated the user guide note to describe the actual /c "{command}" wrapping behavior.

@Malcolmnixon Malcolmnixon merged commit cbae34f into main May 7, 2026
15 checks passed
@Malcolmnixon Malcolmnixon deleted the double-quoted-command-fix branch May 7, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants