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

[FR] Provide Callback for HUD window, so that user can replace it with other UI widgets(e.g. nvim-notify) #449

Closed
glyh opened this issue Dec 21, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@glyh
Copy link
Contributor

glyh commented Dec 21, 2022

I just played around nyoom.nvim, it looks pretty cool. But I realize sometime the notification window of notify overlaps with conjure's HUD. I think it would be good if it's possible to replace the HUD interface with nvim-notify. Of course this should be opt-in, by default we still should use the HUD.

@Olical Olical added the enhancement New feature or request label Dec 21, 2022
@Olical
Copy link
Owner

Olical commented Dec 21, 2022

Never heard of nvim-notify but yeah, I'm open to optionally supporting it if:

  • It's detected by Conjure
  • It's toggled on with an option

Just like the Tree Sitter support really (although I default that to on now because it's just better). Will consider adding support myself at some point soon, shouldn't be too bad since all HUD control runs through one spot in the code, it's fairly well organised and shouldn't require changing many places across the code.

Mostly just adding a new Fennel module, maybe under conjure.extras.nvim-notify so we start an optional "extras" section or something, that is called from the conjure.log module when required. I think the log module handles the HUD... IIRC.

@glyh
Copy link
Contributor Author

glyh commented Dec 21, 2022

here you go: nvim-notify

@Invertisment
Copy link

Why not instead provide a lua callback that would allow any plugin ever? Then you could configure it on your own.

@glyh
Copy link
Contributor Author

glyh commented Dec 22, 2022

@Invertisment that sounds better, @Olical maybe this is a better approach to preven the codebase from being bloated.

@glyh glyh changed the title Integration with nvim-notify? Providate Callback for HUD window, so that user can replace it with other UI widgets(e.g. nvim-notify) Dec 22, 2022
@shaunsingh
Copy link

Note that nvim notify is just a viewer for vim.notify, which is an api provided by neovim itself. would it make sense to add a conjure option to use vim.notify rather than a floating window to display eval results?

@glyh glyh changed the title Providate Callback for HUD window, so that user can replace it with other UI widgets(e.g. nvim-notify) [FR] Provide Callback for HUD window, so that user can replace it with other UI widgets(e.g. nvim-notify) Jan 5, 2023
@Olical
Copy link
Owner

Olical commented Jan 7, 2023

I don't think vim.notify is the right API for this since many points around Conjure's codebase constantly append to the log. So you'd get notified over and over with single lines from different parts of the code, causing the notify window to presumably flash up and down with parts of the log, but not a coherent view.

The design of the log is just that... a log! You need the context since it is continually append to. A notify / dialog window of sorts does not fit in that idea of a scrolling stateful log of additions. So I don't really know how it'll work with this at all.

I'll look at adding a hook so you can opt out of using the HUD code for something else, but it's up to the user to make that continuous appending behaviour work with whatever display they want to use.

@Olical
Copy link
Owner

Olical commented Jan 7, 2023

I've added a hook API to develop and documented it here, thoughts? Can be extend with more features of Conjure quite easily too.

conjure/doc/conjure.txt

Lines 1110 to 1154 in a74547e

==============================================================================
HOOKS *conjure-hooks*
Hooks allow you to override parts of Conjure's functionality with more Lua
code, allowing you to completely override behaviour or integrate other systems
in ways that the original design ever anticipated.
You manipulate hooks with the `conjure.hook` module, so you'll need to require
that into your own Lua configuration code (probably aliased as `hook`) to
continue. From there you may execute the `(hook.override *name* *fn*)` function to define
your own hook overrides.
If you need to also call the original code you can use `(hook.get *name*)` to
fetch the original Conjure internal function which you may call at your
discretion.
So here's an example of overriding the HUD with a simple `print` statement:
>
(module my.config
{autoload {hook conjure.hook}})
(hook.override :display-hud
(fn [opts]
(print "There's something new in the log, you might want to look."))
See below for all of the possible hooks and their default implementations and
arguments when executed, this list can be extended upon request and
consideration.
`(display-hud opts)`
Creates and displays the HUD floating window, scrolling the latest lines
into focus. The `opts` table may contain the key `low-priority?` which
indicates that this isn't so essential to display right away.
stdout and stderr should count as low priority most of the time when it
comes from clients that support this feature. This is because some
programs may spit out lots of stdio and we want to back off from HUD spam
if there is a LOT of low priority traffic. You can probably get away with
ignoring this feature though.
`(close-hud)`
Called when the HUD should close, either by direct user interaction or
because the user did something indirect like moving their cursor after a
while. The passive and active closing behaviour is identical from the
perspective of this function.

@Olical Olical closed this as completed Feb 26, 2023
@glyh
Copy link
Contributor Author

glyh commented Jun 19, 2023

Hello sorry for the delay, I'm looking at this right now. If I want to hook conjure into vim notify, is there a way I can get the content of evaluated buffer?
I see there's a function upsert-buf, which seems like what I need, could you make this public?

Here's a trivial script of what I'm trying to do:

        local hook = require('conjure.hook')
        hook.override('display-hud',
          function(opts)
            local log = require('conjure.log')
            local buf = log['upsert-buf']()
            local content = vim.nvim_buf_get_lines(buf, 0, -1, false)
            vim.notify(content)
          end)

Obviously this won't work since I can't access upsert-buf.

@glyh
Copy link
Contributor Author

glyh commented Jun 19, 2023

Now I'm thinking this is ugly, for the following reasons:

  1. UI is tied to the logging logic
  2. Using hook I have to access local variable, that force us to make the internal implementation public.

A better idea is to redesign this part, but for now I'm thinking the HUD is good enough, but anyone who is interested may look into that direction.

I assume noone will be using this code for now so @Olical could you please roll it back?

@glyh
Copy link
Contributor Author

glyh commented Jun 21, 2023

Also if a rewrite of the UI logic is possible I'm willing to help on it :)

@Olical
Copy link
Owner

Olical commented Jun 21, 2023

I don't think a total rewrite is the right move or very easy since I'm streaming lines from the REPL to your view. Sometimes the users needs that live stream, sometimes they don't care, Conjure can't easily do both (streaming and batching for notify plugins).

So I think it'll need to stay streaming but with more flags on messages to say "this is a result, here's the full body". If you use notify with that sort of idea it means a long running process that constantly logs will display nothing until it's done (which could be never!). So those things will need thinking about.

Conjure's entire UX is designed around the streaming log buffer, therefore so is the HUD, that won't be going away. I'm open to adding more events and metadata though that allow people to hook into notify if they really want to, but they will have to make decisions on tradeoffs. Will require quite a lot of timing / debounced code in order to batch stdout calls so a stdout print of 100 lines doesn't produce 100 notify calls in 1ms.

There's a few ways this could cause so many notifications that it pegs your CPU and essentially locks up Neovim I think. Will have to be careful with those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants