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.

Sign up for GitHub

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

Jump to bottom

Support for LspSaga #48

Merged
merged 19 commits into from
Dec 17, 2021
Merged

Support for LspSaga #48

merged 19 commits into from
Dec 17, 2021

Conversation

eaxly
Copy link
Contributor

@eaxly eaxly commented Dec 11, 2021 •
edited
Loading

Hey there!
Thank you all for developing this theme!

I recently started using lspsaga and am very enjoying it functionality wise.
Problem is, lspsaga defines its own highlight groups, so it looks a bit off compared to the rest of the colorscheme.

Some Examples

Here on the borders and the arrow icon of the rename prompt:
image

Here again the borders and I guess the weird black line:
image

And so on...
image

Where is this going

I will try in the next three weeks or so to add full support for it.

Also this is the first time I am doing a PR on anything really so any help is greatly appreciated 🙂 (and patience too)

@eaxly
lspsaga: update readme
66cc72c 
add lspsaga to the readme
@mvllow
Copy link
Member

mvllow commented Dec 11, 2021

How exciting! Let us know if you have troubles with anything – one tip to start would be to test locally by adding your local folder to your plugin manager. Eg. using packer

- use('rose-pine/neovim')
+ use('~/your-folder')

Then a :PackerSync and you're all set 😌 Now changes to your folder will reflect in your editor. Hope that helps!

@eaxly
feat: code actions and diagnostics hl-groups
47aebd5 
Highlight groups for:
- code actions
- diagnostics
@eaxly
Copy link
Contributor Author

eaxly commented Dec 12, 2021 •
edited
Loading

image

This is the new diagnostic preview...

image

... and this the code actions prompt

@eaxly
feat(nvim): add some more colors
8e8ecca 
Adding some more lspsaga colors.
@eaxly
Copy link
Contributor Author

eaxly commented Dec 15, 2021 •
edited
Loading

Most colours are now added. Moving onto testing if all colours show up correctly.

@eaxly
Copy link
Contributor Author

eaxly commented Dec 15, 2021

just the definition preview that has something wrong with it...

@eaxly
Copy link
Contributor Author

eaxly commented Dec 15, 2021

image
🤔

@mvllow
Copy link
Member

mvllow commented Dec 15, 2021 •
edited
Loading

I don't have experience with LspSaga but once you're comfortable with the colours we can review 😌 Thank you for working on this!

Edit: Also, the neovim matrix channel might be a good place to ask questions about LspSaga highlighting. Maybe take a look at tokyonight theme to reference as it looks like they support LspSaga.

eaxly added 4 commits December 16, 2021 18:22
@eaxly
feat(lspsaga): finishing touches
a218fd5 
Add the finihing touches and add the last needed highlightgroups

Disclaimer: Preview Definitions still don't work
@eaxly
style(lspsaga): remove comments and rearrange
81faeee 
Remove uneeded comments and rearrange some code
@eaxly
fix(lspsaga): preview definitions
173aeb6 
Preview Definitions now get highlighted! Why?
Because it's *Definitions* not *Defintions*
@eaxly
feat(lspsaga): add one new highlight
96cc6f2 
Add LspSagaBorderTitle highlight
@eaxly
Copy link
Contributor Author

eaxly commented Dec 16, 2021

Ready for review!

@eaxly eaxly marked this pull request as ready for review December 16, 2021 17:56
mvllow
mvllow requested changes Dec 16, 2021
View reviewed changes
Copy link
Member

@mvllow mvllow left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After making changes, please rebase and format with stylua 😌 Thank you!

lspsaga.norg Outdated Show resolved Hide resolved
lua/rose-pine/theme.lua Outdated Show resolved Hide resolved
lua/rose-pine/theme.lua Outdated Show resolved Hide resolved
lua/rose-pine/theme.lua Outdated Show resolved Hide resolved
eaxly and others added 3 commits December 17, 2021 09:58
@eaxly
fix(lspsaga): remove lspsaga.norg
77ee765
@eaxly
style(lspsaga): rearrange diagnostic highlights
fbc31eb 
Rearrange `DiagnosticInformation` and `DiagnosticWarning` to their respective spots
@eaxly
Merge branch 'main' into main
5d26b3d
@eaxly
Copy link
Contributor Author

eaxly commented Dec 17, 2021 •
edited
Loading

Hope this is good 😬...
Edit: Wait a second... this is not rebasing 🤦

eaxly added 6 commits December 17, 2021 11:08
@eaxly
Revert "Merge branch 'main' into main"
01de6cd 
This reverts commit 5d26b3d, reversing
changes made to 79c84ef.
@eaxly
lspsaga: update readme
8c58d0d 
add lspsaga to the readme
@eaxly
feat: code actions and diagnostics hl-groups
3a8ed9d 
Highlight groups for:
- code actions
- diagnostics
@eaxly
style(lspsaga): remove comments and rearrange
448f95e 
Remove uneeded comments and rearrange some code
@eaxly
style(lspsaga): rearrange diagnostic highlights
e1936a4 
Rearrange `DiagnosticInformation` and `DiagnosticWarning` to their respective spots
@eaxly
Revert "Merge branch 'main' into main"
e95efe6 
This reverts commit 5d26b3d, reversing
changes made to 79c84ef.
@eaxly
Copy link
Contributor Author

eaxly commented Dec 17, 2021

I don't have a good feeling about this... sorry!

@mvllow
Copy link
Member

mvllow commented Dec 17, 2021

No worries! I'll take a look a little later today and we'll get it all taken care of 😌

@eaxly
Copy link
Contributor Author

eaxly commented Dec 17, 2021

Thank you! ❤️

mvllow
mvllow reviewed Dec 17, 2021
View reviewed changes
Copy link
Member

@mvllow mvllow left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use stylua to format

Edit: Once stylua is installed, I believe you can run stylua theme.lua (or wherever that file is located for you). If you have any troubles let me know and I'll format it on my end

lua/rose-pine/theme.lua Outdated Show resolved Hide resolved
@eaxly
Copy link
Contributor Author

eaxly commented Dec 17, 2021

That is everything done?

@mvllow
Copy link
Member

mvllow commented Dec 17, 2021

Not sure why stylua didn't pick up our formatting rules but not a problem, everything looks good now! Thanks for all the hard work 😌

@mvllow mvllow merged commit adec84e into rose-pine:main Dec 17, 2021
@eaxly
Copy link
Contributor Author

eaxly commented Dec 17, 2021

Thank you for your kindness and patience @mvllow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvllow mvllow mvllow approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eaxly @mvllow