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

Less should be able to communicate with our config system #1093

Open
1 task done
savetheclocktower opened this issue Sep 13, 2024 · 2 comments
Open
1 task done

Less should be able to communicate with our config system #1093

savetheclocktower opened this issue Sep 13, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@savetheclocktower
Copy link
Contributor

savetheclocktower commented Sep 13, 2024

Have you checked for existing feature requests?

  • Completed

Summary

Let me try to both expand and summarize what I said here.

When Pulsar upgraded to a recent version of Less last year, it enabled a new style of plugin. Less plugins can be declared within a given scope, then used in that scope:

// utilities.js
function config (keyPath) {
  return atom.config.get(keyPath.value);
}

module.exports = {
  install: function(_less, _pluginManager, functions) {
    functions.add('config', config);
  }
};
@plugin "utilities.js";

body {
  font-size: config('some-theme.fontSize');
}

This is the simplest possible version of this idea. I find it very compelling, especially compared to the hacky things that I've seen themes do (and done myself!) in order to make themes more configurable. (When a setting changes, regenerate a file on the fly that defines some variables, then disable and re-enable the package!)

This approach can even be extended to allow the user to define fallback values or to combine bare numeric values with units:

@plugin "utilities.js";

body {
  font-size: config('some-package.fontSize', 13px);
  line-height: config('some-package.lineHeight', 1, 'rem');
}

This is the easy part.

The hard part is how frequently that function would get called in the course of stylesheet generation.


Suppose we wanted to make @font-size configurable in a UI theme. That's a great idea, since practically all UI text uses (or defines itself relative to) the value of @font-size — and it can't easily be changed from a user stylesheet.

If a theme defined a line like this in its ui-variables.scss

@font-size: config('some-package.fontSize', 13px);

…it'd be defining it in a file that is requested by practically every non-editor stylesheet in the app. There's a strong expectation that importing ui-variables.less is like importing a flat file of dumb exports.

That's because the style sheets are evaluated in a Pulsar context, but each compiled stylesheet encounters its own reference to utilities.js, and loads that file fresh each time. This makes it currently impossible to (e.g.) cache config lookups on the theory that they won't change during a stylesheet regeneration — you can try, but the cache will get wiped away when Less is done building that file.

What benefits does this feature provide?

Elegance! No longer having to use style hacks! Perhaps even an emerging convention about what should be configurable in a UI theme. #1087 was useful for illustrating how weird it is that we have a setting for editor font size, but not for the chrome around the editor.

Any alternatives?

Suppose I change a package's font size via a setting. That package has to regenerate all its stylesheets to reflect the new @font-size variable. Then all the people that depend on those stylesheets have to adjust their stylesheets. Over the course of this process, ui-variables.less is included dozens of times, and those calls can't practically share state with one another. (Unless I've overlooked something; it happens pretty regularly!)

To elaborate: suppose your custom UI theme had a ui-variables.less that looked like this, in part:

@plugin "utilities.js";

@font-size: config('some-package.fontSize', 13px);

If you were to change the value of some-package.fontSize, you'd want all the stylesheets that rely on @font-size to regenerate. Practically speaking, that's all stylesheets. And each time one of those stylesheets imports ui-variables, utilities.js gets a fresh evaluation that does not share state with any of the previous times that it's been evaluated.

If you were to make your config function slightly smarter so that it could cache config value lookups, your cache would only be useful if the same .less file looked up the same value more than once.

It would be great if we could find a way around this! Ideally, the process of regenerating a series of editor stylesheets would be formalized into something that could share state across the entire task.

I don't know how to pull that off, but it'd be nice.


In this particular case, there's another possibility: it's hard to ignore the fact that Less's variable system is getting in the way here. Hypothetically, if UI themes used CSS variables, none of this would be a problem:

:root {
  --font-size: 13px;
}

// Some other package
.some-status-bar-item {
  font-size: calc(var(--font-size, 13px) * 0.8);
}

// User stylesheet override
:root {
  --font-size: 15px;
}

With CSS variables, we get all the things we want: math operations, fallbacks to defaults, late binding, easy redefinition without having to rebuild stylesheets or even keep track of stylesheet dependencies.

It's probably not possible to do this in a backward-compatible way, but I'd love it if instead of

@font-size: 13px;

we could do

:root {
  --font-size: 13px;
}

@font-size: var(--font-size);

// Some other package
.some-status-bar-item {
  font-size: (@font-size * 0.8);
  // Magically translated to…
  // font-size: calc(var(--font-size) * 0.8);
}

This almost works! But Less won't allow me to do math operations on var(--font-size). I could, however, envision a mode in which Less assumes I know what I'm doing and, knowing that @font-size is a CSS variable, converts
(@font-size * 0.8) into calc(var(--font-size) * 0.8). It wouldn't know if that was valid, but that's the point: by making @font-size refer to a CSS variable, I'm opting into runtime resolution of that binary expression.

This wouldn't be “elegant” or anything — I'm just trying to paint us out of the corner we're in. If we were inventing UI themes from scratch now, we wouldn't need Less very much! The only thing Less is better at is manipulation of color variables — darken @x by 10%, like @y but with alpha of 0.5 — and we should be getting that in CSS pretty soon.

@savetheclocktower savetheclocktower added the enhancement New feature or request label Sep 13, 2024
@savetheclocktower savetheclocktower changed the title Less should be able to communicate with our config syste, Less should be able to communicate with our config system Sep 13, 2024
@savetheclocktower
Copy link
Contributor Author

There are related issues here and I've mentioned some of them in Discord:

  1. Our Less compilation strategy assumes that it doesn't have to recompile a file if its contents haven't changed. But a config function throws a wrench in that assumption! And it’s not easy to get Pulsar to recompile a stylesheet that it thinks hasn't changed.

  2. Less is designed to be late binding, and brags about it:

    @font-size: 12px;
    p { font-size: @font-size; }
    @font-size: 14px;
    
    // Results in CSS:
    p { font-size: 14px; }

    This works because Less does two passes; it resolves all variable references before it produces any output. So you'd think that you could just redefine @font-size in the user stylesheet and have it reflected in all stylesheets, right? Presumably that would work if we assembled all stylesheets and processed them in a batch somehow — but we don't. Each separate package has its styles processed in isolation from others.

    If we could somehow change this, it would solve many of the problems listed above, but also create some new problems. We'd have to make sure that one badly-written stylesheet wouldn't wreck the styles for the entire editor, and we'd also have to make sure that a variable defined in one stylesheet doesn't leak into a different stylesheet from a different package. (I'm sure that lots of stylesheets incidentally use the same local variable names and don't expect them to clash with one another.) I haven't looked into it yet, but I'm skeptical that we could thread that needle.

@savetheclocktower
Copy link
Contributor Author

The good news: I managed to write a Less plugin that largely allows someone to perform math on @foo when @foo is itself assigned to a CSS variable (like var(--foo)).

@font-size: 13px;
h1 { font-size: (@font-size * 2); }

// Produces:
h1 { font-size: 26px; }

// ---

@font-size-var: var(--font-size);
h1 { font-size: (@font-size-var * 2); }

// Produces:
h1 { font-size: calc(var(--font-size) * 2); }

In other words, it turns compile-time evaluations into runtime evaluations.

The bad news is that there are a number of math functions — round, floor, ceil, mod, and some trigonometric functions — that also operate on numbers, and they're used in several places in various .less files. To translate these to runtime evaluations would require that equivalent functions exist in CSS, and they don't in Pulsar.

But they will eventually! These functions were added to the CSS spec and are present in Chrome 125, which means they'll be available to us when we're able to upgrade to the latest Electron.

This may still be a very silly thing to do, but we could at least play around with it and see if it works. One day we won't need Less variables anymore, and this may be one way we can gently migrate old stylesheets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant