justinhj / battery.nvim

Neovim plugin to detect and view battery information
MIT License
59 stars 9 forks source link

refactor: move parsers to a submodule, and include condition functions in parser modules #40

Closed Agent-E11 closed 2 months ago

Agent-E11 commented 3 months ago

The top-level battery module is getting a little cluttered with all the parsers. I think it would make sense to create a submodule for all the parsers:

lua/battery/parsers/
|-- init.lua
|-- acpi.lua
|-- pmset.lua
|-- ...
-- lua/battery/parsers/init.lua

local M = {}
M.parsers = {
  -- List in order of presedence
  powershell = require('powershell'),
  pmset = require('pmset'),
  acpi = require('acpi'),
  ...
}
return M

Then, we could loop over the parsers:


local parsers = require('battery.parsers')

local function select_job()
  for method, parser_module in pairs(parsers.parsers)
    if parser_module.cond() then -- Modules provide _their own_ condition functions, so that the main battery module doesn't need to worry about dependencies
      return parser_module.get_battery_info_job, method
    end
  end

  -- None of the conditions were satisfied
  return nil, nil -- Side note: return `nil, nil` so that the method check in `health.lua` fails if no method works
end
Agent-E11 commented 3 months ago

Each module's condition function can be the same as it was in the original select_job function:

-- lua/battery/parsers/powershell.lua
local M = {}

function M.cond()
  return vim.fn.has('win32') and vim.fn.executable('powershell') == 1
end
-- lua/battery/parsers/powersupply.lua
local file = require('util.file')

local M = {}

function M.cond()
  return file.is_readable_directory('/sys/class/power_supply/')
end
justinhj commented 3 months ago

yeah it seems nice.

minor thing i don't like the name cond, maybe pred or check

Agent-E11 commented 2 months ago

Ok, that is fine. I am not attached to the name

Agent-E11 commented 2 months ago

I like check, but pred would work too