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

Allow showing a loading spinner instead of showing the console unless there is an error #1519

Open
fnune opened this issue Oct 23, 2024 · 11 comments

Comments

@fnune
Copy link
Contributor

fnune commented Oct 23, 2024

I set my console_timeout to 250 because many of the commands I run end up showing the console, so I'd rather see it quickly.

However, most of the time I can just dismiss it. It will also close automatically. So that means it's not really of any use to me other than for the fact that it lets me know that something's happening and Neogit isn't frozen.

That's why I think this should be Neogit's behavior:

  • Show a spinner whenever a command starts that is taking long (short countdown of ~100ms to start?)
    • This could be made configurable
    • Could also be offered as a statusline component
  • If the underlying command exits with 0, just stop showing the spinner, since Neogit will change its UI accordingly anyway
  • Show the console only if there is an error
    • Again, this could be made configurable
@CKolkey
Copy link
Member

CKolkey commented Oct 23, 2024

Yeah, I had a similar thought the other day. Is this something you want to take a crack at?

@fnune
Copy link
Contributor Author

fnune commented Oct 23, 2024

I can do it, but WDYT about the design alternatives?

  • Show a spinner somewhere within the Neogit status buffer
    • If so, where?
  • Offer it as a statusline component
  • Both?
  • Allow configurable spinner?

I can make my own assumptions/decisions, but happy to hear what you think :)

@CKolkey
Copy link
Member

CKolkey commented Oct 23, 2024

You could probably get away with using a floating window that has the same background color as the buffer, that doesn't get focused on creation. Maybe even just vim.notify() with some extra arguments?

Or, echo/redraw so it appears in the cmd line. The Refs buffer does that when loading cherries for a ref.

Statusbar could be fine, but I havent messed with that before.

I wouldn't put it as -part- of the status buffer though. That would get expensive to rerender just for a spinner

@fnune
Copy link
Contributor Author

fnune commented Oct 23, 2024

You could probably get away with using a floating window that has the same background color as the buffer

What do you mean by this? A sort of overlay on top of the Neogit status window?

Or something small on the side like a Noice notification? (but not using Noice, heh)
image

@fnune
Copy link
Contributor Author

fnune commented Oct 23, 2024

Statusbar could be fine, but I havent messed with that before.

Maybe this is an easy approach and a good start:

  • Offer a way to only show the console on nonzero exit codes
  • Offer a variable or Lua function that can be used in a statusline, or in a statusline plugin

Example:

-- Neogit config
auto_show_console_error_only = true
-- Lualine example with lazy.nvim
return {
  "nvim-lualine/lualine.nvim",
  config = function()
    local lualine = require("lualine")
    lualine.setup({
      sections = {
        lualine_a = { "mode", require("neogit.process").status },
      },
    })
  end,
}

Then it's mostly done.

@CKolkey
Copy link
Member

CKolkey commented Oct 24, 2024

You could probably get away with using a floating window that has the same background color as the buffer

What do you mean by this? A sort of overlay on top of the Neogit status window?

Yeah, like that. Noice (and fidget, nvim-notify, cmp, ...) just place a floating window. Without a border it would blend in very smoothly.

I think showing on error-only would make sense. Since I added some logic to check for git hooks a few days ago, the "timer" idea doesn't really make sense to me. Like, I suppose if something is taking a long time you might want to see why, but besides a hook I'm not sure what would take a long time.

Would the status line get repainted frequently enough to make a spinner meaningful? IIRC it's a poll model, not a push model, so if it stopped polling for updates you might get stale state shown. But maybe I'm wrong 🤷🏼 Regardless, I'm not against it.

@CKolkey
Copy link
Member

CKolkey commented Oct 24, 2024

Here's a little POC for a spinner in the message window, if you like:

api.nvim_create_user_command("SpinnerPOC", function(args)
  local pattern = {
    "-",
    "\\",
    "|",
    "/"
  }

  local function setInterval(interval, callback)
    local timer = vim.uv.new_timer()
    timer:start(interval, interval, function()
      callback()
    end)
    return timer
  end

  local function clearInterval(timer)
    timer:stop()
    timer:close()
  end

  local count = 0
  local timer = setInterval(50, vim.schedule_wrap(function()
    count = count + 1
    vim.cmd(string.format("redraw | echomsg '%s Running Process'", pattern[(count % #pattern) + 1]))
  end))

  vim.defer_fn(
    function()
      clearInterval(timer)
      vim.cmd("redraw | echomsg ''")
    end,
    3000
  )
end, {
    desc = "spin POC",
  })

@fnune
Copy link
Contributor Author

fnune commented Oct 25, 2024

So I've been exploring using :h vim.notify for this. It seems like the default Neovim implementation of vim.notify does not allow one to update an existing notification. So if I make a vim.notify call to update the spinner for each one of its frames, and then I happen to have a plugin like nvim-notify installed, I get this:

image

That means that if I want to use vim.notify I should probably choose between:

  • Showing a static message (no spinner) for users without any notify plugins, or
  • Targeting a specific notify replacement plugin (like nvim-notify), or
  • Detecting which plugin is installed and doing something different depending on that
    • Would probably need to support mini-notify.nvim, nvim-notify...
    • Could decide to show a spinner if the user does not have any plugin enabled, though
    • But I think it would be a mess

The reason I'm thinking about this is because I don't enjoy the idea of showing our own notification buffer, because I know users of notify replacement plugins would enjoy a notification in their own format.

I will explore the statusline idea for a bit now.

@CKolkey
Copy link
Member

CKolkey commented Oct 25, 2024

We used to have a bespoke notification component that I removed a while ago because it was from a time before the vim.notify() interface, and the proliferation of those plugins. Thats just to say I'm not entirely keen on putting it back unless entirely needed and scoped more narrowly.

All sounds reasonable - just showing a popup without a nifty spinner doesn't really communicate what I think you have in mind here, and... we can already do that, so thats no fun.

Lemme know how the statusline idea goes :)

@fnune
Copy link
Contributor Author

fnune commented Oct 25, 2024

Basically something like this:

_G.neogit_progress_statusline_frame = ""

local function on_neogit_process_start()
  local spinners = { "", "", "", "", "", "", "", "", "", "" }
  local index = 1

  local timer = vim.loop.new_timer()
  timer:start(
    0,
    100,
    vim.schedule_wrap(function()
      _G.neogit_progress_statusline_frame = spinners[index] .. " Committing..."
      index = (index % #spinners) + 1
      vim.cmd("redrawstatus")
      -- If lualine is installed: require("lualine").refresh()
    end)
  )

  local function on_neogit_process_end()
    _G.neogit_progress_statusline_frame = ""
    vim.cmd("redrawstatus")
    -- If lualine is installed: require("lualine").refresh()
    timer:stop()
    timer:close()
  end

  vim.defer_fn(on_neogit_process_end, 3000)
end

vim.api.nvim_create_user_command("NeogitProgressStart", on_neogit_process_start, { desc = "" })

function neogit_progress()
  return _G.neogit_progress_statusline_frame or ""
end

Sample usage in Lualine:
image

Sample usage in a regular statusline:

vim.o.statusline = "%!v:lua.neogit_progress()"

@fnune
Copy link
Contributor Author

fnune commented Oct 25, 2024

I'm just exploring, please don't hesitate to say no to my ideas 😄

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

No branches or pull requests

2 participants