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

Entire mini.nvim, vim-fugitive and fzf-lua support #81

Closed
231tr0n opened this issue Jun 26, 2024 · 19 comments · Fixed by #83
Closed

Entire mini.nvim, vim-fugitive and fzf-lua support #81

231tr0n opened this issue Jun 26, 2024 · 19 comments · Fixed by #83
Labels
question Further information is requested

Comments

@231tr0n
Copy link

231tr0n commented Jun 26, 2024

Question or Suggestion

Can support for entire mini.nvim suite and fzf-lua be added. Currently this theme doesn't support mini.statusline, mini.indentscope afaik.

@231tr0n 231tr0n added the question Further information is requested label Jun 26, 2024
@231tr0n 231tr0n changed the title Entire mini.nvim support Entire mini.nvim and fzf-lua support Jun 26, 2024
@scottmckendry
Copy link
Owner

Hi @231tr0n!
Thanks for raising the issue. I'll be adding support for fzf-lua very soon :)

@scottmckendry
Copy link
Owner

Hello @echasnovski!

Hope I'm not being too cheeky asking this of you, but I can see you've made contributions to several other colorschemes just in the last day. I thought I'd jump on the opportunity to see if you'd be interested in adding Offical Mini Support™ to cyberdream.nvim as well. Would love input from the original author!

image

@scottmckendry
Copy link
Owner

@231tr0n I've added a borderless theme for fzf-lua that looks like this:

image

This mimics the default telescope theme quite closely. For it to look correct, you need to set fzf_colors to true in your fzf config. E.g.

return {
    "ibhagwan/fzf-lua",
    dependencies = { "nvim-tree/nvim-web-devicons" },
    config = function()
        require("fzf-lua").setup({
            fzf_colors = true,
        })
    end,
}

@231tr0n 231tr0n changed the title Entire mini.nvim and fzf-lua support Entire mini.nvim, vim-fugitive and fzf-lua support Jun 28, 2024
@231tr0n
Copy link
Author

231tr0n commented Jun 28, 2024

image
@scottmckendry, Is support for vim-fugitive possible. I did not want to create a new defect, so I modified the existing one.
If possible can you make the diff changes a bit darker if this is planned. Currently, they are way too light in color.

@echasnovski
Copy link
Contributor

Hello @echasnovski!

Hope I'm not being too cheeky asking this of you, but I can see you've made contributions to several other colorschemes just in the last day. I thought I'd jump on the opportunity to see if you'd be interested in adding Offical Mini Support™ to cyberdream.nvim as well. Would love input from the original author!

Hi @scottmckendry!

Since you are the first one to ask this (and nicely so), here you go. I'd like to explicitly stress though, that this is not an Offical Mini Support™, but my attempt to make 'mini.nvim' experience more pleasant for users. I'd much rather enjoy seeing that color scheme authors or their users add this kind of support on their own (it is explicitly stated as a welcome way to contribute to 'mini.nvim' with an explicit list of highlight groups needed for full support).


I would also like to give some feedback for issues that were apparent when adding 'mini.nvim' support:

  • The Visual highlight group defines both foreground and background. This is generally ok (although I prefer background only), but the result has way too low contrast ratio (1.71, to be exact). To the point that text is barely readable.
  • Having Diff* highlight only through foreground is rarely a good choice for usability. Mostly because they are used to highlight already highlighted text. Using background highlighting is usually a better choice. I'd prefer something like what is used for MiniDiffOverXxx highlight groups in PR.
  • The CursorLine and CursorColumn highlight groups define lines used to quickly locate cursor on screen. As cursor can be used in floating windows, it is reasonable to have those two groups be such that they are visible both in normal and floating windows. Hence I am not really sure what the purpose of custom MiniFilesCursorLine is.
  • I'd personally create all shades with appropriate names beforehand instead of using util.blend() in the highlight group definitions. This allows for a more coherent future definitions in plugin extensions.

Hope this helps.

@scottmckendry
Copy link
Owner

You're a ⭐ @echasnovski! Thanks for the excellent feedback. Especially around useability and contrast. I want the colorscheme to be as friendly as possible for users and contributors alike.

Appreciate all your hard work on neovim and mini! It's people like you that make the neovim community such a pleasure to be a part of 💚

scottmckendry added a commit that referenced this issue Jun 28, 2024
partial support for vim-fugitive on #81
@scottmckendry
Copy link
Owner

@231tr0n I've fixed the highlights for vim-fugitive's :Git diff command in the latest commit. As far as I can tell, everything else looks pretty good out of the box. If you spot anything else, let me know 🙂

@231tr0n
Copy link
Author

231tr0n commented Jun 28, 2024

image
image
@scottmckendry The mini.statusline and MiniHiPatterns dont look properly with the latest changes.
MiniHipatternsNote
MiniHipatternsTodo
MiniHipatternsHack
MiniHipatternsHack
MiniStatuslineModeReplace
MiniStatuslineModeCommand
MiniStatuslineModeInsert
MiniStatuslineModeVisual
MiniStatuslineModeNormal
MiniStatuslineModeOther
are the related highlight groups afaik. Is it possible to change these properly for better visibility?

scottmckendry added a commit that referenced this issue Jun 28, 2024
For some unknown reason, the `Visual` highlight group has had pretty
poor constrast for a long time. I'm honestly not sure how it went
unnoticed for so long. Thanks to echasnovski for pointing it out on #81
scottmckendry added a commit that referenced this issue Jun 28, 2024
small change to fix reported issue in transparent mode on #81
@scottmckendry
Copy link
Owner

Should be resolved now. Pretty sure this would only be an issue when transparency is enabled - due to the bg color being set to "NONE". Let me know if the most recent change fixes it 👍🏻

@231tr0n
Copy link
Author

231tr0n commented Jun 28, 2024

Should be resolved now. Pretty sure this would only be an issue when transparency is enabled - due to the bg color being set to "NONE". Let me know if the most recent change fixes it 👍🏻

@scottmckendry yup it fixes it. One last addon, currently if we open buffers horizontally using :split command, there is visually no seperator spearating the two buffers horizontally. Vertically there is a line but my doubt was can something be done about this horizontally.
image
As you can see in the above picture there are 4 buffers open altough I can hardly distinguish the two horizontally open buffers on the left pane. Can some kind of separator be put for this one particular scenario if possible.

@scottmckendry
Copy link
Owner

Oh, that's an easy one! Just set hide_fillchars to false (or remove the option entirely since this is the default) in your config and the separators will reappear 🙂

image

@231tr0n
Copy link
Author

231tr0n commented Jun 28, 2024

Oh, that's an easy one! Just set hide_fillchars to false (or remove the option entirely since this is the default) in your config and the separators will reappear 🙂

image

It is false in my case but still i dont see any separators horizontally.

@scottmckendry
Copy link
Owner

Hmmm... I'm not 100% sure what that could be then. Could you try defining the fillchar manually in your config?
This line here, update it to vim.o.fillchars = [[eob: ,fold: ,foldopen:,foldsep: ,foldclose:,horiz:─]] to see if there's any change.

If not, it might be getting overridden somewhere else.

@231tr0n
Copy link
Author

231tr0n commented Jun 28, 2024

Hmmm... I'm not 100% sure what that could be then. Could you try defining the fillchar manually in your config? This line here, update it to vim.o.fillchars = [[eob: ,fold: ,foldopen:,foldsep: ,foldclose:,horiz:─]] to see if there's any change.

If not, it might be getting overridden somewhere else.

It is still the same even after the change you suggested.The thing is vertical line is coming, only horizontal is the issue.
image

@scottmckendry
Copy link
Owner

Looks like it might be a global statusline thing. If you run :set laststatus=3 does that fix it?

@scottmckendry scottmckendry reopened this Jun 29, 2024
@scottmckendry scottmckendry added the waiting for op This issue is waiting for a response from the original poster label Jun 29, 2024
@231tr0n
Copy link
Author

231tr0n commented Jun 29, 2024

Looks like it might be a global statusline thing. If you run :set laststatus=3 does that fix it?

Yup this fixes it. Thank you soo much @scottmckendry.

@scottmckendry scottmckendry removed the waiting for op This issue is waiting for a response from the original poster label Jun 29, 2024
@231tr0n
Copy link
Author

231tr0n commented Jun 29, 2024

CLosing this issue.

@231tr0n 231tr0n closed this as completed Jun 29, 2024
@echasnovski
Copy link
Contributor

You're a ⭐ @echasnovski! Thanks for the excellent feedback. Especially around useability and contrast. I want the colorscheme to be as friendly as possible for users and contributors alike.

Appreciate all your hard work on neovim and mini! It's people like you that make the neovim community such a pleasure to be a part of 💚

You're welcome! Best of luck keeping up with the not-always-easy burden of maintaining a color scheme.


  • Having Diff* highlight only through foreground is rarely a good choice for usability. Mostly because they are used to highlight already highlighted text. Using background highlighting is usually a better choice. I'd prefer something like what is used for MiniDiffOverXxx highlight groups in PR.

I've remembered the other (probably more prominent) reason why Diff* highlight groups usually have distinctive background. DiffAdd, DiffChange, and DiffDelete are used to highlight whole lines, meaning the part after end of line is also highlighted. If there is no background attribute in those highlight groups, this is not visible. And there is also an issue of highlighting whitespace with these groups.

scottmckendry added a commit that referenced this issue Jun 29, 2024
as suggested in #81 - much better support for diffing plugins OOTB like
diffview.nvim.
@scottmckendry
Copy link
Owner

I've just added a fix for this in the latest commit. Gave it a spin with diffview.nvim and it looks SOO much better! Thank you again 🙂

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

Successfully merging a pull request may close this issue.

3 participants