Add option to use theme provided cell background (#675)

[kylebutts]

Adds option cells.background.useTheme that when true will use the theme's built in cell background.

Positron Light:

Gruvbox Light Soft:

[juliasilge]

The new .vsix is available here (download artifact, unzip, install VSIX):

https://github.com/quarto-dev/quarto/actions/runs/13730481250

[juliasilge]

Thank you so much for this PR @kylebutts! 🙌

We are unfortunately getting to some dependencies between settings here, like the problem outlined at:
https://design.tidyverse.org/independent-meaning.html

The settings with the bad dependencies between them here are:

• quarto.cells.background.enabled
• the two colors quarto.cells.background.light and quarto.cells.background.dark
• the new quarto.cells.background.useTheme

If we were starting out from scratch, I would want to make the setting called quarto.cells.background.enabled an enum (find "enum / enumDescriptions / markdownEnumDescriptions / enumItemLabels" here) with options:

• "default" (maybe then change the colors to quarto.cells.background.darkDefault and quarto.cells.background.lightDefault)
• "useTheme"
• "off"

If we make that change now, it would be breaking for people who have set quarto.cells.background.enabled to false. I would imagine this is a very small number of people (no one has checked this setting in to GitHub at least) and I personally would be willing to make such a breaking change.

@cscheid What do you think? I think this is a very good change in this PR, but where do you land on the relative badness of a breaking change in settings vs. confusing interaction of settings?

[juliasilge]

Suggested change

	"type": "string",
	"format": "color",
	"type": "boolean",

This is more like true vs. false, not a string formatted as a color, like what is show in this example:
https://code.visualstudio.com/api/references/contribution-points#Configuration-example

An example in this file is "quarto.cells.background.enabled".

[juliasilge]

Suggested change

	"markdownDescription": "Should the background of executable code cells be based on the theme provided color.\n\nThis uses the `notebook.selectedCellBackground` color from the VSCode theme."
	"markdownDescription": "Use the `notebook.selectedCellBackground` color from the current theme as the background of executable code cells."

[kylebutts]

Great, thanks for the feedback @juliasilge. I'll wait for feedback on your question and will either implement these changes or you suggested quarto.cells.background.enabled enumeration idea.

[cscheid]

@cscheid What do you think? I think this is a very good change in this PR, but where do you land on the relative badness of a breaking change in settings vs. confusing interaction of settings?

My personal philosophy wrt breaking changes.

• if we're going to break things, we should do it in bigger chunks. It's better to do bigger changes fewer times than smaller changes more times. The downside of breakage is that we inflict it on users, and so we should try to minimize the number of times they need to respond to our changes.
• The later we make breaking changes, the more I'm inclined to try and keep the previous functionality in. Fewer users => higher willingness to make the change

If I understand it right, not a lot of users appear to be exposed to the breakage here. If so, I would go ahead and make a breaking change.

With that said, the specific change towards making enabled an enum would confuse me, I think. I would expect an enabled field to always support booleans true and false.

[juliasilge]

How about this set of settings?

• quarto.cells.background, an enum with options "default", "useTheme", and "off"
• quarto.cells.background.darkDefault and quarto.cells.background.lightDefault, which get ignored unless quarto.cells.background is "default"

[kylebutts]

Crud, I messed up the commit when rebasing. I can open a new PR if you want.

But, I have implemented what @juliasilge recommended

[juliasilge]

This is looking good, @kylebutts! 🙌 Thank you so much for iterating on this with us.

It looks like something hasn't quite gone right and we have lost the defaults for quarto.cells.background.delay and the default background colors:

Can you take a look? It's possible it's the whitespace changes on 973 and 974? 🤷‍♀️

Also, can you make an update in CHANGELOG.md?

And yeah, it looks like 0e61dc2 got applied as a new, separate commit with the same changes as e268ba4, so if you could revert that commit or start over or similar, that would be fantastic. Thank you again for your perseverance here!

[kylebutts]

@juliasilge I think what's happening is that quarto.cells.background makes further . arguments (like quarto.cells.background.delay) not work. What do you think about quarto.cells.background.display as the option name for the choice of default/useTheme/off?

[juliasilge]

Ah gotcha! Yes, let's do quarto.cells.background.display or quarto.cells.background.color.

[kylebutts]

Moved to #679