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.

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

fix: make sure config_path option for installs updates correct config #590

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

simifalaye
Copy link
Contributor

Fixes an issue with the config_path option on installs (NOTE: this only affected the new config_path feature added last PR and not anything else). This was meant to be committed as part of the last PR but I had stashed these changes temporarily while testing a slightly different implementation and then forgot to re-apply the stashed changes afterwards (I apologize for this).

The original intention was that when you provided a config_path to the parse_rocks_toml function, it would add the import to the base config and then provide you a wrapper object which wrapped both the base config and the newly created import config so that you could write both at once when done updating. The issue with this is that the goal of the config_path option is so that you could make changes ONLY to the specified config file but since I was providing a wrapped object, it meant that despite the imported config having preference, it could end up preferring the base config for writing if the nested table paths didn't exist in the import config (hence defeating the purpose of this feature). I realized this and changed it locally to asynchronously write the base config when the imports have been updated and only return the rocks_toml for the specified config_path so that all modifications would be guaranteed to be only done on the config_path but this was not committed unfortunately.

Copy link
Member

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

Thanks 🙏
CI (luacheck) is failing on some unused variables in the mut_rocks_toml_reapper module.

@mrcjkb mrcjkb merged commit 47872af into nvim-neorocks:master Nov 18, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants