Skip to content

Fix quoted paths in options strings#71

Merged
ChrisChinchilla merged 2 commits into
mainfrom
70-execfile-migration-breaks-quoted-paths-in-option-strings-eg---reference-doc
Apr 8, 2026
Merged

Fix quoted paths in options strings#71
ChrisChinchilla merged 2 commits into
mainfrom
70-execfile-migration-breaks-quoted-paths-in-option-strings-eg---reference-doc

Conversation

@ChrisChinchilla

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

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 improves how user-configured option strings (Pandoc options and Docker options) are converted into argument arrays by adding a quote-aware parser, so paths/options containing spaces can be passed correctly without shell interpolation.

Changes:

  • Added parseShellArgs() to split option strings while preserving quoted substrings.
  • Updated Pandoc and Docker option handling to use parseShellArgs() instead of whitespace splitting.
  • Updated the built dist/extension.js to reflect the new parsing behavior.

Reviewed changes

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

File Description
src/extension.ts Introduces parseShellArgs() and uses it when expanding pandocOptions / dockerOptions into argv.
dist/extension.js Compiled output updated to include the new parser and updated argv building.

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

Comment thread src/extension.ts Outdated
Comment thread src/extension.ts
Comment on lines +16 to +40
let i = 0;
while (i < input.length) {
const ch = input[i];
if (ch === '"' || ch === "'") {
const quote = ch;
i++;
while (i < input.length && input[i] !== quote) {
current += input[i];
i++;
}
i++; // skip closing quote
} else if (/\s/.test(ch)) {
if (current.length > 0) {
args.push(current);
current = "";
}
i++;
} else {
current += ch;
i++;
}
}
if (current.length > 0) {
args.push(current);
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

The new argument-splitting behavior is central to this change (quoted paths/options), but there are no tests asserting the resulting argv. Please add unit/integration tests that cover common cases like --resource-path="/a b", mixed quoting (e.g. --opt=foo"bar baz"), and Docker options containing quoted values, to prevent regressions across platforms.

Suggested change
let i = 0;
while (i < input.length) {
const ch = input[i];
if (ch === '"' || ch === "'") {
const quote = ch;
i++;
while (i < input.length && input[i] !== quote) {
current += input[i];
i++;
}
i++; // skip closing quote
} else if (/\s/.test(ch)) {
if (current.length > 0) {
args.push(current);
current = "";
}
i++;
} else {
current += ch;
i++;
}
}
if (current.length > 0) {
args.push(current);
}
let inSingleQuote = false;
let inDoubleQuote = false;
let tokenStarted = false;
for (let i = 0; i < input.length; i++) {
const ch = input[i];
if (ch === "\\" && !inSingleQuote) {
const next = input[i + 1];
if (next !== undefined) {
current += next;
tokenStarted = true;
i++;
} else {
current += ch;
tokenStarted = true;
}
continue;
}
if (ch === "'" && !inDoubleQuote) {
inSingleQuote = !inSingleQuote;
tokenStarted = true;
continue;
}
if (ch === '"' && !inSingleQuote) {
inDoubleQuote = !inDoubleQuote;
tokenStarted = true;
continue;
}
if (/\s/.test(ch) && !inSingleQuote && !inDoubleQuote) {
if (tokenStarted) {
args.push(current);
current = "";
tokenStarted = false;
}
continue;
}
current += ch;
tokenStarted = true;
}
if (tokenStarted) {
args.push(current);
}

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Repository owner deleted a comment from promptless Bot Apr 8, 2026
@ChrisChinchilla
ChrisChinchilla merged commit c6c7624 into main Apr 8, 2026
4 of 11 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.

2 participants