YaLTeR / wl-clipboard-rs

A safe Rust crate for working with the Wayland clipboard.
Apache License 2.0
223 stars 16 forks source link

Add xdg_mime feature for using xdg_mime for mime type autodetection #13

Open mcoffin opened 3 years ago

mcoffin commented 3 years ago

This adds a new feature, xdg_mime, that will force the use of xdg-mime for querying autodetected mime types instead of tree_magic.

Justification

If you use wl-copy to copy a shell script, tree_magic reports its mime type as application/x-shellscript, which is not a text mime type, and therefore you cannot paste that script in to text boxes in say, Firefox, for example.

For users that want the same functionality as the upstream wl-clipboard utilities, which uses xdg-mime to autodetect mime types, they may compile with --features xdg_mime to force the use of xdg-mime instead of tree_magic.

While sticking with the pure-Rust implementation would be preferable, providing this as optional functionality can make these binaries usable for those who need that functionality to work as it would with the other tools.

mcoffin commented 3 years ago

The second push is just a removal of a debug statement that I had missed when I submitted :)

YaLTeR commented 3 years ago

Hmm, if xdg-mime gives better results compared to tree_magic then perhaps it's a better idea to just use xdg-mime and drop tree_magic? Are there any major downsides?

bugaevc commented 3 years ago

<...> mime type as application/x-shellscript, which is not a text mime type, and therefore you cannot paste that script in to text boxes in say, Firefox, for example. For users that want the same functionality as the upstream wl-clipboard utilities <...>

FYI, upstream wl-clipboard also recognizes MIME types that end with script to be text types and offers them as plain text as well as the original type.

Perhaps wl-clipboard-rs could learn a few similar tricks :smile:

YaLTeR commented 3 years ago

Also for me xdg-mime also reports application/x-shellscript:

$ xdg-mime query filetype ~/source/sh/avs.sh 
application/x-shellscript
mcoffin commented 3 years ago

Also for me xdg-mime also reports application/x-shellscript:

$ xdg-mime query filetype ~/source/sh/avs.sh 
application/x-shellscript

@YaLTeR if you run it on the tmp file created by wl-copy, though, it will report text/x-shellscript (due to is missing the .[zcf]?sh extension, so it interprets the shebang line at the top of the file), and correctly assign the text mime types. As for getting better results than tree_magic, I would also say that making this feature a "default" feature would be a good idea, but I didn't do it by default just to be minimally intrusive with the MR. Just let me know if that's desired and I'll add it as a default, or invert the logic to have the feature be use_tree_magic.

mcoffin commented 3 years ago

At the end of the day, I wouldn't say that tree_magic's results are globally worse, but they do cause some issues with pasting if you pipe in a shell script.

An alternative solution would be to just detect when tree_magic reports application/x-shellscript and convert it to text/x-shellscript + application/x-shellscript.

YaLTeR commented 3 years ago

Are there any other issues with tree_magic that you've noticed? Maybe instead of using xdg-mime it's sufficient to just add additional text MIME type detection from wl-clipboard?

mcoffin commented 3 years ago

Are there any other issues with tree_magic that you've noticed? Maybe instead of using xdg-mime it's sufficient to just add additional text MIME type detection from wl-clipboard?

That sounds like a good solution, keep the default as tree_magic, keep a feature to force xdg-mime, but add the same detection logic that they have in wl-clipboard upstream for both cases.

If you're good with that I'll update this PR maybe... tonight when I have some time.

YaLTeR commented 3 years ago

@YaLTeR if you run it on the tmp file created by wl-copy, though, it will report text/x-shellscript (due to is missing the .[zcf]?sh extension, so it interprets the shebang line at the top of the file)

I still can't get anything different from application/x-shellscript.

┌ ~
└─ wl-copy < source/sh/avs.sh 
┌ ~
└─ xdg-mime query filetype /tmp/wl-copy-buffer-p7ELVZ/avs.sh 
application/x-shellscript
┌ ~
└─ cp source/sh/avs.sh tmp
┌ ~
└─ xdg-mime query filetype tmp
application/x-shellscript

Are you sure xdg-mime is actually any different from tree_magic and it's not just those detection logic from wl-clipboard?

mcoffin commented 3 years ago

Are you sure xdg-mime is actually any different from tree_magic and it's not just those detection logic from wl-clipboard?

@YaLTeR I'm positive. If you run wl-copy < some-script.sh and the script starts with #!/bin/bash or similar, then run xdg-mime /tmp/path/to/TEMPFILE_GENERATED_BY_WL_COPY, then it should give text/x-shellscript.

As a second check, my version of xdg-utils (which contains xdg-mime on arch), is 1.1.3+19+g9816ebb-1, which looks like it might be a git offset from a stable version, which is kinda odd considering I'm on the stable upstream branch of that package.

AnyTimeTraveler commented 3 months ago

I don't like that it calls out to a binary that may or may not be installed on the user's system, especially since there is no fallback to tree_magic upon an error, or any kind of error handling apart from panicking.

Especially, since there is a rust library implementation of xdg-mime: https://github.com/ebassi/xdg-mime-rs