Skip to content

Use user scope ini file for settings and pass to Core on Windows - #9018

Merged
sithlord48 merged 2 commits into
masterfrom
fix-daemon
Oct 15, 2025
Merged

Use user scope ini file for settings and pass to Core on Windows#9018
sithlord48 merged 2 commits into
masterfrom
fix-daemon

Conversation

@nbolton

@nbolton nbolton commented Oct 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #8998

Todo:

  • Fix clang format issues (my Windows machine has bad clang-format version)
  • Test dev env on all OS
  • Test packages on fresh Windows VM (no ACL issues)
  • Tidy up commits

nbolton
nbolton commented Oct 9, 2025
View reviewed changes
Comment thread src/lib/gui/dialogs/SettingsDialog.cpp
nbolton
nbolton commented Oct 9, 2025
View reviewed changes
Comment thread src/lib/gui/core/CoreProcess.cpp Outdated
@github-actions

This comment was marked as outdated.

Sign in to view
sithlord48
sithlord48 reviewed Oct 9, 2025
View reviewed changes
Comment thread src/lib/gui/core/CoreProcess.cpp Outdated
@nbolton
nbolton force-pushed the fix-daemon branch 8 times, most recently from 77356f6 to 2e379b4 Compare October 14, 2025 16:11
@nbolton
nbolton marked this pull request as ready for review October 14, 2025 16:33
@nbolton
nbolton requested a review from sithlord48 October 14, 2025 16:33
sithlord48
sithlord48 requested changes Oct 14, 2025
View reviewed changes

@sithlord48 sithlord48 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The last commit is overly complex
We only are portable if the settings file is in appDir/settings/appName.conf

Comment thread src/lib/gui/dialogs/SettingsDialog.cpp Outdated
sithlord48
sithlord48 reviewed Oct 14, 2025
View reviewed changes
Comment thread src/lib/common/Settings.cpp
sithlord48
sithlord48 reviewed Oct 14, 2025
View reviewed changes
Comment thread src/lib/common/Settings.cpp Outdated
sithlord48
sithlord48 reviewed Oct 14, 2025
View reviewed changes
Comment thread src/lib/common/Settings.cpp Outdated
@nbolton

nbolton commented Oct 14, 2025

Copy link
Copy Markdown
Member Author

The last commit is overly complex

We only are portable if the settings file is in appDir/settings/appName.conf

Ah yes, we can use logic instead and lose a member. Is that what you're thinking?

@nbolton
nbolton marked this pull request as draft October 15, 2025 08:59
@nbolton

nbolton commented Oct 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Seems that some of the complexity is coming from this:

deskflow/src/lib/common/Settings.cpp

Lines 218 to 221 in 8c632a0

#ifdef Q_OS_WIN
if (!isPortableMode())
return SystemDir;
#endif

IIRC, I tried removing it so we don't use 'Program Data' but something didn't work quite right (my memory is foggy). I can try again, but using 'Program Data' is how it works right now in master, so maybe we should land this PR as-is since it fixes the problem and approach the 'Program Data' issue later (if it is an issue).

@nbolton
nbolton requested a review from sithlord48 October 15, 2025 09:36
@nbolton
nbolton marked this pull request as ready for review October 15, 2025 09:36
This was referenced Oct 15, 2025
Closed
Closed
sithlord48
sithlord48 reviewed Oct 15, 2025
View reviewed changes
Comment thread src/lib/common/Settings.cpp Outdated
@sithlord48

sithlord48 commented Oct 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Seems that some of the complexity is coming from this:

deskflow/src/lib/common/Settings.cpp

Lines 218 to 221 in 8c632a0

#ifdef Q_OS_WIN
if (!isPortableMode())
return SystemDir;
#endif

IIRC, I tried removing it so we don't use 'Program Data' but something didn't work quite right (my memory is foggy). I can try again, but using 'Program Data' is how it works right now in master, so maybe we should land this PR as-is since it fixes the problem and approach the 'Program Data' issue later (if it is an issue).

I will leave that up to you since it only affects windows I can't test correctly anyway .

@nbolton
nbolton requested a review from sithlord48 October 15, 2025 12:11
@nbolton
nbolton enabled auto-merge (rebase) October 15, 2025 12:11
sithlord48
sithlord48 reviewed Oct 15, 2025
View reviewed changes
Comment thread src/lib/common/Settings.cpp
@nbolton
nbolton marked this pull request as draft October 15, 2025 12:24
auto-merge was automatically disabled October 15, 2025 12:24

Pull request was converted to draft

@nbolton
nbolton requested a review from sithlord48 October 15, 2025 12:29
@nbolton
nbolton marked this pull request as ready for review October 15, 2025 12:29
@nbolton
feat: Change to local user ini (not native)
f647196 
Windows registry is horrible to deal with and makes it impractical to pass settings to the Core when run via daemon on Windows.

- Pass settings path to Core when launched via daemon
- Introduce portable mode detection logic on Windows
- Generalize `m_settingsFile` use
- Reduce #ifdef size for Settings ctor path logic
@nbolton
refactor: Rename setSettingFile to setSettingsFile for consistency
3dc62f2
@sithlord48
sithlord48 enabled auto-merge (rebase) October 15, 2025 12:45
@sithlord48
sithlord48 merged commit 687fd54 into master Oct 15, 2025
30 checks passed
@sithlord48
sithlord48 deleted the fix-daemon branch October 15, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sithlord48 sithlord48 sithlord48 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deskflow: daemon broken with 1.24.0.63

2 participants

@nbolton @sithlord48