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

[WIP] feat: notifications #1445

Closed
wants to merge 1 commit into from
Closed

[WIP] feat: notifications #1445

wants to merge 1 commit into from

Conversation

matoous
Copy link
Contributor

@matoous matoous commented Jan 5, 2022

Adds support for notifications in similar vein as https://github.com/rcarriga/nvim-notify.

  • display notifications
  • hide old notifications
  • add command to show picker with all notifications
  • hook into the event loop to hide the notification when the time comes instead of when the ui is re-rendered
  • allow notifications to be disabled

@matoous
Copy link
Contributor Author

matoous commented Jan 6, 2022

@archseer one design question please: I need to re-render the editor when the notification times out, how you you approach this? Ideally, I would like to be able to do this from within Notifications.add() method so the caller doesn't have to care, would that somehow be possible?

@archseer
Copy link
Member

archseer commented Jan 8, 2022

Yeah this type of UI complexity was purposely left out for now 😅

You could expand the idle timer implementation to support more timers that emit events. Another option would be to spawn a new job that sleeps for N ms then looks up the component via the compositor and does something.

@archseer
Copy link
Member

archseer commented Jan 8, 2022

What's the usecase for these though?

@matoous
Copy link
Contributor Author

matoous commented Jan 8, 2022

What's the usecase for these though?

The usecase would be to hide the notification after for example 5s. Right now it stays visible until you do something that forces that editor to re-render. I don't think is blocker per-se but would be nice to eventually have this functionality. What do you think? Maybe I can fix the rest and we can leave this for future.

@kirawi kirawi mentioned this pull request Jan 9, 2022
@archseer
Copy link
Member

archseer commented Jan 9, 2022

Oh, I mean what would you use notifications for? For build failures etc I think it's best to open a dismissable popup (nothing worse than an error that disappears before you manage to read it)

I also think it's unlikely you'd have so many notifications open that it would require a picker

@matoous
Copy link
Contributor Author

matoous commented Jan 9, 2022

The main idea were LSP messages and lsp status but it can also be kept as it is and we can put those into the command entry line, in which case I don't think I have a use case for this as of now.

@matoous
Copy link
Contributor Author

matoous commented Mar 29, 2022

No time to work on this one and also there isn't clear use case, closing.

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