-
-
Notifications
You must be signed in to change notification settings - Fork 396
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
Add New Monokai Pro Theme #413
Conversation
I'm not so sure about atom-dark -- it seems too similar to |
It would be better to create separate PRs though. Did you based the themes on their official palettes? If I'm not wrong, Monokai was requested quite sometime ago so it would be a great addition. Also, precisely today we were discussing about how we didn't have any pitch black background themes so Xcode would be awesome to have also. |
Sure, I'll move doom-xcode to a separate PR. I based Monokai based on their official palette, except for the different background color which I set darker than what they use. I couldn't find an official palette for the xcode theme, so I used a tool to inspect xcode and pull the RGB values. |
@kadenbarlow I would think it's better to change Monokai's background to match the original one from the spec, just for the sake of keeping it as similar to the original as possible. I'll test it and use it to check all the faces work ok and that there's no contrast problems. Then if everything's ok I'll merge it! Thanks! |
@ema2159 Sounds great. I updated the theme to match the Monokai Pro background and made a couple of other small tweaks so the theme more closely matches Monokai Pro. https://monokai.pro/vscode. Let me know if any faces need tweaking and I can make changes. Thanks! |
Quick question. Where did you grabbed the colors? Or how did you probed them? |
On this site https://monokai.pro/. |
Hi. What about all the different flavors of monokai-pro? Like Spectrum, Ristretto and so one. I'd love to see them in doom as well. I could create PR's for them if you'd like me to. |
Go ahead! We're reviewing this current one so it may take some time |
Hi @ema2159 . I've created a first "beta". It's not ready for a PR yet. However, seems to be I am too stupid to even load the theme properly. I've added
into my This is what I get: After looking @ @kadenbarlow theme. I suspect it's not quite accurate. As far as I know every Monokai Pro flavor/filter only provides 6 colors (Apart from black to white of course): For example for the As far as I know every other color is just a darkened/lightened variant of the above, like this one: So I'm wondering where @kadenbarlow got his colors from especially Cheers. |
themes/doom-monokai-pro-theme.el
Outdated
(type light-blue) | ||
(strings yellow) | ||
(variables orange) | ||
(numbers yellow) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed
themes/doom-monokai-pro-theme.el
Outdated
(operators pink) | ||
(type light-blue) | ||
(strings yellow) | ||
(variables orange) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables should just be white. Function/method parameters are orange when declaring a function. But not variables itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes it's a little bit more difficult given that themes sometimes vary between modes (languages) and Emacs doesn't provide a robust API for that matter. I'll find some time to review this properly a the weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. However, at least for JS and PHP, Monokai Pro doesn't vary in regards to variables.
I think the main problem is that major modes don't support the depth of customization some themes require. I only have experience with php-mode
in regards to Monokai Pro. For example the concatenation operator (.
) can't be fontified with what php-mode
offers at the moment. I've actually submitted two PRs (594, 606). The latter is being reviewed right now. With those two PRs merged, flexibility with how php syntax can be fontified will be greatly improved.
However I don't know what the state of other major modes is.
@minikN You'll notice that red is used for the vc-deleted face, not for any of the syntax highlighting. I think that red is a good indicator of deleted lines in version control and green is good for additions in version control. Also, I agree with @ema2159 It can be hard to get a consistent look across different modes. I tried to get a good balance of matching Monokai and getting a good contrast between colors in the different modes that I tried out. |
Alright. That clarified it. Would you be able to give an example where Monokai Pro varies between different modes? At least for JS and PHP I haven't noticed that. Regardless, I would still recommend sticking to the original as close as possible. Monokai Pro uses So both your Reading through the your previous comments, it sound like you used a color picker to pick colors from the screenshots on https://www.monokai.pro ? If so, that might not be accurate. If I load the theme in VSCode and do |
@ema2159 Would you be able to help with that? |
@minikN of course, I'll just need sometime. I'm a little busy with my new job but as soon as I can I'll lend you a hand. |
@minikN wrt (package! doom-monokai-pro-spectrum-theme
:recipe (:host github :repo "minikN/doom-emacs-monokai-pro-spectrum")) There are two problems with the theme's definition:
Try removing the -(def-doom-theme monokai-pro-spectrum
+(def-doom-theme doom-monokai-pro-spectrum |
@minikN Oh, I forgot to mention a third issue: you need to add the theme's directory to ;;;###autoload
(add-to-list 'custom-theme-load-path (file-name-directory load-file-name)) So Emacs knows where to find it. |
Thank you @hlissner. I will try this tonight. Is there any guideline as to how specific a theme should be? I'm talking about mode-specific faces. I have a local repo for Monokai Pro in which I've added several new faces to PHP mode based on my PR:
So my question is whether or not there is a threshold as to how fine grained a theme should be. Thanks. |
It can be as fine grained as you like, so long as you avoid properties that could throw errors, e.g. That said, I wasn't aware of php-mode's faces, perhaps they should be added to doom's base theme as well, so all themes can provide rudimentary support for the mode. |
Thank you @hlissner . Well as I said, https://github.com/emacs-php/php-mode/pull/594 is merged already, however https://github.com/emacs-php/php-mode/pull/606 is still in review. Unfortunately, I'm still stuck with loading the theme. I applied the changes you mentioned. If I now repace |
@minikN That likely means ;;;###autoload
(when (and (boundp 'custom-theme-load-path) load-file-name)
(add-to-list 'custom-theme-load-path
(file-name-as-directory (file-name-directory load-file-name)))) |
This worked. Thanks! |
I improved the palette and theming overall. It should look almost the same as VS Code's. |
@ema2159 thanks for making those changes. Sorry been super busy at work the past week or so. |
@kadenbarlow don't worry. Thanks for contributing! |
I'm pretty new to the emacs community, but started using Doom recently (introduced to it by Doom Themes) and have loved it. I created these three themes for myself that I rotate through depending on my mood. Wondering if any of these are worth adding in?
doom-monokai-pro
edit: removed other two themes