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

function for editing command line in ext editor #3627

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc_src/index.hdr.in
Expand Up @@ -1001,6 +1001,10 @@ Some bindings are shared between emacs- and vi-mode because they aren't text edi

- @key{Alt,w} prints a short description of the command under the cursor.

- @key{Alt,e} edit the current command line in an external editor. The editor is chosen from the first available of the `$VISUAL` or `$EDITOR` variables.

- @key{Alt,v} Same as @key{Alt,e}.

\subsection emacs-mode Emacs mode commands

- @key{Home} or @key{Control,A} moves the cursor to the beginning of the line.
Expand Down
4 changes: 4 additions & 0 deletions share/functions/__fish_shared_key_bindings.fish
Expand Up @@ -96,4 +96,8 @@ function __fish_shared_key_bindings -d "Bindings shared between emacs and vi mod
# Make it easy to turn an unexecuted command into a comment in the shell history. Also,
# remove the commenting chars so the command can be further edited then executed.
bind $argv \e\# __fish_toggle_comment_commandline

# The [meta-e] and [meta-v] keystrokes invoke an external editor on the command buffer.
bind \ee edit_command_buffer
bind \ev edit_command_buffer
end
44 changes: 44 additions & 0 deletions share/functions/edit_command_buffer.fish
@@ -0,0 +1,44 @@
function edit_command_buffer --description 'Edit the command buffer in an external editor'
set -l f (mktemp)
if set -q f[1]
mv $f $f.fish
Copy link
Member

Choose a reason for hiding this comment

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

I think you know that this is racy. Though it seems a rather theoretical criticism since this function won't be executed in a tight loop by multiple shells at the same time.

Any fix to this would require either an alternative to mktemp or a mktemp that accepts a suffix, which is unlikely to be available on BSD or macOS.

Copy link
Member

Choose a reason for hiding this comment

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

In psub, we use mktemp -d and then add a file with a suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if this were executed in a tight loop it is so improbable there would be collision that it can be safely ignored. It's comparable to getting two commits in the fish project with the same leading eight digits of the commit hash.

set f $f.fish
else
# We should never execute this block but better to be paranoid.
if set -q TMPDIR
set f $TMPDIR/fish.(echo %self).fish
else
set f /tmp/fish.(echo %self).fish
end
touch $f
or return 1
end

# Edit the command line with the users preferred editor or vim or emacs.
commandline -b >$f
if set -q VISUAL
eval $VISUAL $f
else if set -q EDITOR
eval $EDITOR $f
else
echo
echo (_ 'External editor requested but $VISUAL or $EDITOR not set.')
echo (_ 'Please set VISUAL or EDITOR to your preferred editor.')
commandline -f repaint
command rm $f
return 1
end

# Here we're checking the exit status of the editor.
if test $status -eq 0 -a -s $f
Copy link
Member

Choose a reason for hiding this comment

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

Maybe comment that this "$status" is the status of the editor, so we need to be careful not to break that pattern here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

# Set the command to the output of the edited command and move the cursor to the
# end of the edited command.
commandline -r (cat $f)
commandline -C 999999
Copy link
Member

Choose a reason for hiding this comment

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

We should really have a nicer way to set the cursor to the end, and commandline should probably also accept stdin.

Copy link
Member

Choose a reason for hiding this comment

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

We should really have a nicer way to set the cursor to the end,

-1 perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flaom, Perhaps you should open a new issue, and take ownership of it, rather than commenting on a closed PR where your proposal has zero chance of being implemented?

else
echo
echo (_ "Ignoring the output of your editor since its exit status was non-zero")
echo (_ "or the file was empty")
end
command rm $f
end