Skip to content

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Jul 10, 2025

This change is using quartoConfig.cliPath() to get the quarto binary to use to be the same the one quarto dev-call was run.

Otherwise, the quarto use could be the one on PATH, which can be different that the one used (dev version vs released version if both installed), and also on windows we need quarto.exe or quarto.cmd depending on which is used. quarto on PATH won't always work without the extension.

and why we have this function for example
https://github.com/quarto-dev/quarto-cli/blob/fd54213c3e3f5fa929f6669b9ffb41547954f31c/tests/utils.ts#L192-L197

Opening the PR so that this is traced, and also for you to check this is ok to use that on Mac too.

@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented Jul 10, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@cderv cderv requested a review from cscheid July 10, 2025 11:55
@cscheid
Copy link
Collaborator

cscheid commented Jul 10, 2025

That's exactly correct, thank you. I now also reviewed the rest of our codebase for instances of "quarto" or 'quarto'. I think that after this fix, we use it correctly everywhere, although we have some lua duplication. In astpipeline.lua, we build the path correctly, but manually.

Copy link
Collaborator

@cscheid cscheid left a comment

Choose a reason for hiding this comment

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

👍

@cderv
Copy link
Collaborator Author

cderv commented Jul 10, 2025

In astpipeline.lua, we build the path correctly, but manually.

Oh interesting. We have a param for filter that we should probably use.

config = {
cli_path = function() return param('quarto-cli-path', nil) end,

I'll do another PR

@cderv cderv merged commit f91782d into main Jul 10, 2025
51 checks passed
@cderv cderv deleted the dev-call/windows/show-ast-trace branch July 10, 2025 12:42
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.

3 participants