xenodium / ready-player

GNU General Public License v3.0
129 stars 5 forks source link

Allow video files to be handled also by file handler. #4

Closed zellerin closed 2 months ago

zellerin commented 2 months ago

This prevents (possibly huge) video file from being actually read as whole by emacs.

xenodium commented 2 months ago

Huge thanks for this! Lemme go play with it...

xenodium commented 2 months ago

I think we can reduce to something like:

(defun ready-player-handler (operation &rest args)
  "Prevent `insert-file-contents' OPERATION with ARGS."
  (pcase operation
    ('insert-file-contents
     (cl-destructuring-bind (filename visit _beg _end _replace) args
       (when visit
         (setq buffer-file-name filename))
       (list buffer-file-name (point-max))))
    (_ (let ((inhibit-file-name-handlers
              (cons 'ready-player-handler
                    (and (eq inhibit-file-name-operation operation)
                         inhibit-file-name-handlers)))
             (inhibit-file-name-operation operation))
         (apply operation args)))))

Does this work for you too?

ps. If thumbnail and metadata loading are disabled, opening the buffer is instantaneous. I'll look into loading those async.

xenodium commented 2 months ago

I've pushed the change in https://github.com/xenodium/ready-player/commit/c43a86a4b50e6cd478027cdef51308dda5643de5

Suppress loading media (faster buffer opening on big files)

Credit goes to Tomas Zellerin @zellerin https://github.com/xenodium/ready-player/pull/4 for this contribution.

Huge thanks!
zellerin commented 2 months ago

This works if you have updated both alists, right? Maybe that is better way how to do it. Or maybe the (insert ...) part of the ready-player-mode should go to insert-file-content. I do not think it really matters, but I did not see proper documentation on this anyway.

This version still presents the "... is too big, open anyway?" message.

zellerin commented 2 months ago

Also, minor style issue - now you do same thing (add relevant item/s to the alist) in two different ways. I am to blame, I used regexp-opt and did not realize you use dolist.

xenodium commented 2 months ago

Thanks for this!

This works if you have updated both alists, right?

I may be missing one bit here... If I'm understanding correctly, you mean updating both auto-mode-alist and file-name-handler-alist? If so, both are updated at:

https://github.com/xenodium/ready-player/blob/c43a86a4b50e6cd478027cdef51308dda5643de5/ready-player.el#L126C1-L140C39

I intend to rename that function, but don't want to break users. I'll follow up with a migration strategy (prolly a warning to switch to a new function).

Maybe that is better way how to do it. Or maybe the (insert ...) part of the ready-player-mode should go to insert-file-content. I do not think it really matters, but I did not see proper documentation on this anyway.

I'm reworking display a little and making thumbnail and metadata loading async. May simplify a little to keep it out.

This version still presents the "... is too big, open anyway?" message.

You're right. Following up with that addition, but first need to fix the metadata display as it currently always shows 0 size.

edit: typos

xenodium commented 2 months ago

Also, minor style issue - now you do same thing (add relevant item/s to the alist) in two different ways. I am to blame, I used regexp-opt and did not realize you use dolist.

Yup. Good point. Will make consistent also.

zellerin commented 2 months ago

as it currently always shows 0 size

This is what binding of

(file-name-handler-alist nil)

should have fixed in the second commit

xenodium commented 2 months ago

This is what binding of (file-name-handler-alist nil) should have fixed in the second commit

Ah, my bad. It wasn't obvious. I changed it to use the size coming from metadata, though I can see pros and cons to both. Your suggestion has the advantage of guaranteed size display for all files. The alternative is just consistent with other metadata (but not guaranteed).

xenodium commented 2 months ago

I've introduced async thumbnail/metadata loading in the latest. Along with your changes, it should make things snappier.

zellerin commented 2 months ago

I've introduced async thumbnail/metadata loading in the latest. Along with your changes, it should make things snappier.

This is great. However, it lost the ffmpeg-based thumbnail ability - intentionally?

Anyway, I guess I can close this

xenodium commented 2 months ago

I'll look into re-adding ffmpeg-based. The way I had introduced it wasn't great. For now, maybe try the ffmpegthumbnailer alternative?

xenodium commented 2 months ago

Ok. ffmpeg thumbnailer is back in https://github.com/xenodium/ready-player/commit/17b0e7219584f281a942f8eeac879b225098108e Sorry for the troubles.

zellerin commented 2 months ago

No troubles caused :)