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.

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

Support platforms without KDE idle mechanism #39

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

moreheadm
Copy link

@moreheadm moreheadm commented Aug 15, 2024

This change allows aw-watcher-window-wayland to work even if the KDE idle mechanism is not present. It does that by attempting to instantiate a KDE Idle object as normal, but not panicking if that fails, and instead marking the idle part of the watcher as inactive, and not making subsequent calls to it.

The background is that Issue #35 [1] reports that sway has switched idle mechanisms to not use the KDE idle mechanism. Presumably, other window managers may also fail to support the KDE idle mechanism. Pending full support for idle-notify-v1, it's still useful to use activity watch on sway to get focused window titles, so this commit allows the program to run on sway.

Some changes to Cargo.lock were necessary to compile.

[1] #35


🚀 This description was created by Ellipsis for commit 4ebb5a4

Summary:

This PR updates aw-watcher-window-wayland to handle platforms without KDE idle mechanism by modifying assign_idle_timeout to return a Result and adjusting idle event handling in src/main.rs.

Key points:

  • Update src/idle.rs to modify assign_idle_timeout to return Result<(), String> instead of panicking.
  • Handle absence of KDE idle mechanism by marking idle watcher as inactive in src/main.rs.
  • Add error handling in src/main.rs to print error message if KDE idle mechanism is not available.
  • Conditional heartbeat sending for AFK events based on idle mechanism availability in src/main.rs.

Generated with ❤️ by ellipsis.dev

This change allows aw-watcher-window-wayland to work even if the KDE
idle mechanism is not present. It does that by attempting to instantiate
a KDE Idle object as normal, but not panicking if that fails, and
instead marking the idle part of the watcher as inactive, and not making
subsequent calls to it.

The background is that Issue ActivityWatch#35 [1] reports that sway has switched idle
mechanisms to not use the KDE idle mechanism. Presumably, other window
managers may also fail to support the KDE idle mechanism. Pending full
support for idle-notify-v1, it's still useful to use activity watch on
sway to get focused window titles, so this commit allows the program to
run on sway.

Some changes to Cargo.lock were necessary to compile.

[1] ActivityWatch#35
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 4ebb5a4 in 32 seconds

More details
  • Looked at 1756 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/main.rs:182
  • Draft comment:
    Good practice to check is_idle_active before using idle functionality to prevent errors if the idle mechanism isn't available.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The code in src/main.rs checks if the idle mechanism is active before attempting to send heartbeat events related to idle status. This is a good practice as it ensures that the program does not attempt to use the idle functionality if it was not successfully initialized, which could lead to errors or unexpected behavior.

Workflow ID: wflow_wKZTy5C0dSQF9xFb


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@johan-bjareholt
Copy link
Member

Not the perfect solution, but a good start. Thanks for a simple and clean PR.

@johan-bjareholt johan-bjareholt merged commit 98d81e1 into ActivityWatch:master Oct 8, 2024
1 check passed
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