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

feat: color for flash #174

Merged
merged 3 commits into from
Jul 21, 2023
Merged

feat: color for flash #174

merged 3 commits into from
Jul 21, 2023

Conversation

Asthestarsfalll
Copy link
Contributor

@Asthestarsfalll Asthestarsfalll commented Jul 19, 2023

@mvllow
Copy link
Member

mvllow commented Jul 19, 2023

Thank you for adding this; because Substitute is not specific to a single plugin and is available in other situations, e.g. :%s/abc/xyz/, we should move this up into the base highlights.

As far as the colour, what you have is fine and feel free to leave it but I'm curious how you feel about something blended, e.g. Substitute = { fg = p.love, bg = p.love, blend = 10 }.

@Asthestarsfalll
Copy link
Contributor Author

I have tested this before:
2023-07-20_16-20

The contrast appears to be minimal as the blend effect causes the background to appear dimmer.

So I think the better choice is to use different colour of bg and fg without blend

@mvllow
Copy link
Member

mvllow commented Jul 20, 2023

That makes sense. So let's move this to ~ line 76, below ['SpellRare'] = { sp = p.subtle, undercurl = true }, because it's not specific to "folke/flash.nvim". And for that same reason, it's probably best to also set the fg = p.base because when you do :%s/abc/xyz/ it becomes hard to read on some matches. If you have any questions let me know, I'm happy to make these adjustments myself too if needed :)

@mvllow mvllow merged commit 4ac6683 into rose-pine:main Jul 21, 2023
@mvllow
Copy link
Member

mvllow commented Jul 21, 2023

Grazie!

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.

2 participants