New function __fish_preview_current_file to open current file in pager#6855
New function __fish_preview_current_file to open current file in pager#6855exploide wants to merge 1 commit into
Conversation
krobelus
left a comment
There was a problem hiding this comment.
This is really useful! Let's add a changelog entry and docs under Shared bindings in index.rst, so users will notice.
|
I incorporated most of your suggestions. Please have a second look. However, I encountered another problem, so I would not merge this now: This use of Some existing functions or completions might have the same problem. Can this easily be solved by using The problem with |
|
I think your solution works fine for most use cases, but it is surprisingly tricky to fix the issues.
Yeah I just realized that.
Yeah, the third eval evaluates the result of a former evaluation. We The second eval suffers from the double-tokenization issue; that's When the cursor is on something that does not tokenize (for example I got rid of some variable quoting, instead I explicitly check function __fish_preview_current_file --description "Open the file at the cursor in a pager"
set -l pager less --
set -q PAGER && echo $PAGER | read -at pager
# commandline -t will never return an empty list. However, the token
# could comprise multiple lines, so join them into a single string.
set -l file (commandline -t | string collect)
# count $file is 1
# $backslash will parsed as regex which may need additional escaping.
set -l backslash '\\\\'
not status test-feature regex-easyesc && set backslash $backslash$backslash
test -z $file && set file (string replace -ra -- '([ ;#^<>&|()"\'])' "$backslash\$1" (commandline -oc)[-1])
# count $file is 0 or 1
set -q file[1] || return
# Now file contains exactly one element, the unexpanded token.
# strip -option= from token if necessary
set file (string replace -r -- '^-[^\s=]*=(.*)' '$1' $file)
true # Clear $status from string replace, we don't want to return because of it.
eval set -l files $file || return # Return when eval fails.
if set -q files[1] && test -f $files[1]
$pager $files
commandline -f repaint
end
end |
|
Thanks for continuing this. I will test that tomorrow and come back. And thanks for linking the issue related to an expand builtin. I think that would be the way to go. Good that @faho already has something like that. The problem with evaluating harmful file names is already present for example in the completions of |
|
Seems to work so far, thanks. I pushed that version now. It's ready from my side. At least until we have a safe alternative to eval. When happens the following? When a filename contains line breaks?
|
Exactly, you can create a file like Merged to master as 8025e80, thank you! |
Description
New function
__fish_preview_current_fileto open current file in pager. Bound to Alt+O by shared key bindings.Fixes issue #6838
TODOs: