Skip to content
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

Updates from my fork. smerge-tool, magit, notmuch #123

Closed
wants to merge 3 commits into from
Closed

Updates from my fork. smerge-tool, magit, notmuch #123

wants to merge 3 commits into from

Conversation

AdeAttwood
Copy link
Contributor

I forked this project a while ago to add smerge-tool support that then turned into adding some more stuff and I thought it better see if you wanted any of the changes upstream.

I have been using these changes in my config using base16 shell and mostly the base16_tomorrow theme and occasionally base16_tomorrow-night .

Smerge and Magit

Changes the diffs to have the background colour as the default color, then the foreground colour to be the added or removed color (red or green). Before, the faces were not set, and I was getting the background as some random purple color.

Notmuch

Support for this was not implemented. This mainly gets rid of the bright green foreground color that was quite unreadable.

The default notnumch has some really bright green in notmuch-search that was a
bit unreadable. This tones that back to have a more readable faces.
@belak
Copy link
Member

belak commented Jun 7, 2022

Sorry for the delay! We're working on moving some base16 repos to a common place and I missed this. I'm happy to merge this, but is there any chance you have before/after screenshots?

@AdeAttwood
Copy link
Contributor Author

So while doing a before and after on the packages, it looks like things have changed with all the diff faces. I don't think the smerge and magit commits are relevant any more, the before is better. The notmuch is still looking OK though, don't know if you want to cherry-pick in the notmuch commit, or you want me to amend the PR.

Smerge

Before

smerge-before

After

smerge-after

Notmuch

Before

Screenshot at 2022-07-08 21-04-19

After

notmuch-after

Let me know what you think, I am happy with either 👍

@belak
Copy link
Member

belak commented Jul 13, 2022

I've applied the notmuch patch for now - I'm a bit conflicted on diffs (in regards to base16) because there's not a light green available, which makes it hard to style diffs properly, so I'm holding off on that for now.

@AdeAttwood
Copy link
Contributor Author

That's cool, there was some funny stuff going on with the before and afters on the smerge patches. This PR can be closed now 👍

@AdeAttwood AdeAttwood closed this Jul 13, 2022
@ooglyhLL
Copy link

Could the diff and smerge issue perhaps be addressed by synthesizing a lighter variant with functions from color.el? While not strictly base16 anymore, it might serve the purpose for that specific use case.

@belak
Copy link
Member

belak commented Jul 13, 2022

I experimented with this a few years back, but I was never sure what to do with it, since lighten/darken doesn't work in the terminal.

I have a feeling this may be something which gets addressed when the org finalizes ansi16 (which would have a light and dark green/red) and/or base17 (which will probably be a different set of themes which don't support the terminal, so we can be a little more adventurous with the colors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants