rcarriga / nvim-notify

A fancy, configurable, notification manager for NeoVim
MIT License
3k stars 80 forks source link

Issue with changing notification position - bug? #124

Closed levouh closed 2 years ago

levouh commented 2 years ago

Having trouble uploading gifs at the moment to demonstrate, but will update later when they are working


Taking what is in fade_in_slide_out.lua and changing the col value from vim.opt.columns:get() to just 1 moves the notifications from the top right to the top left as I would expect.

However, when changing the stages_util.DIRECTION.TOP_DOWN to stages_util.DIRECTION.BOTTOM_UP in an attempt to then move them to the bottom left results in issues where a secondary notification overlaps the first one vertically by ~2 rows rather than having them stay disjointed. Because this is an issue with the row (and I the initial position of the 2+ notification is correct), I believe this is an issue with slot_after_previous (which is used in [lua] index 2+ of the stages configuration).

Here is what I have after swapping things around:

local notify = { col_offset = 1 }

local function get_stages()
  local stages_util = require("notify.stages.util")

  return {
    function(state)
      local next_row = stages_util.available_slot(
        state.open_windows,
        state.message.height + 2,
        stages_util.DIRECTION.TOP_DOWN
      )

      if not next_row then
        return nil
      end

      return {
        relative = "editor",
        anchor = "NE",
        width = state.message.width,
        height = state.message.height,
        col = notify.col_offset,
        row = next_row,
        border = "rounded",
        style = "minimal",
        opacity = 0,
      }
    end,
    function(state, win)
      return {
        opacity = { 100 },
        col = { notify.col_offset },
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.TOP_DOWN),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
    function(state, win)
      return {
        col = { notify.col_offset },
        time = true,
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.TOP_DOWN),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
    function(state, win)
      return {
        width = {
          1,
          frequency = 2.5,
          damping = 0.9,
          complete = function(cur_width)
            return cur_width < 3
          end,
        },
        opacity = {
          0,
          frequency = 2,
          complete = function(cur_opacity)
            return cur_opacity <= 4
          end,
        },
        col = { notify.col_offset },
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.TOP_DOWN),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
  }
end
rcarriga commented 2 years ago

It seems to be working fine for me, are you sure you didn't miss changing one of the direction arguments to BOTTOM_UP? Can you provide the config that isn't working for bottom up?

levouh commented 2 years ago

This is what I am currently using (notify.col_offset = 1):

  return {
    function(state)
      local next_row = stages_util.available_slot(
        state.open_windows,
        state.message.height + 2,
        stages_util.DIRECTION.BOTTOM_UP
      )

      if not next_row then
        return nil
      end

      return {
        relative = "editor",
        anchor = "NE",
        width = state.message.width,
        height = state.message.height,
        col = notify.col_offset,
        row = next_row,
        border = "rounded",
        style = "minimal",
        opacity = 0,
      }
    end,
    function(state, win)
      return {
        opacity = { 100 },
        col = { notify.col_offset },
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.BOTTOM_UP),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
    function(state, win)
      return {
        col = { notify.col_offset },
        time = true,
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.BOTTOM_UP),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
    function(state, win)
      return {
        width = {
          1,
          frequency = 2.5,
          damping = 0.9,
          complete = function(cur_width)
            return cur_width < 3
          end,
        },
        opacity = {
          0,
          frequency = 2,
          complete = function(cur_opacity)
            return cur_opacity <= 4
          end,
        },
        col = { notify.col_offset },
        row = {
          stages_util.slot_after_previous(win, state.open_windows, stages_util.DIRECTION.BOTTOM_UP),
          frequency = 3,
          complete = function()
            return true
          end,
        },
      }
    end,
  }

is what I'm using. I still cannot upload pictures or GIFs for some reason, otherwise I would show you what it looks like @rcarriga.

rcarriga commented 2 years ago

Still can't reproduce unfortunately. I'm thinking there's some setting you have enabled/disabled that the previous slot function doesn't account for.

Can you try with this init.lua

nvim --clean -u init.lua

-- ignore default config and plugins
vim.opt.runtimepath:remove(vim.fn.stdpath("config"))
vim.opt.packpath:remove(vim.fn.stdpath("data") .. "/site")
vim.opt.termguicolors = true

-- append test directory
local test_dir = "/tmp/nvim-config"
vim.opt.runtimepath:append(vim.fn.expand(test_dir))
vim.opt.packpath:append(vim.fn.expand(test_dir))

-- install packer
local install_path = test_dir .. "/pack/packer/start/packer.nvim"
local install_plugins = false

if vim.fn.empty(vim.fn.glob(install_path)) > 0 then
  vim.cmd("!git clone https://github.com/wbthomason/packer.nvim " .. install_path)
  vim.cmd("packadd packer.nvim")
  install_plugins = true
end

local packer = require("packer")

packer.init({
  package_root = test_dir .. "/pack",
  compile_path = test_dir .. "/plugin/packer_compiled.lua",
})

packer.startup(function(use)
  use("wbthomason/packer.nvim")

  use({
    "rcarriga/nvim-notify",
    config = function()
      local stages_util = require("notify.stages.util")
      require("notify").setup({
        background_colour = "#000000",
        stages = {
          function(state)
            local next_row = stages_util.available_slot(
              state.open_windows,
              state.message.height + 2,
              stages_util.DIRECTION.BOTTOM_UP
            )

            if not next_row then
              return nil
            end

            return {
              relative = "editor",
              anchor = "NE",
              width = state.message.width,
              height = state.message.height,
              col = 1,
              row = next_row,
              border = "rounded",
              style = "minimal",
              opacity = 0,
            }
          end,
          function(state, win)
            return {
              opacity = { 100 },
              col = { 1 },
              row = {
                stages_util.slot_after_previous(
                  win,
                  state.open_windows,
                  stages_util.DIRECTION.BOTTOM_UP
                ),
                frequency = 3,
                complete = function()
                  return true
                end,
              },
            }
          end,
          function(state, win)
            return {
              col = { 1 },
              time = true,
              row = {
                stages_util.slot_after_previous(
                  win,
                  state.open_windows,
                  stages_util.DIRECTION.BOTTOM_UP
                ),
                frequency = 3,
                complete = function()
                  return true
                end,
              },
            }
          end,
          function(state, win)
            return {
              width = {
                1,
                frequency = 2.5,
                damping = 0.9,
                complete = function(cur_width)
                  return cur_width < 3
                end,
              },
              opacity = {
                0,
                frequency = 2,
                complete = function(cur_opacity)
                  return cur_opacity <= 4
                end,
              },
              col = { 1 },
              row = {
                stages_util.slot_after_previous(
                  win,
                  state.open_windows,
                  stages_util.DIRECTION.BOTTOM_UP
                ),
                frequency = 3,
                complete = function()
                  return true
                end,
              },
            }
          end,
        },
      })
      vim.notify = require("notify")
    end,
  })

  if install_plugins then
    packer.sync()
  end
end)
levouh commented 2 years ago

Good point - I should have started with this as well. The above minimal configuration does not see the same issues, so I will start to bisect mine to see where things are going wrong and report back.

levouh commented 2 years ago

@rcarriga I was able to reproduce it with the init.lua file you provided above, and executing :luafile % on the following in a buffer:

vim.schedule(function()
  vim.notify("foo")

  vim.defer_fn(function()
    vim.notify({"bar", "baz"})

  end, 1500)
end)

Also media works now so here's what it looks like from my side:

https://user-images.githubusercontent.com/31262046/186188042-2def25b2-fedf-4742-b4a5-2cacd9d9c26a.mp4

levouh commented 2 years ago

Not sure if it makes a difference @rcarriga, but when using (the first vim.notify is called immediately, rather than scheduled for later):

vim.notify("foo")

vim.defer_fn(function()
  vim.notify({"bar", "baz"})
end, 1500)

note that the top-most notification sliding down over the bottom one no longer happens:

https://user-images.githubusercontent.com/31262046/186451764-d55373b8-45a8-4c99-960c-7599e856b213.mp4

however the top-most notification doesn't move itself all the way down to the bottom either.

levouh commented 2 years ago

Spamming a bit with debugging notes - apologies in advance.


The print statements below print out next_row for the first stages function, which shows us where the window starts. The subsequent prints are for the second stages function with all other stages functions removed.

For the first window (1007):

image

and for the second window (1008) with some co-mingling of the first window as well:

image

it starts in the right place with the return value from available_slot, but then the return value from slot_after_previous ends up pushing it down by one. Later when 1007 is gone and 1008 slides down, we can also see that it is off by one (too high):

image

Will keep looking as time permits and post here if I find any updates, but hopefully this helps narrow in on what the problem actually is.

rcarriga commented 2 years ago

I was able to reproduce the issue with the second notification not going to the bottom and have fixed it, pull the latest master to confirm. Unfortunately, I'm not seeing that issue with the second appearing over the first though

levouh commented 2 years ago

Interesting that it does not reproduce for you. When you are testing, what are the values of vim.opt.columns:get() and vim.opt.lines:get()? I wonder if it has something to do with the editor size where I am running it versus where you are @rcarriga.

Also note the example the reproduces the issue has a top level vim.schedule wrapper. I cannot imagine how this makes a difference beyond timing of things though.

The issue with notifications not going back down all the way is fixed per your last commit, however the overlapping issue still exists.

rcarriga commented 2 years ago

OK figured out the issue. Please try out master now :smile:

levouh commented 2 years ago

Things are flip-flopped now. The issue with notifications overlapping is fixed, however now the second notification doesn't go all the way to the bottom of the screen.

Appreciate the effort either way though @rcarriga. Let me know if you want me to try to setup a minimal config to reproduce the "not going all the way to the bottom" issue.

rcarriga commented 2 years ago

I found an issue with cmdheight=0, did you have that set? Could you try lastest master?

levouh commented 2 years ago

I have cmdheight=1 - will try the latest master.

levouh commented 2 years ago

@rcarriga as far as I can tell it works:

https://user-images.githubusercontent.com/31262046/188733441-f501926f-36b7-4ce9-bdf6-847b8d84e43e.mp4

There's a bit of jitter at the end there, but it goes down to the correct spot for sure. Thanks for all the work on this!

rcarriga commented 2 years ago

Brilliant :smile: I've updated the rounding to help a little with the jitter slightly. You can also tweak the damping/frequency options to change the spring physics but unfortunately rendering in a terminal has its limitations