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

When filename contains comma, extra unrelated folder opens #9

Open
pintassilgo opened this issue Oct 3, 2022 · 5 comments
Open

When filename contains comma, extra unrelated folder opens #9

pintassilgo opened this issue Oct 3, 2022 · 5 comments

Comments

@pintassilgo
Copy link

pintassilgo commented Oct 3, 2022

Tested on Linux, desktop environment: KDE Plasma, file manager Dolphin.

If current file has comma in its name, locatefile opens not just its folder, but also another unrelated folder, probably the system default one, in my case it's /home/user/, aka ~/.

I guess this is a missing escaping issue. locatefile should try to escape comma in path. Currently, I believe it's causing , to be interpreted by command as an argument separator, thus launching two folders instead of just one.

Log:

mpv /tmp/tst,tst.mp4 --msg-level='locatefile=debug'
[locatefile] Loading lua script /home/user/.config/mpv/scripts/locatefile.lua...
[locatefile] loading mp.defaults
[locatefile] loading file /home/user/.config/mpv/scripts/locatefile.lua
 (+) Video --vid=1 (*) (h264 720x1280 29.952fps)
 (+) Audio --aid=1 (*) (aac 1ch 44100Hz)
AO: [pulse] 44100Hz mono 1ch float
VO: [gpu] 720x1280 yuv420p
(Paused) AV: 00:00:00 / 00:00:29 (2%) A-V:  0.000
[locatefile] File detected '/tmp/tst,tst.mp4', your OS file browser will be launched. 
[locatefile] Linux detected. 
[locatefile] Command to be executed: 'dbus-send --print-reply --dest=org.freedesktop.FileManager1 /org/freedesktop/FileManager1 org.freedesktop.FileManager1.ShowItems array:string:"file:/tmp/tst,tst.mp4" string:""' 
method return time=1664780033.610051 sender=:1.575 -> destination=:1.954 serial=2768 reply_serial=2
@pintassilgo
Copy link
Author

pintassilgo commented Oct 3, 2022

I can't assure it works on every single Linux machine due to fragmentation, but this fixed it for me:

-  file_browser_linux_cmd = "dbus-send --print-reply --dest=org.freedesktop.FileManager1 /org/freedesktop/FileManager1 org.freedesktop.FileManager1.ShowItems array:string:\"file:$path\" string:\"\""
+  file_browser_linux_cmd = "dirname $path | xargs -0 xdg-open"
-  return path_root() .. table.concat(parts, path_sep())
+  local path = path_root() .. table.concat(parts, path_sep())
+  return path:gsub('"', '\\"'):gsub("'", "\\'"):gsub(" ", "\\ ")

I ditched dbus-send because this seems to be an open issue and the proposed solution is "don't use dbus-send".

I tested the solution in path with comma, single quote, double quote, space and bracket, all worked.

@HubKing
Copy link

HubKing commented Mar 16, 2023

It does not work for SMB path. It opened two windows of Gnome Files with error messages. It is an SMB path mapped by GVFS, I believe, and the format is like:

/run/user/1000/gvfs/smb-share:server=servername,share=path/on/the/smb/server

which is actually smb://servername/path/on/the/smb/server.

@HubKing
Copy link

HubKing commented Apr 14, 2023

I could made it work. Basically, the idea is replacing with %20 and , with %2C in the file path. It would have been a really simple task, if the script were written in a language I knew, but I have never used Lua before, so it took me a while to understand the bizarre syntax. Anyway, add the following two lines below line 133 in the script.

    path = path:gsub("( )", "%%%%20")
    path = path:gsub("(,)", "%%%%2C")

Like:

image

@HubKing
Copy link

HubKing commented Apr 14, 2023

I can't assure it works on every single Linux machine due to fragmentation, but this fixed it for me:

-  file_browser_linux_cmd = "dbus-send --print-reply --dest=org.freedesktop.FileManager1 /org/freedesktop/FileManager1 org.freedesktop.FileManager1.ShowItems array:string:\"file:$path\" string:\"\""
+  file_browser_linux_cmd = "dirname $path | xargs -0 xdg-open"
-  return path_root() .. table.concat(parts, path_sep())
+  local path = path_root() .. table.concat(parts, path_sep())
+  return path:gsub('"', '\\"'):gsub("'", "\\'"):gsub(" ", "\\ ")

I ditched dbus-send because this seems to be an open issue and the proposed solution is "don't use dbus-send".

I tested the solution in path with comma, single quote, double quote, space and bracket, all worked.

Damn, after creating my own solution, I realised that you had posted your solution. I am not sure which of "xdg-open" or "dbus-send" is the newer standard, but your way, does it select the current file? Doesn't it only open the parent directory without selecting the current file?

@pintassilgo
Copy link
Author

but your way, does it select the current file? Doesn't it only open the parent directory without selecting the current file?

It doesn't select the file, at least on my file manager (Dolphin). It only opens directory. If you know how to make it select the file, please let me know.

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

No branches or pull requests

2 participants