Fix Solarized Light and Dark color schemes
ClosedPublic

Authored by acrouthamel on Sep 7 2018, 6:21 PM.

Details

Summary

Solarized Light contrast was low. Upon investigation it was determined much of the color scheme did not follow Solarized base 16 colors or rules. Color scheme for both Dark and Light variants were adjusted to match the latest Solarized color scheme release palette and color interaction rules. Color schemes were also matched closely to the Vim color scheme created by the original Solarized author.

This is my first "frameworks" patch, so please let me know if I missed anything.

This has also been added to KSyntaxHighlighting in D15340.

BUG: 382075

Solarized Light Before:


Solarized Light After:

Solarized Dark Before:

Solarized Dark After:

Test Plan

Open Kate with new rc files and inspect the Color settings. Open a CPP file to review syntax highlighting.

Diff Detail

Repository
R39 KTextEditor
Branch
fix-solarized-colors
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2577
Build 2595: arc lint + arc unit
acrouthamel created this revision.Sep 7 2018, 6:21 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptSep 7 2018, 6:21 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
acrouthamel requested review of this revision.Sep 7 2018, 6:21 PM
acrouthamel edited the summary of this revision. (Show Details)Sep 7 2018, 6:27 PM
acrouthamel edited the test plan for this revision. (Show Details)
acrouthamel added a project: KTextEditor.

Thanks for working on this. Looks good to me - just a minor question about the section for Solarized light.

And also related: would you also create the .theme files for KSyntaxHighlighting?

src/data/katesyntaxhighlightingrc
104

Am I mistaken, or did you remove the section here?

acrouthamel updated this revision to Diff 41170.Sep 7 2018, 7:13 PM
  • Fixed section code
acrouthamel marked an inline comment as done.Sep 7 2018, 7:15 PM

I've fixed the section. I'm actually working on the KSyntaxHighlighting themes now, you'll see those in another patch.

src/data/katesyntaxhighlightingrc
104

No, you're right, that's a mistake. I just fixed it.

acrouthamel updated this revision to Diff 41171.Sep 7 2018, 7:29 PM
acrouthamel marked an inline comment as done.
  • Solarized Light Region Marker fix and small Dark tweak.
acrouthamel updated this revision to Diff 41172.Sep 7 2018, 7:52 PM
  • Fixed Light scheme Error color
dhaumann added inline comments.Sep 7 2018, 8:05 PM
src/data/katesyntaxhighlightingrc
143

Isn't "Others" now missing? Every color theme should define 31 colors. If this was not the case before, then it was also wrong before :)

Can you check again?

acrouthamel updated this revision to Diff 41176.Sep 7 2018, 8:29 PM
  • Some Marker color tweaks to ensure Solarized interaction rule adherence
acrouthamel updated this revision to Diff 41177.Sep 7 2018, 8:34 PM
  • Fixed Others definition
acrouthamel marked 2 inline comments as done.Sep 7 2018, 8:36 PM

You were right again. For some reason that Others= disappeared from my ~/.config/katesyntaxhighlightingrc so I didn't notice it. Even though it was defined and displaying fine in the GUI. Weird.

I also made a few more adjustments to Markers colors to adhere to the Solarized rules for gray colors. I think it is pretty set now, unless something else disappeared...

@dhaumann, I've added these colors to KSyntaxHighlighting over here: D15340

acrouthamel edited the summary of this revision. (Show Details)Sep 7 2018, 8:58 PM

Solarized dark still only has 28 colors, right? Can you add the missing ones?

acrouthamel updated this revision to Diff 41181.Sep 7 2018, 9:37 PM
  • Added three missing definitions

Solarized dark still only has 28 colors, right? Can you add the missing ones?

Man, I'm looking forward to this JSON system in the future. LOL. Ok, fixed again. I counted all of them before commit.

dhaumann accepted this revision.Sep 7 2018, 9:41 PM

Looks good to me. Can you commit yourself?

This revision is now accepted and ready to land.Sep 7 2018, 9:41 PM

Thanks, I can. I'm getting an error in Arcanist, so let me diagnose that and I'll land it.

acrouthamel closed this revision.Sep 7 2018, 11:26 PM