Skip to content

Conversation

@rinkulu
Copy link
Contributor

@rinkulu rinkulu commented Dec 8, 2025

Config values no_systray and always_ontop are saved as booleans (internally stored as strings in the .toml file); however, the code that uses these values attempts to read them with the get_int method. This method cannot parse the strings "True" and "False" and always defaults to 0 when encountering them.

From config.__init__.Config:

    def get_int(self, key: str, default=0) -> int:
        """Adaptive int (handles booleans stored as ints)."""
        val = self.get(key.lower())
        if isinstance(val, int):
            return val
        try:
            return int(val)
        except (ValueError, TypeError):
            return default

Note that this docstring is misleading and does not represent the actual behavior of this method (or maybe I'm just stupid and can't catch its meaning).

This causes the application to de facto ignore these user settings. This PR fixes the problem by changing the related calls to get_bool, also removing excessive constructions around them (e.g., bool(config.get_int(...))).

These changes also fix cases when EDMC would require a restart every time the user saved settings if no_systray is set to True.

Another solution to this issue might be rewriting the get_int method to make it correctly handle boolean-like strings similarly as get_bool does.

@dvdmuckle
Copy link
Contributor

IMO I feel like if you want to get a bool, you should get a bool, so I'm not sure about honoring the docstring here. Then again, if that was in fact the behavior in 5.13.3, we should honor it.

@Rixxan Rixxan added this to the 6.1.0 milestone Dec 10, 2025
@rinkulu rinkulu mentioned this pull request Dec 13, 2025
2 tasks
@Rixxan Rixxan self-assigned this Dec 13, 2025
@Rixxan Rixxan merged commit 6efe205 into EDCD:develop Dec 13, 2025
3 checks passed
@Rixxan Rixxan modified the milestones: 6.1.0, 6.0 Dec 13, 2025
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.

3 participants