Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: protect code cell options from formatting #655

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mcanouil
Copy link
Contributor

@mcanouil mcanouil commented Feb 8, 2025

Implement a mechanism to prevent code cell options from being formatted, ensuring that only relevant code is affected during formatting operations.

@mcanouil mcanouil marked this pull request as ready for review February 9, 2025 15:13
@cscheid
Copy link
Contributor

cscheid commented Feb 21, 2025

Thanks for the PR.

I assume this was done to prevent black and other Python auto-formatting tools from mangling our #| syntax. Is that the case? If so, I wonder if a more targeted, Python-only fix is better.

@cscheid
Copy link
Contributor

cscheid commented Feb 21, 2025

Another thought here is that this kind of PR really makes me wish we had a test suite in this repo.

@mcanouil
Copy link
Contributor Author

mcanouil commented Feb 21, 2025

I assume this was done to prevent black and other Python auto-formatting tools from mangling our #| syntax. Is that the case? If so, I wonder if a more targeted, Python-only fix is better.

I was thinking about making it specific at first but then I realised that the expected comments are all hardcoded, so I reconsidered, and made a general protection for the comments symbols (hardcoded in two places).

Another thought here is that this kind of PR really makes me wish we had a test suite in this repo.

I'm not yet very familiar on this part, but I can't agree more with this.
That's on my personal roadmap before releasing Quarto Wizard 1.0.0.

I did "manual tests" as far as this approach can go.

@mcanouil
Copy link
Contributor Author

Let's hold on on this PR as the behaviour is not new at all so it can wait a bit for me to implement a test soon as I get some spare time.

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