-
Notifications
You must be signed in to change notification settings - Fork 44
fix: protect code cell options from formatting #655
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
base: main
Are you sure you want to change the base?
fix: protect code cell options from formatting #655
Conversation
Thanks for the PR. I assume this was done to prevent |
Another thought here is that this kind of PR really makes me wish we had a test suite in this repo. |
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).
I'm not yet very familiar on this part, but I can't agree more with this. I did "manual tests" as far as this approach can go. |
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. |
@mcanouil we now have extension tests! If you're interested in merging this PR, want to add some tests? |
We are continuing to get more user reports of this such as in posit-dev/positron#9432, so I'd love to see a test written here so we can merge this fix in! |
I'll try to make some time to make this happen. |
…mcanouil/quarto into pr/mcanouil/655
I've setup the test, but I can't get to make the formatting command to work. npx @vscode/test-cli --grep "Code Block Formatting" --install-extensions "ms-python.black-formatter" I don't know how to trigger all the logic of the formatCellCommand The easiest would be to export I've also tried to trigger the whole document formatting, but it does not work for an unknown reason. I have to drop this for now, but if someone has a better understand of the whole formatting logic of the extension (not that trivial) and knows how to trigger the activation, feel free to take over. |
Ah, I don't think we can use any other extensions in the tests, because of the way that they use a special build of VS Code that is specially built for running extension tests. Instead, what we'll want to do is mock out a formatter to run in the tests. This is the same thing that #754 needs. We may want to put the formatting mock somewhere that all tests can use it. I also think that exporting a function to use in a test is a very good/reasonable way to go. |
Well, I don't think the issue is the presence/absence of the formatter. I can only make the test install the extension to be sure |
Ok, now I have some information. The vscode-test instance shows a notification that the selection marker is not within a cell that supports formatting which is clearly wrong, but no clue why
from yarn run test --grep "Code Block Formatting" |
For reference: In another PR I added a mocked cell formatter in a test here: 873fcab |
@vezwork Thanks, I'll try to take a look next week-end and see to finalise this PR. |
Even changing to your testFormatter function does not work. See when running extension debug mode: Screen.Recording.2025-10-19.at.19.10.58.movI have to drop this as It costs me way too much time for no good reasons. Feel free to take over as I won't spend more time on this for the next few months at least. |
Added subtitle with link to pull request for context.
Implement a mechanism to prevent code cell options from being formatted, ensuring that only relevant code is affected during formatting operations.