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

Update syntax-variables.less to include language entity colors #123

Conversation

savetheclocktower
Copy link
Contributor

Suppose you've written a package that makes some sort of editor enhancement. For various reasons, you'd really like to know what color is used to highlight variables in their syntax theme.

The good news is that Pulsar defines a list of variables in syntax-variables.less in each of the eight built-in themes which lists this token and others. (Amazingly, not all of them actually consume these variables in the rest of the stylesheets, so it may only be coincidental when the values line up; but B+ for effort.) Pulsar also defines some fallback values so that these values can at least be used consistently in a user's stylesheet file, and in any package stylesheet, without Less complaining about an undefined variable.

Somehow, though, these variables never made it to the template that PPM uses to generate new syntax themes. I've fixed that here. I've also taken the liberty of adding two variables — @syntax-color-comment and @syntax-color-string — that I can't believe were excluded from this list initially.

Will this make much of a difference when most community themes aren't actively maintained? Maybe not. But the best time to plant a tree was back in 2015 (when some other trees were planted in a different repository) and the second best time is now.

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

I trust the explanation in the PR body, and based on that, looks good to me!

Do we need to add @syntax-color-comment and @syntax-color-string in static/variables/syntax-variables.less file of Pulsar core repo as well? Would it break users' existing themes?

@savetheclocktower
Copy link
Contributor Author

We do need to add them, yes. I'll tackle them whenever.

There's no chance of breaking users' themes. The only things that would break are things (packages, etc.) that assume the presence of these variables without checking — and since lots of themes already don't have those variables (because they weren't in the generated template!), I can't imagine there are many of those.

@DeeDeeG DeeDeeG merged commit a570e91 into pulsar-edit:master Feb 1, 2024
11 checks passed
savetheclocktower added a commit to savetheclocktower/pulsar that referenced this pull request Feb 3, 2024
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