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

Zsh completion does not match enough extensions #2273

Closed
Hello71 opened this issue Sep 1, 2015 · 16 comments
Closed

Zsh completion does not match enough extensions #2273

Hello71 opened this issue Sep 1, 2015 · 16 comments

Comments

@Hello71
Copy link
Contributor

Hello71 commented Sep 1, 2015

right now it only matches asf|asx|avi|flac|flv|m1v|m2p|m2v|m4v|mjpg|mka|mkv|mov|mp3|mp4|mpe|mpeg|mpg|ogg|ogm|ogv|qt|rm|ts|vob|wav|webm|wma|wmv. more common audio extensions: 3gp|act|aiff|aac|amr|au|awb|m4a|mpc|oga|opus|ra|webm. more video extensions: rmvb|m4v.

not sure if all are actually supported by mpv, but at least youtube-dl saves files as m4a and webm.

@ghost
Copy link

ghost commented Sep 1, 2015

Generally mpv supports many more file extensions than that, but since file format detection usually works by content and not by extension, most extensions are not known.

Which probably means the zsh script shouldn't white-list file extensions and just accept everything.

@Hello71
Copy link
Contributor Author

Hello71 commented Sep 1, 2015

mmhmm. maybe it would be better to have some option for this? "all files, try file extensions, detect ff, check fe then ff"

I don't know how that would work in zsh though.

@haasn
Copy link
Member

haasn commented Sep 1, 2015

Strongly agree that it should complete on all file types. When in doubt, don't try to be smart.

I run into this issue often enough. For example, PNG/JPG files don't get detected even though I open them routinely.

@Hello71
Copy link
Contributor Author

Hello71 commented Sep 1, 2015

for file extensions, here is a list:

$ grep -Phor '(FF_DEF_RAW(SUB|VIDEO)_DEMUXER[^,]*,[^,]*,[^,]*, |\.extensions[^"]*|modplug_extensions[^"]*)"\K[^"]*' | sed 's/,$//;s/ //g;s/,/\n/g' | sort -u | tr '\n' '|'
264|265|302|3g2|3gp|669|722|A64|a64|aa|aa3|aac|abc|ac3|adf|adp|adts|adx|aea|afc|aif|aifc|aiff|amf|amr|ams|ans|ape|apl|apng|aqt|art|asc|asf|ass|ast|au|avc|avi|avr|avs|bcstm|bfstm|bin|bit|bmp|bmv|brstm|caf|cavs|cdata|cdg|cdxl|cgi|chk|cif|daud|dbm|dif|diz|dmf|dnxhd|dpx|drc|dsm|dss|dtk|dts|dtshd|dv|dvd|eac3|f4v|fap|far|ffm|ffmeta|flac|flm|flv|g722|g723_1|g729|gif|gsm|gxf|h261|h263|h264|h265|h26l|hevc|ice|ico|idf|idx|ircam|isma|ismv|it|itgz|itr|itz|ivf|jls|jpeg|jpg|js|jss|latm|lbc|ljpg|loas|lrc|lvf|m1v|m2a|m2t|m2ts|m2v|m3u8|m4a|m4v|mac|mdgz|mdl|mdr|mdz|med|mid|mj2|mjpeg|mjpg|mk3d|mka|mks|mkv|mlp|mmf|mod|mov|mp2|mp3|mp4|mpa|mpc|mpeg|mpg|mpl2|mpo|mt2|mtm|mts|mvi|mxf|mxg|nfo|nist|nut|oga|ogg|ogv|okt|oma|omg|opus|paf|pam|pbm|pcx|pgm|pgmyuv|pjs|png|psm|psp|ptm|pvf|qcif|ra|rco|rcv|rgb|rm|roq|rsd|rso|rt|s3gz|s3m|s3r|s3z|sami|sbg|sdr2|sf|shn|sln|smi|son|sox|spdif|sph|spx|srt|ssa|stl|stm|sub|sup|swf|tak|tco|thd|ts|tta|txt|ult|umx|v|vb|vc1|viv|vob|voc|vqe|vqf|vql|vt|vtt|w64|wav|webm|webp|wma|wmv|wtv|wv|xl|xm|xmgz|xml|xmr|xmz|y4m|yop|yuv|

When in doubt, don't try to be smart.

true, but probing all files actually requires more work.

here are options as I see them:

  1. use the current list
  2. add the extensions from the original description
  3. use all ffmpeg-supported extensions from above
  4. probe all files
  5. probe files that match option 3
  6. have options 2, 3, 4, 5 conditional on some variable (MPV_COMPLETION_TYPE?)

I am in favor of options 3 and 6. 5 has all the downsides of 3 (i.e. may not match some files) with only the small upside of discarding irrelevant files. 4 is too slow when operating in large directories. for context, on my CPU, ffprobe file.pdf takes 0.14 seconds and ffprobe file.{mp4,m4a,mkv} takes 0.04-0.08 seconds. this would mean that a directory of 100 unsupported files would take about 14 seconds whereas 100 supported files would take 4-8 seconds.

@haasn
Copy link
Member

haasn commented Sep 1, 2015

Wait, why is ffprobe being run when tab completing?

That makes no sense to me. Can't it just display a list of files normally like every other completion script in the world manages to do so?

@Hello71
Copy link
Contributor Author

Hello71 commented Sep 1, 2015

it's not, at least now anyways; the idea is that completers should filter arguments to only those that the command can accept. I guess I'll add 7, "kick the file completer and show everything".

@haasn
Copy link
Member

haasn commented Sep 2, 2015

Then I vote 7. When in doubt, don't try to be smart.

Especially if it causes bad behavior.

@ghost
Copy link

ghost commented Sep 2, 2015

+1

@Hello71
Copy link
Contributor Author

Hello71 commented Sep 2, 2015

@ghedo, @qmega?

@qmega
Copy link
Contributor

qmega commented Sep 2, 2015

Sorry, saw this yesterday but haven't had time.

Just to be clear, what currently happens is matching files with a simple glob based on file extension, with a hardcoded list (shown in the OP) that probably came from an old mplayer completion script.

Running ffprobe on every file for tab completion would be ridiculous, IMO. Tab completion isn't going to save you a whole lot of time if it takes several seconds to run. And imagine if you were on a network filesystem. I think file extension is the only thing we're going to be able to work with here.

So the only thing to really decide here is whether or not it's worth trying to filter by extension at all. I'm not entirely sure how I feel about that, actually. I remember being mildly annoyed when I first started using the completion script and wishing it would just give me all files (as has been expressed in this thread already), but now that I've gotten used to it, it is kind of nice sometimes, to to weed out extra files if you're in a directory with a bunch of different stuff in it. Note that (at least in my zsh config), it will fall back to all files if nothing matches, so it will still complete files that don't have matching extensions if you type enough to eliminate any files that do (or are in a directory where there aren't any).

If we are going to keep the extension-based filtering, putting that huge list in is definitely not going to work. One issue is that it probably depends on how ffmpeg was compiled, but even ignoring that, a lot of those don't actually make any sense. For example, I'm sure there's some media file out there that ends with .js, but realistically, a random .js file is probably javascript and better left out of the completion. Another example is subtitle files like .ass/.srt/.lrc, which can't be played by themselves. In fact, files like that may have the same name as a video file they go along with, and actually get in the way of completion for that file. One thing that might actually be cool would be if there is a video file and an audio file with the same name, only complete the video one, but I'm not sure if that'd be worth the effort to implement.

Even with a manually maintained list of extensions, there isn't necessarily going to be a list that is perfect for everyone. For example, haasn wants PNG/JPG, but I don't use mpv on those very often, so I'd rather they be excluded if I'm in a directory that has those along with some video or audio files.

If a given user cares enough, they can override the pattern using zstyle. For example, zstyle ':completion:*:complete:mpv:*' file-patterns '*.(#i)(svg|svgz):example %p:globbed-files *(-/):directories' '*:all-files' will complete svg files along with the ones included in the completion script, and zstyle ':completion:*:complete:mpv:*' file-patterns '*.(#i)(svg|svgz):example *(-/):directories' '*:all-files' is the same thing but ignores the patterns built into the script. So the option to customize the list of extensions is already there, it's built into zsh. The choices we have, as I see it, are:

  • Keep a conservative list of (default) extensions. We could add the common ones from the OP, but not just everything ffmpeg supports. Saves keystrokes in the common case, but adds annoyance when trying to play a file with no extension or an uncommon one.
  • Keep a large list of (default) extensions. I don't really like this one because it's a pain to maintain and will probably end up with false positives. And it still doesn't help with no-extension files.
  • Don't filter by extension at all. Users who want to can do it themselves with a zstyle.

In any case, customization is possible if you really care. However, the zstyle stuff is kind of weird and non-obvious, so you do have to really care; the majority of users likely won't even know they can change it, much less actually do so. (I didn't even know you could do that until I just looked it up.) So what it comes down to is, by default, do we want to provide a smoother common-case experience at the expense of possible annoyance/confusion, or be safe and show everything, but make people type more?

@Argon-
Copy link
Member

Argon- commented Sep 2, 2015

Note that (at least in my zsh config), it will fall back to all files if nothing matches, so it will still complete files that don't have matching extensions

Could it be possible to build this behavior right into the completion script?

@qmega
Copy link
Contributor

qmega commented Sep 2, 2015

Probably, but it seems unnecessary when zsh already does it by default and allows the user to customize it. It's actually the same file-patterns that controls it.

On Sep 2, 2015, at 13:51, Argon- [email protected] wrote:

Note that (at least in my zsh config), it will fall back to all files if nothing matches, so it will still complete files that don't have matching extensions

Could it be possible to build this behavior right into the completion script?


Reply to this email directly or view it on GitHub.

@otommod
Copy link
Contributor

otommod commented Sep 2, 2015

Would it be possible to put the ones with the known extensions first in the lest, and any other remaining file after that? That way you can tap Tab once or twice in the common case, but still be able to complete those lesser used files.

Do you thing something like that would be possible/useful?

@qmega
Copy link
Contributor

qmega commented Sep 4, 2015

Would it be possible to put the ones with the known extensions first in the lest, and any other remaining file after that? That way you can tap Tab once or twice in the common case, but still be able to complete those lesser used files.

That should be possible, at least if the user's configuration agrees. The list-dirs-first style does something similar, but only if the user has something like zstyle ':completion:*' group-name '' to display differently tagged completions separately.

But having two separate lists will break alphabetical order, which could be confusing, and then there's also the question of where in the order to put directories. Also, it doesn't have all the advantages of normal filtering. It might help in the case that you're tabbing through a list, but it doesn't reduce the amount you have to type if you want to narrow it down that way. The advantage of filtering based on extension is not just a smaller list, but (perhaps more importantly) that if a matching and non-matching file share some prefix, you can complete the matching file right away by typing that prefix, instead of having to disambiguate. Basically, I don't think there's any way we're going to get around making a trade-off here.

So I'd rather not do something like that. I understand the want for a compromise, but I think it's giving up a lot of the benefit of extension filtering and adding potential confusion, in exchange for better results in what I think would be a pretty rare case.

@j605
Copy link
Contributor

j605 commented May 28, 2016

As OP has pointed out youtube-dl downloads m4a and webm. And aac is also popular enough to add it to the default list.

@qmega
Copy link
Contributor

qmega commented Sep 5, 2016

#3459 reminded me of this. I'm now leaning toward getting rid of the extensions entirely as well. I realized that the current behaviour still surprises me sometimes; even though I immediately know what happened, it makes me stop typing and think. I'll probably still try to customize it for my own config, but we can't come up with any such config that suits everyone, and it's probably better to have people who want customization do it themselves, rather than make everyone else "customize" things to remove behaviour that doesn't do what they expect. I can put some examples on the wiki for people who do want to customize. You can also obviate much of the need for this kind of filtering if you use a clever matcher-list that allows you to easily complete against extensions.

I think I'll PR that next weekend if there isn't a ton of complaining in the meantime.

Calling @wgmk here because he/she might have an opinion.

@qmega qmega mentioned this issue Sep 10, 2016
@ghost ghost closed this as completed in eec7660 Sep 10, 2016
This issue was closed.
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

6 participants