Skip to content

fix: incorrect paths for the server configs shown in help message - #8532

Merged
sithlord48 merged 3 commits into
masterfrom
fixHelpText_configPaths
Apr 28, 2025
Merged

fix: incorrect paths for the server configs shown in help message#8532
sithlord48 merged 3 commits into
masterfrom
fixHelpText_configPaths

Conversation

@sithlord48

@sithlord48 sithlord48 commented Apr 23, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Related to #8525

When looking at the above i noticed the server configuration paths are completely wrong and the fallback is also wrong . It is safest to have the user provide them in the command like we do when running from the GUI / Daemon.

  • No longer use a fallback for the server config
  • Adjust the help message
  • Remove redundant command
  • Update log level when config can't be loaded to LOG_ERR
  • Correct the include for common/Settings.h => common/QSettingsProxy.h
  • Remove unused CONFIG_NAME variable

@sithlord48
sithlord48 requested a review from nbolton April 23, 2025 14:21
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch 3 times, most recently from de6001a to c97b4ed Compare April 23, 2025 14:28
@sithlord48 sithlord48 mentioned this pull request Apr 24, 2025
Closed
12 tasks
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch from c97b4ed to b87db39 Compare April 24, 2025 02:00
@nbolton
nbolton force-pushed the fixHelpText_configPaths branch from b87db39 to 0520536 Compare April 25, 2025 08:20
@nbolton

nbolton commented Apr 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

I am seeing failed to load config error:

[2025-04-25T09:28:39] INFO: running command: /home/nick/Projects/deskflow/build/bin/deskflow-server -f --debug DEBUG --name spock-tux --enable-crypto --disable-client-cert-check --address :24800 -c /home/nick/.config/Deskflow/deskflow-server.conf --tls-cert /home/nick/.config/Deskflow/tls/deskflow.pem
	/home/nick/Projects/deskflow/src/lib/gui/core/CoreProcess.cpp:244
[2025-04-25T09:28:39] DEBUG: core process state changed: starting -> started
	/home/nick/Projects/deskflow/src/lib/gui/core/CoreProcess.cpp:605
[2025-04-25T09:28:39] DEBUG: opening configuration ""
	/home/nick/Projects/deskflow/src/lib/deskflow/ServerApp.cpp:197
[2025-04-25T09:28:39] DEBUG: cannot open configuration ""
	/home/nick/Projects/deskflow/src/lib/deskflow/ServerApp.cpp:203
[2025-04-25T09:28:39] FATAL: deskflow-server: failed to load config: /home/nick/.config/Deskflow/deskflow-server.conf
	/home/nick/Projects/deskflow/src/lib/deskflow/ServerApp.cpp:188

Will investigate why.

nbolton
nbolton requested changes Apr 25, 2025
View reviewed changes
Comment thread src/lib/deskflow/ServerApp.cpp
Comment thread src/lib/deskflow/ServerApp.cpp Outdated
Comment thread src/lib/deskflow/ServerApp.cpp Outdated
nbolton
nbolton requested changes Apr 25, 2025
View reviewed changes
Comment thread src/lib/deskflow/ServerApp.cpp
@nbolton
nbolton force-pushed the fixHelpText_configPaths branch from 0520536 to b17f320 Compare April 25, 2025 09:08
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch from b17f320 to f493d8f Compare April 25, 2025 12:07
@sithlord48
sithlord48 requested a review from nbolton April 25, 2025 12:08
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch from f493d8f to b25428a Compare April 25, 2025 12:37
@sithlord48

sithlord48 commented Apr 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

The settings.h include is needed build without it.
common/QSettingsProxy.h needs to be included instead

@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch 2 times, most recently from 406e168 to f8ecb34 Compare April 25, 2025 13:10
nbolton
nbolton previously approved these changes Apr 25, 2025
View reviewed changes

@nbolton nbolton left a comment

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.

Tested, works fine. That #include is still a bit weird though.

Comment thread src/lib/deskflow/ServerApp.cpp Outdated
@nbolton
nbolton force-pushed the fixHelpText_configPaths branch from f8ecb34 to 84e3233 Compare April 25, 2025 16:27
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch from 84e3233 to 7eeb8da Compare April 25, 2025 16:55
@sithlord48
sithlord48 enabled auto-merge (rebase) April 25, 2025 16:56
@sithlord48
sithlord48 requested a review from nbolton April 25, 2025 17:13
nbolton
nbolton approved these changes Apr 28, 2025
View reviewed changes
Comment thread src/lib/deskflow/ServerApp.cpp
@sithlord48
refactor: remove auto fallback paths for server binary
8b7df87 
they are not correct in the help and its dynamic based on the first found Deskflow.conf file. The gui sets this manually and users running the cli should too
@sithlord48
refactor: correct includes in ServerApp
396355e
@sithlord48
chore: Remove unused CONFIG_NAME var in ServerApp
c7af23c
@sithlord48
sithlord48 force-pushed the fixHelpText_configPaths branch from 7eeb8da to c7af23c Compare April 28, 2025 11:11
@sithlord48
sithlord48 merged commit f49f5f7 into master Apr 28, 2025
@sithlord48
sithlord48 deleted the fixHelpText_configPaths branch April 28, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nbolton nbolton nbolton 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.

2 participants

@sithlord48 @nbolton