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.

Sign up for GitHub

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

BugFix: hook not executed when only config is updated #3

Closed
wants to merge 5 commits into from
Closed

BugFix: hook not executed when only config is updated #3

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8a1c30
Update dot.lua
kesslerdev Nov 8, 2024
fa0764d
Implement hooks for config
kesslerdev Nov 8, 2024
61375e4
Rename hooks
kesslerdev Nov 8, 2024
6cea24a
Improve Hooks doc
kesslerdev Nov 8, 2024
51b3874
Add tests for new hooks
kesslerdev Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions dot.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,12 @@ end

local function handle_config_symlink(config, module_dir, options)
if not config.config then
return
return false
end

local configs = type(config.config) == "table" and config.config[1] and config.config or { config.config }
local all_configs_linked = true
local config_changed = false

for _, cfg in ipairs(configs) do
local source = os.getenv "PWD" .. "/" .. module_dir:gsub("^./", "") .. "/" .. cfg.source:gsub("^./", "")
Expand All @@ -387,6 +388,7 @@ local function handle_config_symlink(config, module_dir, options)
local success, err = delete_path(output)
if success then
print_message("success", "config → removed " .. output)
config_changed = true
else
print_message("error", "config → " .. err)
end
Expand All @@ -404,13 +406,14 @@ local function handle_config_symlink(config, module_dir, options)
local success, err = ensure_parent_directory(output)
if not success then
print_message("error", "config → " .. err)
return
return false
end

-- Copy source to output
local success, err = copy_path(source, output)
if success then
print_message("success", "config → copied " .. source .. " to " .. output)
config_changed = true
else
print_message("error", "config → " .. err)
end
Expand All @@ -433,19 +436,19 @@ local function handle_config_symlink(config, module_dir, options)
print_message("warning", "config → existing config backed up to " .. result)
else
print_message("error", "config → " .. result)
return
return false
end
else
print_message("error", "config → file already exists at " .. output .. ". Use -f to force.")
return
return false
end
end

-- Ensure parent directory exists
local success, err = ensure_parent_directory(output)
if not success then
print_message("error", "config → " .. err)
return
return false
end

local cmd = string.format('ln -sf "%s" "%s"', source, output)
Expand All @@ -454,6 +457,7 @@ local function handle_config_symlink(config, module_dir, options)
print_message("error", "config → failed to create symlink: " .. error_output)
else
print_message("success", "config → symlink created for " .. output)
config_changed = true
end
end
end
Expand All @@ -466,6 +470,8 @@ local function handle_config_symlink(config, module_dir, options)
elseif all_configs_linked and options.unlink_mode then
print_message("success", "all configurations are unlinked")
end

return config_changed
end

-- Process each module by installing/uninstalling dependencies and managing symlinks
Expand All @@ -490,10 +496,10 @@ local function process_module(module_name, options)

local dependencies_changed = process_brew_dependencies(config, options.purge_mode)

handle_config_symlink(config, module_dir, options)
local config_changed = handle_config_symlink(config, module_dir, options)

-- Run post_install or post_purge hooks
if dependencies_changed or options.hooks_mode then
if dependencies_changed or config_changed or options.hooks_mode then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heyyy @kesslerdev thanks so much for the PR. I didn't make it like this cause a "post-install" hook in my head only runs when you "install" the dependencies. I guess there would be an argument for your use-case, but might be better to create a "post-link" hook. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pablopunk! Thanks for the feedback. I agree, a new hook like "post-link" (and also a "post-unlink") definitely fits the use case better. I’ve updated the docs and the tests to include these hooks, so everything’s good to go. Thanks again for the suggestion! 😊

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems tests are failing

if options.purge_mode and config.post_purge then
run_hook(config.post_purge, "post-purge")
elseif not options.purge_mode and config.post_install then
Expand Down
Loading