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

Support mini.statusline #79

Merged
merged 2 commits into from
Aug 5, 2023
Merged

Support mini.statusline #79

merged 2 commits into from
Aug 5, 2023

Conversation

leaysgur
Copy link
Contributor

Hello! 👋🏻 I'd like to support mini.statusline.

(This is my first time contributing to a Lua project, so please let me know if I missed something. 😅 )

@AlexvZyl AlexvZyl self-assigned this Jul 31, 2023
@AlexvZyl AlexvZyl added the Enhancement New feature, request or suggestion label Jul 31, 2023
@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 3, 2023

Thanks for the PR!

Now that you mention it being based on lualune... What if you did something like:

local C = require 'lualine.themes.nordic'

return {
    MiniStatuslineModeNormal = {
        bg = C.normal.a.bg,
        fg = C.normal.a.fg,
        bold = (C.normal.a.gui == 'bold'),
    },
    -- ...
}

And then mini.statusline will always be up to date with lualine.

@leaysgur
Copy link
Contributor Author

leaysgur commented Aug 3, 2023

Thank you for the review!

I thought about it too at first, but didn't because nightfox and tokyonight seemed to keep them separate.
I'm not sure but there may be some cases where lualine should not be the base spec?

Which do you prefer? 👀

@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 3, 2023

lualine.themes is cloned when nordic is cloned... I can't imagine a situation where it will not be in the base spec? Or am I missing somthing?

@leaysgur
Copy link
Contributor Author

leaysgur commented Aug 3, 2023

Oh, sorry!

  • lualine seems to consist of 3 units(a, b, c)
  • If someday another statusline module is introduced and it consists of 4 units, 4th one should be defined independently
  • It might feel inconsistent?

And now I come up with another concern.

  • If one day lualine gets major updates and changes its data structure
  • All other statusline modules that depend on lualine spec may be need to updated too
  • Is it OK?

This was my intention. 😄

@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 4, 2023

I am unable to keep every single plugin up to date. I would much rather have mini.statusline break due a change in lualine, than have to go to a bunch of plugins when I want to change one color.

I plan on making neo-tree depend on nvim-tree in the same way.

@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 4, 2023

Thanks for the PR!

Now that you mention it being based on lualune... What if you did something like:

local C = require 'lualine.themes.nordic'

return {
    MiniStatuslineModeNormal = {
        bg = C.normal.a.bg,
        fg = C.normal.a.fg,
        bold = (C.normal.a.gui == 'bold'),
    },
    -- ...
}

And then mini.statusline will always be up to date with lualine.

So if you don't mind doing it like this? I much prefer this way, after thinking about it :)

@leaysgur
Copy link
Contributor Author

leaysgur commented Aug 5, 2023

That's all right. (Just wanted to make sure 👍🏼)

I'll try to fix it tonight.

@leaysgur
Copy link
Contributor Author

leaysgur commented Aug 5, 2023

@AlexvZyl Fixed~~!

@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 5, 2023

Do you mind adding a screenshot?

@leaysgur
Copy link
Contributor Author

leaysgur commented Aug 5, 2023

Sure! like this?

Before

Screen Shot 2023-08-05 at 22 36 07
Screen Shot 2023-08-05 at 22 36 18
Screen Shot 2023-08-05 at 22 38 55
Screen Shot 2023-08-05 at 22 36 54
Screen Shot 2023-08-05 at 22 36 44

After

Screen Shot 2023-08-05 at 22 37 24
Screen Shot 2023-08-05 at 22 37 32
Screen Shot 2023-08-05 at 22 37 47
Screen Shot 2023-08-05 at 22 37 55
Screen Shot 2023-08-05 at 22 38 06

Please let me know what you would like to record if you have any requests. 📸

@AlexvZyl
Copy link
Owner

AlexvZyl commented Aug 5, 2023

Thanks for bearing with my pedantry :)

@AlexvZyl AlexvZyl merged commit c36b22b into AlexvZyl:main Aug 5, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature, request or suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants