Re: Review of new man page of sensible-editor
Justin B Rye <justin.byam.rye@gmail.com> writes:
> RL wrote:
>> #!/bin/sh
> [...]
>
> This looks like a great improvement on what we've got, and also better
> than the quarter-finished version I had, but unfortunately it looks
> like an improvement on the version in buster. The sid version has had
> some refactoring - though it doesn't fix the bug we'd noticed - so it
> would make sense to start from there.
Oh good point - i use stable. Updated version below - using 0.0.17
The sid version also has a slightly different logic to bullseye: it
tries VISUAL (from the environment) but then it reads ~/.selected_editor
and then tries EDITOR, SELECTED_EDITOR - so if you did 'export
EDITOR=vi' but ~/.selected_editor said EDITOR=emacs you'd get emacs. I
assume that is another bug and that the environment should take
precedence over the file.
My previous version also had too much quoting in places so that passing
arguments inside EDITOR would not have worked. This one should be
better. (tested with "EDITOR='ls -l' and it seems to work)
And going back to the man-page: sensible-editor doesn't use the variable
SENSIBLE_EDITOR at all, so that bit (3rd bullet) can go.
#!/bin/sh
# Copyright 2007 Jari Aalto; Released under GNU GPL v2 or any later version
# Copyright 201
# Prevent recursive loops, where environment variables are set to this script
p="$(command -v "$0")"
PreventLoops()
{
[ -n "$VISUAL" ] && [ "$(command -v "$VISUAL" || true)" = "$p" ] && VISUAL=
[ -n "$EDITOR" ] && [ "$(command -v "$EDITOR" || true)" = "$p" ] && EDITOR=
[ -n "$SELECTED_EDITOR" ] && [ "$(command -v "$SELECTED_EDITOR" || true)" = "$p" ] && SELECTED_EDITOR=
}
UnableToRun()
{
# 'Command not found' or 'Permission denied'
[ "$1" -ne 126 ] && [ "$1" -ne 127 ]
}
Try()
{
"$@"
ret=$?
if UnableToRun "$ret"; then
exit "$ret"
fi
}
# work around for #991982
nano ()
{
if [ -z "$TERM" ]; then
return 126
else
command nano "$@"
fi
}
PreventLoops
for candidate in "$VISUAL" "$EDITOR"; do
if [ -n "$candidate" ]; then
# $candidate must be unquoted on next line as it may contain arguments
Try $candidate "$@"
fi
done
# fix #987675
if [ -n "$HOME" ]; then
if [ -r ~/.selected_editor ]; then
. ~/.selected_editor 2>/dev/null
elif [ -z "$EDITOR" ] && [ -z "$SELECTED_EDITOR" ] && [ -t 0 ]; then
select-editor && . ~/.selected_editor 2>/dev/null
fi
fi
PreventLoops
for candidate in "$EDITOR" "$SELECTED_EDITOR" editor nano nano-tiny vi; do
if [ -n "$candidate" ]; then
Try $candidate "$@"
fi
done
echo "Couldn't find an editor!" 1>&2
echo 'Set the $EDITOR environment variable to your desired editor.' 1>&2
exit 1
(i dont know if #991982 also applies to nano-tiny as well as nano?)
diff:
--- sensible-editor.sid 2022-08-20 18:52:05.079219739 +0100
+++ sensible-editor.new 2022-08-20 19:02:01.954669461 +0100
@@ -1,57 +1,65 @@
#!/bin/sh
# Copyright 2007 Jari Aalto; Released under GNU GPL v2 or any later version
# Copyright 201
-ret="$?"
-# Prevent recursive loops, where these values are set to this script
+
+# Prevent recursive loops, where environment variables are set to this script
p="$(command -v "$0")"
-[ -n "$EDITOR" ] && [ "$(command -v "$EDITOR" || true)" = "$p" ] && EDITOR=
-[ -n "$VISUAL" ] && [ "$(command -v "$VISUAL" || true)" = "$p" ] && VISUAL=
-[ -n "$SELECTED_EDITOR" ] && [ "$(command -v "$SELECTED_EDITOR" || true)" = "$p" ] && SELECTED_EDITOR=
+PreventLoops()
+{
+ [ -n "$VISUAL" ] && [ "$(command -v "$VISUAL" || true)" = "$p" ] && VISUAL=
+ [ -n "$EDITOR" ] && [ "$(command -v "$EDITOR" || true)" = "$p" ] && EDITOR=
+ [ -n "$SELECTED_EDITOR" ] && [ "$(command -v "$SELECTED_EDITOR" || true)" = "$p" ] && SELECTED_EDITOR=
+}
-IsError()
+UnableToRun()
{
- # Operating system command not found
- [ "$1" -ne 126 ] && [ $1 -ne 127 ]
+ # 'Command not found' or 'Permission denied'
+ [ "$1" -ne 126 ] && [ "$1" -ne 127 ]
}
-Run()
+Try()
{
- "$@"
- ret=$?
- IsError "$ret"
+ "$@"
+ ret=$?
+ if UnableToRun "$ret"; then
+ exit "$ret"
+ fi
}
-# workarround #991982
+# work around for #991982
nano ()
{
if [ -z "$TERM" ]; then
- return 126
+ return 126
else
- command nano "$@"
+ command nano "$@"
fi
}
-if [ -n "$VISUAL" ]; then
- Run "$VISUAL" "$@" && exit "$ret"
-fi
+PreventLoops
+for candidate in "$VISUAL" "$EDITOR"; do
+ if [ -n "$candidate" ]; then
+ # $candidate must be unquoted on next line as it may contain arguments
+ Try $candidate "$@"
+ fi
+done
# fix #987675
if [ -n "$HOME" ]; then
if [ -r ~/.selected_editor ]; then
- . ~/.selected_editor 2>/dev/null || true
+ . ~/.selected_editor 2>/dev/null
elif [ -z "$EDITOR" ] && [ -z "$SELECTED_EDITOR" ] && [ -t 0 ]; then
- select-editor && . ~/.selected_editor 2>/dev/null || true
+ select-editor && . ~/.selected_editor 2>/dev/null
fi
fi
-
-Run ${EDITOR:-${SELECTED_EDITOR:-editor}} "$@" ||
-Run nano "$@" ||
-Run nano-tiny "$@" ||
-Run vi "$@" ||
-{
- echo "Couldn't find an editor!" 1>&2
- echo "Set the \$EDITOR environment variable to your desired editor." 1>&2
- exit 1;
-}
-exit "$ret"
+PreventLoops
+for candidate in "$EDITOR" "$SELECTED_EDITOR" editor nano nano-tiny vi; do
+ if [ -n "$candidate" ]; then
+ Try $candidate "$@"
+ fi
+done
+
+echo "Couldn't find an editor!" 1>&2
+echo 'Set the $EDITOR environment variable to your desired editor.' 1>&2
+exit 1
Reply to: