search

braver / vscode-solarized

Solarized for VS Code
MIT License
32 stars 10 forks source link

added workbench theming #4

Closed DavidRGriswold closed 6 years ago

DavidRGriswold commented 6 years ago

Hi! I went ahead and added workbench theming in a fork, copying the default solarized dark and light workbench themes. I think it would be long-term better to convert the the tmTheme file to a JSON entirely (there's an npm tool to do it) rather than importing it as I did here, but this was quick and dirty and works, and will allow you to continue updating in textmate mode if you want to add other languages or whatever.

3

braver commented 6 years ago

Looks really nice actually.

Can you chose to use the default workbench/UI theme with Solarized just for the code?

DavidRGriswold commented 6 years ago

Hmm. I've only been using vscode for about three days so I don't know how to add options to an extension yet, but maybe I can poke around with it for a while and try to figure it out. Something like...

braver.solarized.theme.workbench: true (or false)

where true chooses to use the solarized workbench theme while false uses the standard dark/light from vs/vs-dark?

braver commented 6 years ago

I have no idea how other themes do it. There should be examples out there right?

DavidRGriswold commented 6 years ago

Hmm. I looked at several examples of themes of various kinds, and it looks like the standard practice is just to include multiple theme names. So instead of including two names you could include four:

If you want me to do that, I'm happy to - would only require adding a few lines to the package.json

Disadvantage over a setting that this clutters up the color theme area for the user, but given that I found theme packs with 30 or more themes (!!!) this may not be a big deal. To do it programmatically looks like it would potentially require repackaging as a more complete extension, with some javascript or typescript code.

braver commented 6 years ago

That sounds like a pretty reasonable solution. Like you say, it could be worse. I think for compatibility the existing themes should keep their names, and we label the new themes with workbench color "... (themed workbench)".

I'd also like to have commented lines like // "foreground": "", removed from the code before merging.

DavidRGriswold commented 6 years ago

Hmm. Playing around with this, I realized that my workbench theming was actually overriding some of your colors (highlighted text colors, for example). Fixing it will take a little more work than anticipated, but I do know what to do. How would you feel about my converting the tmTheme files to json files so they are in native VSCode format?

braver commented 6 years ago

Sure, go for it. SublimeText will get a new format soon too, so the original themes will end up changing and being incompatible with VS Code anyway.

DavidRGriswold commented 6 years ago

Okay, that took more work than expected, but it seems to be in a good place. Near as I can tell the workbench now perfectly matches the built in Solarized Dark one and the editor matches perfectly your original editor. The only exception is the 'Find Highlight" areas look slightly different on Solarized light for some reason I can't figure out, but it's a minor difference. I think it may have to do with two different colors somehow mixing with another highlight option in a different way, but I'm not certain.

I DID leave the comments in the JSON files. I may want to tweak some of those options for myself later, and leaving the highlighted ones in there is helpful for me. If you want to pull them out before merging go for it. If you decide not to merge, maybe I'll just release the themed ones as a new theme in the VSCode repository, with credit of course.

braver commented 6 years ago

Pretty sweet! How about I just give you access to this repo so you can just tweak something and release it if you want?

DavidRGriswold commented 6 years ago

That works! Might be a day or two at least have to focus on other things.

On Jan 17, 2018 3:05 AM, "Koen Lageveen" notifications@github.com wrote:

Pretty sweet! How about I just give you access to this repo so you can just tweak something and release it if you want?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/braver/vscode-solarized/pull/4#issuecomment-358240811, or mute the thread https://github.com/notifications/unsubscribe-auth/ATpU7yk0azC_W6QpOsGskm65HTTl9OXQks5tLbfUgaJpZM4Re5Dw .

braver commented 6 years ago

Sure, take your time.

I'm not sure you can do releases now too, I'd really need to check how that works and if I got a key for it somewhere. But at least you can now merge this at will and let me know when you're ready for release and then I'll figure that out :)

DavidRGriswold commented 6 years ago

Okay! All merged in. I updated the changelog and the readme, but feel free to edit them obviously. I do think you have to publish - it looks like it needs a private access token, and vscode marketplace is all done through microsoft accounts apparently, so this github thing definitely doesn't connect.

braver commented 6 years ago

Yeah, I think that worked. Thanks man! If I need to publish anything else later just let me know. I think I may be able to set it up so you can publish too, but the sites to manage this are the most confusing mess I've ever seen.

DavidRGriswold commented 6 years ago

Not a problem! Color themes don't need a lot of updating so should be good for a while.

On Jan 20, 2018 10:54 AM, "Koen Lageveen" notifications@github.com wrote:

Yeah, I think that worked. Thanks man! If I need to publish anything else later just let me know. I think I may be able to set it up so you can publish too, but the sites to manage this are the most confusing mess I've ever seen.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/braver/vscode-solarized/pull/4#issuecomment-359185230, or mute the thread https://github.com/notifications/unsubscribe-auth/ATpU7_NNyljN1Y0jI6ayH5FLgQJsoFniks5tMhorgaJpZM4Re5Dw .

DavidRGriswold commented 5 years ago

Hey, long time no chat. I fixed the little issues on the VS Code braver solarized theme that had shown up in the github. If you get a chance to publish it to the VS Code marketplace at some point, that would be great!

Thanks, David

On Sat, Jan 20, 2018 at 3:09 PM David Griswold novachild@gmail.com wrote:

Not a problem! Color themes don't need a lot of updating so should be good for a while.

On Jan 20, 2018 10:54 AM, "Koen Lageveen" notifications@github.com wrote:

Yeah, I think that worked. Thanks man! If I need to publish anything else later just let me know. I think I may be able to set it up so you can publish too, but the sites to manage this are the most confusing mess I've ever seen.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/braver/vscode-solarized/pull/4#issuecomment-359185230, or mute the thread https://github.com/notifications/unsubscribe-auth/ATpU7_NNyljN1Y0jI6ayH5FLgQJsoFniks5tMhorgaJpZM4Re5Dw .

braver commented 5 years ago

Will do! 👌🏻

braver commented 5 years ago

Done!