Skip to content

Conversation

@NotAShelf
Copy link
Owner

@NotAShelf NotAShelf commented Jan 5, 2025

Adds a module for feline.nvim. Should be done, but we'd like to add examples to components.active and components.inactive before this can be merged.

@NotAShelf NotAShelf force-pushed the feline branch 4 times, most recently from 77f52f7 to b3b8d3a Compare January 10, 2025 10:30
@NotAShelf NotAShelf marked this pull request as ready for review January 10, 2025 10:36
@NotAShelf NotAShelf requested a review from diniamo January 11, 2025 14:36
config = mkIf cfg.enable {
vim = {
lazy.plugins.feline-nvim = {
event = "UIEnter";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
event = "UIEnter";
event = "DeferredUIEnter";

This is debatable, but if the goal is to start accepting user input as soon as possible, then DeferredUIEnter should be used

Copy link
Owner Author

Choose a reason for hiding this comment

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

Is there a benefit to taking user input as soon as possible?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean yeah? You wouldn't want a 10s startup time would you

enable = mkEnableOption "minimal, stylish and customizable statusline, statuscolumn, and winbar [feline.nvim]";
setupOpts = mkPluginSetupOption "feline-nvim" {
custom_providers = mkOption {
type = attrsOf anything;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be of type luaInline

Copy link
Owner Author

Choose a reason for hiding this comment

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

custom_providers takes a table, in which case we should be able to get a listOf luaInline. No?

Copy link
Contributor

@diniamo diniamo Jan 12, 2025

Choose a reason for hiding this comment

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

Men go read the docs, you have to be able to refer to the custom providers somehow. It's a set of functions (which is what I meant to say in my original comment, but it ended up beibg ambiguous)


disable = mkOption {
default = {};
type = attrsOf (submodule conditionalRenderers);
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense, the defaults above only apply to force_active. Nothing should be disabled by default.

Copy link
Owner Author

Choose a reason for hiding this comment

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

We cannot omit the default here. That's why it has one, and also conditionalRenderers submodule provides upstream defaults.

Copy link
Contributor

@diniamo diniamo Jan 12, 2025

Choose a reason for hiding this comment

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

It literally does not, nothing is disabled by default (in the upstream detaults, which is what I meant by apply)

github-actions bot pushed a commit that referenced this pull request Jan 11, 2025
@github-actions
Copy link

github-actions bot commented Jan 11, 2025

🚀 Live preview deployed from 9df9d51

View it here:

Debug Information

Triggered by: NotAShelf

HEAD at: feline

Reruns: 1524

github-actions bot pushed a commit that referenced this pull request Jan 11, 2025
@NotAShelf NotAShelf force-pushed the main branch 5 times, most recently from 02ee4cc to bc978c4 Compare March 17, 2025 11:42
@NotAShelf NotAShelf closed this Oct 22, 2025
@NotAShelf NotAShelf deleted the feline branch October 22, 2025 05:22
@github-actions
Copy link

✅ Preview has been deleted successfully!

github-actions bot pushed a commit that referenced this pull request Oct 22, 2025
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