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

Tray Icon for Conflicts #140

Open
3 of 6 tasks
metal450 opened this issue Jun 4, 2022 · 50 comments
Open
3 of 6 tasks

Tray Icon for Conflicts #140

metal450 opened this issue Jun 4, 2022 · 50 comments

Comments

@metal450
Copy link

metal450 commented Jun 4, 2022

Relevant components

  • Standalone tray application (based on Qt Widgets)
  • Plasmoid/applet for Plasma desktop
  • Dolphin integration
  • Command line tool (syncthingctl)
  • Integrated Syncthing instance (libsyncthing)
  • Backend libraries

Is your feature request specific to a certain platform/environment? Please specify.
Nope

Is your feature request related to a problem? Please describe.
SyncThing Tray's icons are very for showing when a sync is in process, or there's some error state. However, one status that is easily missed are conflicts - I often find myself navigating into the sync dir, only to discover that there's been a conflict there for awhile that went unnoticed. It would be very useful if the tray icon could also show an indicator when a conflict exists (reusing the "warning" indicator would be fine IMO, so as to eliminate the need for any extra UI work, though a separate/explicit icon would also be great too). A different tray icon to indicate a conflict is really the sole feature I miss from SyncTrayzor, which is Windows only.

Describe the solution you'd like
When a conflict is present, show the tray icon in a warning state (or a separately configurable UI icon).

Describe alternatives you've considered
n/a

Additional context
n/a

@Martchus
Copy link
Owner

Martchus commented Jun 4, 2022

Sounds useful, indeed. Not sure whether the official GUI shows this information (and therefore Syncthing provides an API to query it). If not, maybe it'll be better to implement it in Syncthing first (instead of doing this work on GUI level).

@metal450
Copy link
Author

metal450 commented Jun 4, 2022

Hmm, not sure how SyncTrayzor detects it then? They actually have a whole built-in conflict resolver too, which is pretty cool (screenshot below), but perhaps a bit overkill haha. Still, they seem to be detecting with a lot of specificity exactly what files are conflicting & where

2022-06-04 11 26 13

2022-06-04 11 27 32

@Martchus
Copy link
Owner

Martchus commented Jun 5, 2022

That yellow notification doesn't look in-line with the official UI at all so I suppose it is custom code. I personally would refrain from doing any file lookups or even modifications in the GUI level. That's the job of the Syncthing backend and having two processes manipulating those files is rather messy. Doing it in the GUI also has the disadvantage that this particular feature would only work locally and if the GUI process has sufficient read/write access (which might differ from the backend process). So it makes more sense to check what the official API has to offer and perhaps create an upstream Syncthing feature request first.

@metal450
Copy link
Author

metal450 commented Jun 5, 2022

That yellow notification doesn't look in-line with the official UI at all

Oh yeah, that window essentially wraps the browser UI. More or less like showing a notification at the top of a Firefox window.

I personally would refrain from doing any file lookups or even modifications in the GUI level. That's the job of the Syncthing backend and having two processes manipulating those files is rather messy.

As far as file modifications, yeah, no need to mess with files or a resolver or anything - was just showing how that other app does it out of possible interest.

Doing it in the GUI also has the disadvantage

Just to clarify, all I was really hoping for is a distinguishing tray icon & nothing more. Knowing that a conflict exists somewhere is more than sufficient (can just look at the log to find it if not immediately apparent). The main problem is when there's one, but I'm not aware of it.

So it makes more sense to check what the official API has to offer and perhaps create an upstream Syncthing feature request first.

Looks like this is where SyncTrayzor's author was discussing it some years back: https://forum.syncthing.net/t/conflict-notification-via-rest-api/9255/6

"If you watch both the “conflict created” events, and the “file updated” events, you should catch all conflicts."

@Martchus
Copy link
Owner

Martchus commented Jun 5, 2022

Ok, although I'm currently not sure to which exact events (from the documentation) these correspond to. Maybe it is also required to use the normal REST-API to get the initial status.

Normally I'm just checking what requests the official GUI does and do the same but it seems that the official GUI doesn't show this info so far.

@metal450
Copy link
Author

metal450 commented Jun 5, 2022

Hmm...Looking at that, it seems the ones that would make the most sense are

https://docs.syncthing.net/events/localchangedetected.html and
https://docs.syncthing.net/events/remotechangedetected.html.

Then there's a notification each time a file has changed, & it's only necessary to quickly glance at its filename to see if it contains the conflict string, right? If it does, set the conflict icon & store the filename in hashmap (so that if a future change, whether local or remote "un-conflicts" the same file, it would remove the icon).

@metal450
Copy link
Author

metal450 commented Jun 5, 2022

Normally I'm just checking what requests the official GUI does and do the same but it seems that the official GUI doesn't show this info so far.

Yeah, to be honest I'm not sure how useful this would even really be in the official GUI, since most people running SyncThing just let it go in the background, & only open the UI when it's time to config. That's why tools like SyncThing Tray & SyncTrayzor are so useful - it provides a more UI-centric way to use the software, with tray icon indicators of what's going on, without needing to bring up the whole UI :)

@Martchus
Copy link
Owner

Martchus commented Jun 6, 2022

I'm already looking for LocalChangeDetected and RemoteChangeDetected events. However, those do not help to detect conflicts that occurred before Syncthing Tray has connected. It also doesn't look like these would be emitted for conflicts.

Yeah, to be honest I'm not sure how useful this would even really be in the official GUI

Even if not, the backend part should still be in the backend and not in the GUI. And currently it doesn't look like the backend (Syncthing itself) provides an API for this. I'll ask on the Syncthing Matrix or IRC channel for help.

@Martchus
Copy link
Owner

Martchus commented Jun 6, 2022

The main Syncthing maintainer's answer is a clear no: https://forum.syncthing.net/t/yet-another-syncthing-tray/8502/165

I suggest filing an upstream issue for such an API first.

@metal450
Copy link
Author

metal450 commented Jun 6, 2022

Yeah, looks like SyncTrayzor shows their little icon by simply looking at the filesystem too: https://github.com/canton7/SyncTrayzor/blob/1ec7e4fadd5e7e64fb56348de630b85ccab35f07/src/SyncTrayzor/Services/Conflicts/ConflictFileManager.cs#L105

@Martchus
Copy link
Owner

Martchus commented Jun 6, 2022

Still not the right approach in my opinion.

@metal450
Copy link
Author

metal450 commented Jun 7, 2022

Fair enough. Personally, I could see an argument either way: if viewed as a frontend+backend system, you're right in that it's more logical to have all file watching take place on the backend. However, I could also see it as unlikely to be added there, given that watching conflicts doesn't provide much use on the the "core" backend itself, & this tray app is more of a "non-core" addon.

Since you have more experience with their dev process, how are they typically with stuff like this? i.e. do requests normally get punted for years (& thus it's unlikely to see such a nice conflict tray icon available on Linux), or they're fairly responsive?

@Martchus
Copy link
Owner

Martchus commented Jun 7, 2022

I suppose it depends on the request and it's usefulness/effort ratio. Judging by Syncthing's code which I've read to search for an API I would guess the effort is quite high since Syncthing does not really keep track of these conflict files in the database after they have been created. But I could be totally wrong here.

Note that it is open source so you can already go ahead and implement it yourself (after clarifying whether a certain approach would be generally acceptable).

@metal450
Copy link
Author

metal450 commented Jun 8, 2022

Note that it is open source so you can already go ahead and implement it yourself

Unfortunately, outside of my ability at this time.

I would guess the effort is quite high

Yeah, my fear/assumption was the same. Kinda why I'd hoped there might be a willingness for something similar to SyncTrayzor, as even if not strictly ideal in terms of internal implementation, from a usability perspective the net result would be the same. If not then not, I guess just stick with finding out about the conflicts from the Windows side.

@metal450
Copy link
Author

For anyone else interested in this, I cobbled together a stopgap solution to notify about conflicts on Linux, if or until this gets implemented :)

#!/bin/bash

monitor(){
    # Watch for files with "sync-conflict" in the name
    inotifywait --monitor --recursive -e create,moved_to --include "sync-conflict" --format '%w%f' "$1" |
    while read NEWFILE; do
        if [[ "${NEWFILE}" =~ .*tmp$ ]]; then
            : #Ignore .tmp files (every syncing file starts as tmp, so notifying for those makes every notification happen 2x)
        else
            # Use sed to parse out just the final dirname/filename
            dirAndFilename=$(echo "${NEWFILE}" | sed -E 's/(.*)\/(.*)\//\2\//')
            # Show system notification. --hint makes it appear as syncthing.
            notify-send "${dirAndFilename}" --hint='string:desktop-entry:syncthing-ui'
        fi
    done
}

monitor "/your/syncthing/folder1/" &
monitor "/your/syncthing/folder2/" &
monitor "/your/syncthing/folder3/"

Note: If the version of inotifywait in your distro's repo doesn't support the --include option (it was added recently), install it from source per https://unix.stackexchange.com/questions/323901/how-to-use-inotifywait-to-watch-a-directory-for-creation-of-files-of-a-specific/606213#606213

Other references:

@stale
Copy link

stale bot commented Aug 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 9, 2022
@stale stale bot closed this as completed Aug 16, 2022
@Martchus Martchus reopened this Aug 16, 2022
@stale stale bot removed the stale label Aug 16, 2022
@stale
Copy link

stale bot commented Oct 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Oct 16, 2022
@stale
Copy link

stale bot commented Dec 15, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 15, 2022
@metal450
Copy link
Author

no activity != solved.

@stale stale bot removed the stale label Dec 15, 2022
Copy link

stale bot commented Nov 13, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 13, 2023
@metal450
Copy link
Author

Seems odd to have to keep commenting on this over & over & over. Shouldn't be closed unless it's actually done.

@stale stale bot removed the stale label Nov 13, 2023
@Martchus
Copy link
Owner

It is unfortunately common that people create issues, then lose interest but never close the issue. So I added the bug to keep the tracker free of such issues.

I'm afraid nobody currently wants this feature bad enough to actually take the effort of implementing it. Maybe I'll work on it at some point but I won't promise anything.

@metal450
Copy link
Author

I'm afraid nobody currently wants this feature bad enough

Not sure I'd equate "nobody is coming here to proactively request it" with "nobody would benefit from it." Many product improvements (the vast majority?) don't come from direct user requests.

@Martchus
Copy link
Owner

Martchus commented Nov 13, 2023

I didn't mean to say nobody would benefit from it. I'm just observing that nobody has created a PR yet; so apparently nobody thinks this is worth the effort. And I'm on the fence myself. (Currently e.g. porting to Plasma 6 is more important to me.)

(And by the way, we don't need an open issue for this to move forward; just someone who spends the effort to come up with a mergable solution. The stalebot closing this issue would not prevent that.)

@metal450
Copy link
Author

Sure, but if it's not an issue, it's not on the "Wish List" & would probably be forgotten?

Copy link

stale bot commented Jan 13, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 13, 2024
@metal450
Copy link
Author

not stale

@stale stale bot removed the stale label Jan 13, 2024
Copy link

stale bot commented Mar 13, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 13, 2024
@metal450
Copy link
Author

not stale

@stale stale bot removed the stale label Mar 13, 2024
Copy link

stale bot commented May 13, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 13, 2024
@metal450
Copy link
Author

not stale

@stale stale bot removed the stale label May 13, 2024
Copy link

stale bot commented Jul 12, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 12, 2024
@andreymal
Copy link

@stale "stale" doesn't mean fixed, I guess?

@stale stale bot removed the stale label Jul 12, 2024
@Martchus
Copy link
Owner

It just means noone is working on this or even just discussing this further. Not sure when/whether I'll implement this feature (or someone else will give it a try).

@soredake
Copy link

@Martchus why this bot even trying to close feature requests when they are supposed to be exempt from it?

@Martchus
Copy link
Owner

I don't know. I guess the exclusion worked at least at some point for "enhancement". Maybe it breaks if the tag contains a white-space. But I don't find it that bad to provoke some reactions also on feature requests to know whether someone is actually still interested in that. And if the ticket is eventually closed it doesn't really make a difference as well¹.


¹ Whether this is implemented or not does not depend on having an open ticket about it. It depends on someone implementing it.

@metal450
Copy link
Author

Personally I still find it odd that I have to keep coming back & saying "not stale" over & over. I interact with a lot of projects on Github & this is the only place I've ever had to do this.

Like...there really isn't a reason a long-term wishlist needs to be constantly emptied - it's just a wishlist.

I guess maybe after a few years it could be stale or something. But needing more "reactions" every 2 months to be considered as "interest hasn't been lost" seems extremely frequent.

@Martchus
Copy link
Owner

Let's see whether using "enhancement" works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants