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

Fix previews (video, pdf) and support uppercase file extensions #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jnk22
Copy link

@jnk22 jnk22 commented Mar 31, 2023

Hi, I have just tried to setup this plugin in NeoVim.

First of all, thank you for keeping this plugin alive @Conni2461 and everyone else involved!

It seems there are a few problems with file previews in its current version (possibly partly related to the breaking change #35):

  1. Video preview does not work when viu is not installed
  2. PDF preview does not work due to preview script not finding the generated preview files
  3. Files with non-lowercase extensions are not supported

This PR is supposed to fix those things:

  1. As viu does not seem to be used for any previews (only chafa is), I have replaced the check for its existence with a check for ffmpegthumbailer which seems to be originally intended for that line.
  2. pdftoppm already appends the file extension .png when using the command option -png (This is at least true for my installed version, 23.03.0 - this might need double checking for earlier versions). Therefore I have removed the additional .png for the output file.
  3. When previewing image files with file extensions like .JPG or .MP4, I get the error message: unknown file ..image_path.JPG. I have added a line to make the extracted extension lowercase to match the following case patterns.

@HendrikPetertje Could you verify if these changes still work for you?

Copy link

@Chiarandini Chiarandini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got the same problem, this fixes it -- oh shoot, I just meant to comment

Copy link

@sudo-burger sudo-burger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine here (MacOS + brew (newer bash than what comes with MacOS) + neovim v0.9.5).
The substitution "${extension,,}" doesn't work with the stock MacOS bash (v3.2.57(1) at the moment).

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

Successfully merging this pull request may close these issues.

3 participants