search

jonathanchu / atom-one-dark-theme

Atom One Dark - An Emacs port of the Atom One Dark theme from Atom.io.
GNU General Public License v3.0
255 stars 52 forks source link

Annotation background color in company selection is inconsistent with the rest of the line #54

Closed finalpatch closed 5 years ago

finalpatch commented 5 years ago

https://github.com/jonathanchu/atom-one-dark-theme/blob/073a30cb86148ef3e00812e08cd844168acff0c5/atom-one-dark-theme.el#L133

Thsi can be fixed with adding company-tooltip-annotation-selection face, that combines the annotation foreground and selection background color:

   `(company-tooltip-annotation-selection ((t (:foreground ,atom-one-dark-mono-2 :background ,atom-one-dark-gray))))
jonathanchu commented 5 years ago

Oh nice, good catch! I'm unfamiliar with this, but should this replace company-tooltip-annotation or be added with company-tooltip-annotation unmodified?

Also happy to accept a PR for this if you know how to confirm it's good to go, I don't use company nearly as much as I should so I might miss the change if it's subtle. 😄

finalpatch commented 5 years ago

Add

On Wed, 26 Jun. 2019, 11:55 pm Jonathan Chu, notifications@github.com wrote:

Oh nice, good catch! I'm unfamiliar with this, but should this replace company-tooltip-annotation or be added with company-tooltip-annotation unmodified?

Also happy to accept a PR for this if you know how to confirm it's good to go, I don't use company nearly as much as I should so I might miss the change if it's subtle. 😄

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jonathanchu/atom-one-dark-theme/issues/54?email_source=notifications&email_token=AAHWY2XZ2C6FE5X5TGU2ECTP4NYL7A5CNFSM4H3INBQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTTJ7Q#issuecomment-505885950, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHWY2XT2BS4LRSUXRMWNXTP4NYL7ANCNFSM4H3INBQQ .

jonathanchu commented 5 years ago

Cheers, thanks @finalpatch! Just pushed that to master.