Skip to content

fix: Resolve user home path at the time of setting#48

Merged
joelspadin merged 1 commit intozmkfirmware:mainfrom
caksoylar:fix-user-home-set
Jan 4, 2026
Merged

fix: Resolve user home path at the time of setting#48
joelspadin merged 1 commit intozmkfirmware:mainfrom
caksoylar:fix-user-home-set

Conversation

@caksoylar
Copy link
Contributor

This gets resolved when the setter is called in the code through @home_path.setter, but that setter isn't used if you set via zmk config user.home .... This can be generalized check for "path" types but I guess currently it's all strings internally. I don't think it makes sense for editor or explorer, since they will be resolved relative to PATH, not cwd at the time of setting.

@joelspadin
Copy link
Collaborator

I can't say I'm a fan of having a special case for this when setting the value, and if there is an issue with not having the path be resolved, it wouldn't help for anyone who already set the variable or who manually edited the config file. Maybe we should change Config.home_path to resolve the path before returning it instead of having Config.home_path.setter resolve it when setting it?

@caksoylar
Copy link
Contributor Author

caksoylar commented Jan 4, 2026

The problem with resolving at use is you lose the cwd of the time user set it. So if I am at ~/ and pointing to zmk config user.home zmk-config, we'd want it to always reflect ~/zmk-config.

Maybe this is a bit ugly to special case for this one use, but I imagine it'd be a common footgun. We could instead assert if the provided path is absolute as an alternative.

@joelspadin
Copy link
Collaborator

True. In that case, I would prefer adding a way to indicate which settings are paths rather than having a special case for just the USER_HOME setting. Then the resolve() call in home_path.setter is also redundant.

@caksoylar
Copy link
Contributor Author

Makes sense, yeah. Given this is currently the only such setting we could do that later. Or if you have a design in mind to incorporate the setting types, let me know (or feel free to do if it will be quicker). I haven't really used ConfigParser before so I don't know the patterns.

@joelspadin
Copy link
Collaborator

The simplest thing I can think of would be to just create a _PATH_SETTINGS = [...] constant and then check if name in _PATH_SETTINGS. Place that constant just below the definitions of the setting names, and it should be fairly obvious that it needs to be updated when adding a new setting that stores a path.

@caksoylar
Copy link
Contributor Author

OK, implemented that now.

@joelspadin joelspadin merged commit bfa01e6 into zmkfirmware:main Jan 4, 2026
1 check passed
@joelspadin
Copy link
Collaborator

Thanks!

@caksoylar caksoylar deleted the fix-user-home-set branch January 4, 2026 23:07
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