rcarriga / nvim-notify

A fancy, configurable, notification manager for NeoVim
MIT License
3.09k stars 83 forks source link

Race condition while updating notification #87

Closed Shatur closed 2 years ago

Shatur commented 2 years ago

The following code updates notification on every new line of the output after pressing <Leader>gs:

local Job = require('plenary.job')

local function run_command()
  local notification
  local output = ''
  local on_data = function(_, data)
    -- output = output .. table.concat(data, '\n')
    output = output .. '\n' .. data
    notification = vim.notify(output, vim.log.levels.INFO, { replace = notification })
  end

  local job = Job:new({
    command = 'git',
    args = { 'status' },
    on_stdout = vim.schedule_wrap(on_data),
    on_stderr = vim.schedule_wrap(on_data),
  })

  job:start()
end

vim.keymap.set('', '<Leader>gs', run_command, { noremap = true })

But on my machine I see a notification with only first line. But when I run :Telescope notify I have a full message.

rcarriga commented 2 years ago

So what's actually happening is the window doesn't automatically resize to fit the message. The reason for this is that there can be other notifications on the screen that would get hidden. Of course the rendering logic could attempt to move other windows but that'd mean more complex logic and handling not enough space somehow.

There are two simple workarounds:

  1. Pre-allocate the size with empty lines, but that'd require knowing the content size
  2. Manually resize the window.

Something like this should do

local Job = require("plenary.job")

local function run_command()
  local notification
  local output = ""
  local length = 0
  local win, height
  local on_data = function(_, data)
    -- output = output .. table.concat(data, '\n')
    output = output .. "\n" .. data
    notification = vim.notify(output, vim.log.levels.INFO, {
      replace = notification,
      on_open = function(win_)
        win, height = win_, vim.api.nvim_win_get_height(win_)
      end,
    })

    vim.api.nvim_win_set_height(win, height + length)
    length = length + 1
  end

  local job = Job:new({
    command = "git",
    args = { "status" },
    on_stdout = vim.schedule_wrap(on_data),
    on_stderr = vim.schedule_wrap(on_data),
  })

  job:start()
end
Shatur commented 2 years ago

Thanks!

Shatur commented 2 years ago

@rcarriga there is another weird behavior with this code. Executing it code twice (in chain, one after another) leads to attempt to perform arithmetic on upvalue 'height' (a nil value).

rcarriga commented 2 years ago

This is an actual race condition because the height is being set before the window opens. Just check height is not nil

    if height then
      vim.api.nvim_win_set_height(win, height + length)
    end
Shatur commented 2 years ago

Oh, thanks!