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

add MarkActiveChatRead ui setting #461

Closed
wants to merge 1 commit into from

Conversation

rkitover
Copy link
Contributor

Add the ui setting MarkActiveChatRead to the chat view settings page,
and check this setting before calling Client::markBufferAsRead() in
MainWin::currentBufferChanged() and MainWin::event().

Defaults to true, which is the current behavior.

@AlD
Copy link
Member

AlD commented Jan 15, 2019

Would be nice to have a description of the intended use case in the commit message.

@digitalcircuit
Copy link
Contributor

This seems like it mimics the behavior of the "set marker line" options (on switch chat, on lose focus), but for marking a buffer as read instead.

I'm also curious about use case - say, quickly checking a buffer to see new messages but leaving it marked unread as a reminder for later? Something else..?

@rkitover
Copy link
Contributor Author

I'm sorry, I will explain:

I use the Qt Quassel client on mulitple computers with a core on my server.

Now suppose I left the client open with my favorite channel on one computer.

Then on the second computer that channel will always be marked read, even though the first computer is idle. This leads me think there are never new messages in it, which I found incredibly annoying.

My first thought was that I would use some sort of cross-platform desktop idle detection API, however Qt does not have anything like this, there are other things such as:

https://github.com/paulcbetts/node-system-idle-time

Or for a specific platform, such as X11:

https://gist.github.com/jamesadney/1879754

(if there is nothing for C++ it could probably be fairly easily ported.)

But this solves the problem in a different way, without adding dependencies or native code.

If you would prefer a solution using idle time in addition to, or instead of, this setting, I could work on that.

@AlD
Copy link
Member

AlD commented Jan 15, 2019

I see. With this setting switched off, you'd have to deselect and reselect a buffer you're currently looking at, to get it marked as read, though, IIRC.

Replacing this bool with a check to determine whether the client is in the foreground / has focus / isn't in a locked desktop session (or a combination of these) would probably be the better approach here. That may even match user expectations enough that it wouldn't need to be configurable.

@digitalcircuit
Copy link
Contributor

Aha! We've actually discussed this long ago over on Freenode/#quassel, and wanted to add support for KDE's KIdleTime.

KIdleTime is a Tier 1 KDE framework, so no additional dependencies, and it supports FreeBSD, Linux (also in the Debian/Ubuntu repos), MacOSX, and Windows. Including it in Windows would involve modifying the Craft blueprints; TheOneRing might be able to help.

(This was originally delayed for some Windows packaging improvements, which have landed by now.)

Aside: Ideally, in addition to not marking buffers as read after a configurable idle delay, the "present or idle" information should be communicated back to the Quassel core, so "away on detach" can finally be joined by "away on idle", a feature that's been in the database and protocol but not (yet) implemented.

This can be saved for a follow-up; though, the "don't mark as read when idle" is purely client-side.

@AlD
Copy link
Member

AlD commented Jan 15, 2019

I agree that having idle time as trigger for away state makes sense. For the buffer read toggle I'd prefer something tied to the window or desktop session state, as described above.

@rkitover
Copy link
Contributor Author

This seems like a better strategy, I'll try to implement something and update this PR.

@rkitover
Copy link
Contributor Author

I've experimented with the visible and active properties of QWindow, unfortunately neither changes on display sleep in KDE, no idea about other platforms.

It is starting to look like we are going to need to implement some kind of idle detection after all, but if you have other ideas I will test them out.

@rkitover
Copy link
Contributor Author

If no one has any objections, my plan is to make a small C library based on node-system-idle-time, or find one if it exists, since it will be very small it could be bundled for the time being to not deal with dependency issues.

@digitalcircuit
Copy link
Contributor

If we're making use of an external library anyways, we're already using one KFramework library from KDE, the Sonnet spell-checking library, so it might be worthwhile seeing how hard it is to add others.

Theoretically, KIdleTime support should be CMake inclusions, a few #ifdefs so it's only enabled in the client when the library's detected (see the QtWebkit/QtWebEngine support), or make it into a hard dependency, and lastly, adding a line in craft-blueprints-quassel for Windows…

 self.runtimeDependencies["kde/frameworks/tier1/kidletime"] = None

I haven't actually tested this; it's possible the project needs to be added to KDE Craft's packaging list or something.

Debian/Ubuntu already have it (libkf5idletime5); I'd be surprised if Gentoo and Arch don't. Not sure on how macOS bundles dependencies with e.g. Sonnet.

I'm not opposed to making a C library around node-system-idle-time, but I do have concerns about future maintenance work. Last commit to that repository is October 2015 (unless you're referring to other forks), and it lacks Wayland support. KIdleTime doesn't yet have Wayland support, but it's still under active maintenance, and as an external dependency, Quassel wouldn't need to do any work for it.

This is merely an opinion as a non-maintainer. Feel free to try other methods, or wait for feedback from the project leads.

@rkitover
Copy link
Contributor Author

Will this framework, if included, work correctly under non-KDE? E.g., windows, gnome/wayland, gnome/x11, macos. If you say wayland is not supported, we could just disable it there, but if the other platforms work that will be ok for the time being I think. We could also have a small #ifdef with native code for wayland.

@digitalcircuit
Copy link
Contributor

KIdleTime has plugins for Windows, macOS, XScreenSaver, and XSync. Though I haven't personally tested these, none of the code appears to be KDE-specific.

Looking at the dependencies for libkf5idletime5 on Ubuntu don't appear to pull in any KDE dependencies, either:

Depends: libc6 (>= 2.14), libqt5core5a (>= 5.5.0), libqt5dbus5 (>= 5.0.2),
libqt5gui5 (>= 5.0.2) | libqt5gui5-gles (>= 5.0.2), libqt5widgets5 (>= 5.0.2),
libqt5x11extras5 (>= 5.1.0), libstdc++6 (>= 4.1.1), libx11-6, libx11-xcb1,
libxcb-sync1, libxcb1, libxext6, libxss1

And there's a Recommends: kwayland-integration. KIdleTime appears to have Wayland integration, but that is window-manager specific.. for now:

Now as a third step the development of the interfaces was required. This happened in a new repository called kwayland-integration which will see the initial release with Plasma 5.4. So far this repository provides two plugins: one for KIdleTime, one for KWindowSystem.

For KIdleTime I designed a specific Wayland protocol which allows us to get notified when a seat went idle for a specified interval. This is an important feature for many applications, like screen brightness, setting chat to away, etc. Given that I think it’s an interface which might be in general useful and I think it could be a candidate for inclusion in Wayland. This is something I will look into after Akademy when I have more time for it.

The Wayland idle protocol proposal appears to still be stuck in discussion.. I'm not quite sure. But Xorg/Windows/macOS should be fine as is.

Former #quassel regular and past contributor rikai actually talked with others on using KIdleTime in Quassel back in 2016-9-16, and the topic of Wayland support came up there, too. In summary:

[10:08] <rikai> Well, either way, sounds like things will be solved shortly one way or another, so i can report that it should be fairly safe to use kidletime with the hopeful addition of wayland coming soon™

Apologies for long delay…

@rkitover
Copy link
Contributor Author

Thank you for the detailed information.

I suggest the way we proceed is we enable this framework, regardless of the WITH_KDE option, since it does not link to KDE.

The only platform actually in use this would not support then, is gnome/wayland.

For gnome/wayland I could perhaps figure out a way to get this information via dbus or some other mechanism not requiring building with gnome libraries.

@rkitover
Copy link
Contributor Author

Incidentally I found a solution for the Gnome/Wayland platform:

https://unix.stackexchange.com/questions/396911/how-can-i-tell-if-a-user-is-idle-in-wayland

This will be used to implement auto-away when idle.

See:

https://api.kde.org/frameworks/kidletime/html/

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
@rkitover
Copy link
Contributor Author

rkitover commented Nov 2, 2019

@AlD @digitalcircuit @justjanne @Sput42

Sorry that I haven't gotten back to this for so long, but I was just looking at it again today.

So, this is going to be much harder than I initially thought. At first I thought it would be just a few lines of code, but now it looks like it will require much more extensive changes. And my initial attempts were completely wrong.

I replaced them with a commit that adds the KIdleTime framework to cmake.

So my plan now is to make an auto-away feature based on user idle time. This will take care of the issue with the buffer being marked read, and also better communicate the user state to other users.

Notes:

  • I may use some code/ideas from this old patch in the tracker: https://bugs.quassel-irc.org/attachments/179/away.diff for any necessary, or not, away support extensions

  • Gnome/Wayland support will be better handled in the KIdleTime framework itself, I will work on that separately

  • KIdleTime does not support detecting the if/when screen-saver/blank is in effect, it only supports idle time detection, so we want to add a configuration option such as "auto away on idle (minutes)" with the default set to say 10 minutes. If set to zero the feature would be disabled.

  • we use the timeoutReached signal feature of KIdleTime to generate events every X minutes of the GUI user being idle, and use the resumeFromIdle signal to detect activity, maintaining away/idle state in the GUI. See API here: https://api.kde.org/frameworks/kidletime/html/classKIdleTime.html

  • on the timeoutReached signal the GUI marks the user as away in the client/core, and sets the marker line. In the client/core this would be similar to the GUI being detached, except still receiving data. The current buffer would not be automatically marked read. The away reason could be configurable, defaulting to something like "user went idle".

  • on the resumeFromIdle signal, the GUI marks as the user as no longer away and clears the marker line

So what do you guys think?

@rkitover
Copy link
Contributor Author

I decided to leave IRC permanently so I am not going to implement this.

@rkitover rkitover closed this Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants