search

Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.68k stars 44 forks source link

Incorrect handling of the `~` shortcut in paths #148

Closed VorpalBlade closed 2 years ago

VorpalBlade commented 2 years ago

Relevant components

Environment and versions

Bug description The icon indicated with the red arrow in the following picture should open the directory in the file manager: syncthingtray_view

This works if the folder is configured using absolute paths. However if the ~ is used (which SyncThing allows as an alias for the home directory) syncthingtray attempts to open the path in the wrong place: Screenshot_20220731_082953

Steps to reproduce

  1. Create a SyncThing share using ~ in the path.
  2. Attempt to open it using the menu of SyncThingTray

Expected behavior It should open in the user's preferred file manager (dolphin in my case).

Screenshots Screenshots are included inline above.

Additional context Add any other context about the problem here.

Martchus commented 2 years ago

I've never been using the ~-feature of Syncthing myself much but I've just wanted to reproduce and noticed that Syncthing actually replaces the ~ with the actual path directly within its UI. So I'm wondering how you end up with directory configuration that contains ~. Did you use Syncthing's CLI edited the config manually (e.g. via syncthingctl provided by this project here)?

Note that so far Syncthing Tray does not try do do any special resolving of the path for you. That would be a new feature, so changing the label.

But if there's a valid way to end up with ~ in the config, I can implement resolving it within Syncthing Tray of course. I suppose ~ should only be substituted if it occurs at the start of the path, right?

VorpalBlade commented 2 years ago

I used the GTK frontend for Syncthing before switching to syncthingtray (when I was also using Cinnamon instead of KDE). It is possible that it added it with a ~ perhaps, I don't have it installed any more, so it would not be convenient to check. I have for sure not edited the config file, nor have I used the CLI.

However, I tested the cli just now (to make sure it wasn't just a bug in the other frontend), and the following CLI commands does add a ~ into the config:

syncthing cli config folders add --id dummy_id --label dummy --path '~/something'

From reading the docs, it isn't clear if this syntax is also supported on the command line or just in the GUI. However, it seems that everything works properly when using a ~, so I assume it is supposed to work. But again, I can't find any specific mention in syncthing documentation with a quick search that ~ is supposed to work, nor any mention that isn't supposed to work.

Strangely enough it is just one single share that uses ~ it in my current config, all others use /home/username. This would suggest that how I added it (web UI or syncthing-gtk-python3) had an effect. Perhaps also typing it in vs selecting it in a file browser would affect things.

I can't imagine a sensible way to interpret ~ except at the start of the path, and as the sole element of a path segment (i.e. ~/whatever but NOT ~whatever/something). The latter is valid UNIX syntax to refer to specific user's home directory, but I don't see any mention of syncthing supporting that anywhere.

Martchus commented 2 years ago

Ok, then I suppose it makes really sense if Syncthing Tray can handle it.

I can't imagine a sensible way to interpret ~ except at the start of the path, and as the sole element of a path segment (i.e. ~/whatever but NOT ~whatever/something)

That makes sense and matches my testing with Bash. So I'll implement it that way.

I suppose for determining the value of ~ I could just use Qt's abstraction for finding the user directory. However, for the best consistency with Syncthing itself I'd better read the "tilde" field from https://docs.syncthing.net/rest/system-status-get.html.

I also realize that this makes zero sense for remote instances where you'd get the same error as in your screenshot. Maybe the open folder buttons should be disabled in this case (which is a different issue, though).

VorpalBlade commented 2 years ago

I also realize that this makes zero sense for remote instances where you'd get the same error as in your screenshot. Maybe the open folder buttons should be disabled in this case (which is a different issue, though).

I'm unfamiliar with this feature, but wouldn't you be unable to open paths for remote instances anyway (unless you mount them via smb/nfs/sshfs/etc)? That however seems way out of scope of this project.

Martchus commented 2 years ago

Yes, you'd be unable to do so. That's why I said it makes no sense. I'd just disable the folder button for remote connections.

Martchus commented 2 years ago

Should be implemented by the added commit. Today is release day so it is also already part of the latest release.