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.

Sign up for GitHub

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

Jump to bottom

fix: #8107, Remove Old tray code #8109

Merged
merged 1 commit into from
Jan 23, 2025
Merged

fix: #8107, Remove Old tray code #8109

merged 1 commit into from
Jan 23, 2025

Conversation

sithlord48
Copy link
Collaborator

@sithlord48 sithlord48 commented Jan 19, 2025 •
edited
Loading

Fixes #8107
Removes all old tray related code

@sithlord48 sithlord48 requested a review from nbolton January 19, 2025 19:44
nbolton
nbolton reviewed Jan 20, 2025
View reviewed changes
Copy link
Member

@nbolton nbolton left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think we should also change the --no-tray arg handler and --help so it mentions this arg is deprecated and has no effect.

@sithlord48
Copy link
Collaborator Author

The tray will still show up for windows just won't change

nbolton
nbolton requested changes Jan 21, 2025
View reviewed changes
Copy link
Member

@nbolton nbolton left a comment

Choose a reason for hiding this comment

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

This PR does not fix #8107.

We should remove the core legacy tray entirely. It is not used anymore; we use the GUI tray now instead.

Making changes to *TaskBarReceiver.cpp is a waste of time.

To fix fix #8107 we need to remove the dead code:

MSWindowsClientTaskBarReceiver.cpp
MSWindowsClientTaskBarReceiver.h
OSXClientTaskBarReceiver.cpp
OSXClientTaskBarReceiver.h
XWindowsClientTaskBarReceiver.cpp
XWindowsClientTaskBarReceiver.h
MSWindowsServerTaskBarReceiver.cpp
MSWindowsServerTaskBarReceiver.h
OSXServerTaskBarReceiver.cpp
OSXServerTaskBarReceiver.h
XWindowsServerTaskBarReceiver.cpp
XWindowsServerTaskBarReceiver.h

We should also do this:

I think we should also change the --no-tray arg handler and --help so it mentions this arg is deprecated and has no effect.

@sithlord48
Copy link
Collaborator Author

sithlord48 commented Jan 21, 2025 via email •
edited
Loading

@sithlord48 sithlord48 requested a review from nbolton January 21, 2025 12:11
@nbolton
Copy link
Member

nbolton commented Jan 21, 2025

When you run the daemon it will show the tray icon on windows for that reason we can not remove the tray icon

I disagree.

The daemon does not have a tray icon (when I created the daemon I knew this would be pointless since it runs in the system context). When we send the client or server command to the daemon via IPC from the GUI, we always use --no-tray. If we didn't then we'd have 2 trays. Nobody has used the legacy core tray since like... 2011 (when we switched from the classic GUI to QSynergy).

The legacy tray code needs to go. It's dead code.

@sithlord48 sithlord48 force-pushed the rmLegayWindowsIcons branch 5 times, most recently from a0aa72a to ec1a617 Compare January 21, 2025 17:34
@sithlord48
Copy link
Collaborator Author

sithlord48 commented Jan 21, 2025 •
edited
Loading

This removes the icons but not those tray classes completely you want them completely removed now ?

This now removes all the old tray code.

@sithlord48 sithlord48 marked this pull request as draft January 21, 2025 18:13
@sithlord48 sithlord48 force-pushed the rmLegayWindowsIcons branch 3 times, most recently from 5945f6f to 066be7a Compare January 21, 2025 19:10
@sithlord48 sithlord48 changed the title fix: #8107, rm dynamic tray icons on win32 server / client fix: #8107, Remove Old tray code Jan 21, 2025
@sithlord48 sithlord48 force-pushed the rmLegayWindowsIcons branch 3 times, most recently from 723bacb to 850d238 Compare January 21, 2025 21:14
@sithlord48 sithlord48 marked this pull request as ready for review January 21, 2025 21:27
@sithlord48 sithlord48 force-pushed the rmLegayWindowsIcons branch 2 times, most recently from 124bb9c to 18e2579 Compare January 21, 2025 21:50
@nbolton
Copy link
Member

nbolton commented Jan 23, 2025 •
edited
Loading

I figured maybe a deprecation warning might be more helpful:

[2025-01-23T17:15:12] FATAL: deskflow-server: unrecognized option `--no-tray'
Try `deskflow-server --help' for more information.
        /home/nick/Projects/deskflow/src/lib/deskflow/ArgParser.cpp:58

i.e. we handle --no-tray as before but print something like:
the --no-tray option is deprecated and will soon be removed

Thoughts?

@sithlord48
Copy link
Collaborator Author

sithlord48 commented Jan 23, 2025 •
edited
Loading

I figured maybe a deprecation warning might be more helpful:

[2025-01-23T17:15:12] FATAL: deskflow-server: unrecognized option `--no-tray'
Try `deskflow-server --help' for more information.
        /home/nick/Projects/deskflow/src/lib/deskflow/ArgParser.cpp:58

i.e. we handle --no-tray as before but print something like: the --no-tray option is deprecated and will soon be removed

Thoughts?

I just figured it was easier to remove them then print a message and should not do much harm. we don't ship any scripts or service files externally that would need this as an example

@nbolton nbolton merged commit 2040df4 into master Jan 23, 2025
28 checks passed
@nbolton nbolton deleted the rmLegayWindowsIcons branch January 23, 2025 19:16
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.

Remove legacy tray icon code
2 participants
@sithlord48 @nbolton