refactor(logger): adapt to new changes upstream

[Tastyep]

Description

Refactor structlog usage for future 0.2 release

• Add a flag for disabling vim.notify logging as notification (to discuss)

How Has This Been Tested?

• Run lvim
• Log something
• Check that it is logged into all sinks

Related work

Tastyep/structlog.nvim#6

[LostNeophyte]

@Tastyep there's a bug here, it doesn't allow setting async to false, false or true returns true

[Tastyep]

@Tastyep there's a bug here, it doesn't allow setting async to false, false or true returns true

Thanks for the heads-up :)
Tastyep/structlog.nvim@fbfec9d

[LostNeophyte]

Something like this is needed, otherwise there are errors after updating, we can't be sure that the structlog version is always the one specified in the snapshot

diff --git a/lua/lvim/core/log.lua b/lua/lvim/core/log.lua
index 2602e64..27adc2a 100644
--- a/lua/lvim/core/log.lua
+++ b/lua/lvim/core/log.lua
@@ -13,21 +13,27 @@ local notify_opts = {}
 local log_notify_as_notification = false
 
 function Log:set_level(level)
-  local logger_ok, logger = pcall(function()
-    return require("structlog").get_logger "lvim"
-  end)
-  local log_level = Log.levels[level:upper()]
-  if logger_ok and logger and log_level then
-    for _, pipeline in ipairs(logger.pipelines) do
-      pipeline.level = log_level
-    end
-  end
+  if
+    not pcall(function()
+      local logger_ok, logger = pcall(function()
+        return require("structlog").get_logger "lvim"
+      end)
+      local log_level = Log.levels[level:upper()]
+      if logger_ok and logger and log_level then
+        for _, pipeline in ipairs(logger.pipelines) do
+          pipeline.level = log_level
+        end
+      end
 
-  local packer_ok, _ = xpcall(function()
-    require("packer.log").cfg { log = { level = level } }
-  end, debug.traceback)
-  if not packer_ok then
-    vim.notify_once("Unable to set packer's log level to " .. level)
+      local packer_ok, _ = xpcall(function()
+        require("packer.log").cfg { log = { level = level } }
+      end, debug.traceback)
+      if not packer_ok then
+        vim.notify_once("Unable to set packer's log level to " .. level)
+      end
+    end)
+  then
+    vim.cmd "echo 'structlog version too old, run `:Lazy sync`'"
   end
 end
 
@@ -130,11 +136,17 @@ end
 ---@param msg any
 ---@param event any
 function Log:add_entry(level, msg, event)
-  local logger = self:get_logger()
-  if not logger then
-    return
+  if
+    not pcall(function()
+      local logger = self:get_logger()
+      if not logger then
+        return
+      end
+      logger:log(level, vim.inspect(msg), event)
+    end)
+  then
+    vim.cmd "echo 'structlog version too old, run `:Lazy sync`'"
   end
-  logger:log(level, vim.inspect(msg), event)
 end
 
 ---Retrieves the handle of the logger object

[Tastyep]

Indeed, it is not something I thought about, since when testing it I removed the directory manually. Could it help if structlog would expose its version number (structlog.version) ?
Maybe in the future it would make sens to implement some kind of configuration migration mechanism on structlog's side.

On another topic, I added the local log_notify_as_notification variable. The intent is just to show that messages can be discarded by using the SinkAdapter. I understand that since we redirect vim.notify calls to the logger, it would generate a notification for each call, which are way too many. Maybe something that could be considered is to disable notifications when the entry point is vim.notify, but enable them when the logger is used directly (by lvim). Tell me what you think about it.

[LostNeophyte]

Could it help if structlog would expose its version number (structlog.version)

With this we could add a mechanism update structlog on startup if the version is too old, and then remove it on next release, but I'm not sure it it's worth it, updates between releases remove the plugins directory anyway so it's just for master users

On another topic, I added the local log_notify_as_notification variable. The intent is just to show that messages can be discarded by using the SinkAdapter. I understand that since we redirect vim.notify calls to the logger, it would generate a notification for each call, which are way too many. Maybe something that could be considered is to disable notifications when the entry point is vim.notify, but enable them when the logger is used directly (by lvim). Tell me what you think about it.

configure_notifications isn't called anywhere currently, I wasn't on the team when it was so I'm not the best person to answer this, but personally I think vim.notify messages should be shown to the user

[kylo252]

Thanks!

[kylo252]

On another topic, I added the local log_notify_as_notification variable. The intent is just to show that messages can be discarded by using the SinkAdapter. I understand that since we redirect vim.notify calls to the logger, it would generate a notification for each call, which are way too many. Maybe something that could be considered is to disable notifications when the entry point is vim.notify, but enable them when the logger is used directly (by lvim). Tell me what you think about it.

Log:configure_notifications(nvim_notify) has been more or less unsupported since we dropped nvim-notify. Let's merge this for now since it's blocking the CI, and we can consider further changes to it later.

[yannk]

I had to go into ~/.local/share/lunarvim/site/pack/lazy/opt/structlog.nvim and force checkout 45b26a2 after this was merged to have lvim not complain about this.

pack/lazy/opt/structlog.nvim/lua/structlog/logger.lua:49: bad argument #1 to 'ipairs' (table expected, got nil)

@kylo252 @LostNeophyte

[LostNeophyte]

I had to go into ~/.local/share/lunarvim/site/pack/lazy/opt/structlog.nvim and force checkout 45b26a2 after this was merged to have lvim not complain about this.
pack/lazy/opt/structlog.nvim/lua/structlog/logger.lua:49: bad argument #1 to 'ipairs' (table expected, got nil)

that's what I was talking about here #3695 (comment),
pr to fix the issue #3755