Update Coldark Themes

[ArmandPhilippot]

Coldark recently updated its colors for better readability. So the colors of both versions have been updated. Comments are no longer italic. Also, Markdown was missing block quote and meta separator colors. And some tokens were not using the correct color.

I recreated the screenshots. Everything is ok when running npm test.

[RunDevelopment]

Wow. Travis queued for 1h and 20min until it finally ran the tests.
@mAAdhaTTah Maybe we should switch to GH actions over here too?

[ArmandPhilippot]

It’s only me who’s bad luck with automated testing. 👍

Another question (I don't know if it's worth creating an issue for that):
Wouldn't it be easier to keep the themes up to date using submodules? Or with automated tests it can't work? (I'm still new to pull request / submodules / automated tests so maybe the submodules are not compatible ...)

[RunDevelopment]

Themes rarely change over here. That may be because authors actually rarely update their themes or because they just don't forward these changes. Idk.

However, I'm skeptical that submodules are the solution. I get why submodules might seem like a good solution but they also have downsides. One being that submodules are not easy to work with in general.

The big problem with submodules for our specific use case is control. Right now, the PrismJS team is in full control. We can manage and change everything. But with submodules, we essentially have to trust the good-will of whoever owns the linked repo.
This is concerning from a security aspect. A malicious attacker could just make a repo with a new theme, make PR, get merged, and then add malicious code to the repo. We'd have to make our build process smart enough to notice something like this or else our next release might compromise the systems of thousands.

From what I see, using submodules brings us a lot of risk and development cost for relatively little benefit.

---

Hmmm, but I see the issue. Maybe we could automate the PR process? We could link the authors' repos and make a bot periodically check whether themes changed. If something changed, it would make PR and ping the author to ask for permission.

But writing that bot will be a lot of work too... Probably not worth it.

[mAAdhaTTah]

I don't think there'd be a security issue because submodules are committed against a specific commit in the target repo. I suppose you could swap out what's under that commit, but I don't know how possible/difficult that is.

But there's a better reason to reject them: they're an absolute nightmare to work with. I used them on a project once and they were terrible.

I'm also not 100% sure what we're trying to keep in sync here. Is there another repository of prism themes we should be pulling from? Otherwise, I think there's some manual translation process involved in converting e.g. a VSCode theme into a Prism theme that I don't know how possible/valuable automation would be.

---

100% on switching to GitHub actions here.

[RunDevelopment]

I'm also not 100% sure what we're trying to keep in sync here. Is there another repository of prism themes we should be pulling from?

Many artists give their Prism themes their own repository. Example. The problem is that themes might be updated in the artist's repo but not here.

[ArmandPhilippot]

@RunDevelopment Thank you for this detailed explanation. I suspected that if the submodules were not used it was for a good reason, in particular safety and compatibility with automated tests. However, I didn't think malicious code could be injected this way. As I said, I lack experience on this side, so I preferred to ask the question.

The bot would indeed be an alternative. However, if as you said, the themes change little, it is indeed a lot of work for little. For my part, I don't mind making a pull request to update. It was a simple suggestion and I understand better why you are not using them.

@mAAdhaTTah I'm not aware of another repository of prism themes. I was referring to a repo dedicated to a theme like me that uses this repo to update the Prism version, then I do a pull request by creating a commit on the fork. I was thinking about submodules after doing a pull request on the bat repo. Each theme has its own repo and they are linked to the bat repo with the submodules.

[mAAdhaTTah]

To be honest, if the theme is available in another repo, it might be better to have that repo be the canonical source for it and remove it from this repo. We could advertise it in Prism, either via links on the website or README, but not having it in two places in the first would eliminate the need to do any syncing.

This isn't intended to suggest we'd "reject" your theme or anything; only that if syncing is an issue we'd want to address, that seems like the easiest solution. Otherwise, I don't mind just doing a manual PR as needed. As @RunDevelopment said, themes don't really change that much. I can imagine the work involved in crafting a solution being greater than the amount of time saved.

[ArmandPhilippot]

Like I said, doing a pull request doesn't bother me. I was thinking of this "solution" for all the themes.

As @RunDevelopment said, "or because they just don't forward these changes": that could have avoided this problem. That said, it does not seem to be so many complaining that a theme is not up to date.

Regarding the link rather than adding to the repo, I don't know. In my case, if I added it here it was to give it more visibility. I figured that Prism users were more likely to view this repo than to look for different repos on Github. So it's up to you to see what looks best. Again, the pull request is not a problem; it was more of a suggestion to automate the process.

Now, I understand your reservations. Thanks for taking the time to explain them. 👍