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: file preview for tsx, js, ts, html, lua #2904

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Zeioth
Copy link

@Zeioth Zeioth commented Sep 29, 2023

The fix is very simple: Instead of trusting the file mimetype, which file very often fail to determine accurately, we pygmentize the file based on its file extension.

screenshot_2023-09-30_00-14-11_998031005

From: https://www.reddit.com/r/ranger/comments/16s46s4 and many other similar threads trying to achieve this.

@Zeioth
Copy link
Author

Zeioth commented Sep 29, 2023

The reason for also adding lua, is file fails to determine its mimetype when mixed with vimscript variables, as in neovim .lua config files.

@Zeioth Zeioth changed the title fix: preview for tsx, html, lua fix: preview for tsx, js, ts, html, lua Sep 29, 2023
@Zeioth
Copy link
Author

Zeioth commented Sep 29, 2023

js and ts added to the fix. I've tested 22+ different programming languages and the preview seem to work fine on all of them now.

@Zeioth Zeioth changed the title fix: preview for tsx, js, ts, html, lua fix: preview for tsx, js, ts, html, lua Sep 29, 2023
@Zeioth Zeioth changed the title fix: preview for tsx, js, ts, html, lua fix: file preview for tsx, js, ts, html, lua Sep 29, 2023
Copy link

@klevvit klevvit left a comment

Choose a reason for hiding this comment

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

I really like the idea of better rendering, but I consider we shouldn't remove or change significantly the options some users may actively use. Additional modifications, e.g. replacement of handle_fallback() with cat or switch to html as text not as page may be useful for some users, but it has huge influence on ranger functionality, so it's better to make it as an easy-to-switch option

ranger/data/scope.sh Show resolved Hide resolved
ranger/data/scope.sh Show resolved Hide resolved
@Zeioth
Copy link
Author

Zeioth commented Oct 26, 2023

I really like the idea of better rendering, but I consider we shouldn't remove or change significantly the options some users may actively use. Additional modifications, e.g. replacement of handle_fallback() with cat or switch to html as text not as page may be useful for some users, but it has huge influence on ranger functionality, so it's better to make it as an easy-to-switch option

The previous handle_fallback() provided error control, but it didn't provide sane defaults. There is no reason to ask the user to implement basic functionality when we can provide it (which this PR does).

If anything, I would add code comments to aid the users interested on re-implementing stuff.

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.

None yet

2 participants