Re: Review of new man page of sensible-editor

Justin B Rye <[email protected]> writes:

> The big problem with the itemised list of stages is that although it
> makes a good summary how sensible-editor's logic *should* work, when
> you look at the script it turns out not to do what it says it does.
> Much of the work is done by the line
>
>  ${EDITOR:-${SELECTED_EDITOR:-editor}} "$@"
>           
> ...which will try EDITOR if that's non-null, or SELECTED_EDITOR if
> that's non-null, or /usr/bin/editor as a fallback.  But this means if
> my shell profile has
>
>  export EDITOR=nosuchprogram
>
> then sensible-editor will try to execute that, receive an errorcode
> 127, and immediately fail over to running *nano* instead, regardless
> of what the alternatives system says should be the system default!
>
> So do we document this, or fix it?

Seems like a fix would give a small improvement and should not harm
anyone. Below is a new version of sensible-editor that does that - and
removes some duplication of code:

#!/bin/sh

# Prevent recursive loops, where a variable is set to this script
p="$(which sensible-editor)"

for candidate in "${VISUAL-}" "${EDITOR-}" "${SELECTED_EDITOR-}"; do
  if [ -n "$candidate" ] && [ "$(which "$candidate" || true)" != "$p" ]; then
    "$candidate" "$@"
	  ret="$?"
	  if [ "$ret" != 126 ] && [ "$ret" != 127 ]; then
		  exit "$ret"
	  fi
  fi
done

if [ -r ~/.selected_editor ]; then
	. ~/.selected_editor 2>/dev/null || true
elif [ -z "${SELECTED_EDITOR-}" ] && [ -t 0 ]; then
	select-editor && . ~/.selected_editor 2>/dev/null || true
fi

for candidate in "${SELECTED_EDITOR-}" editor nano nano-tiny vi; do
  if [ -n "$candidate" ] && [ "$(which "$candidate" || true)" != "$p" ]; then
    "$candidate" "$@"
    ret="$?"
    if [ "$ret" != 126 ] && [ "$ret" != 127 ]; then
      exit "$ret"
	  fi
  fi
done

echo "Couldn't find an editor!" 1>&2
echo "Set the \$EDITOR environment variable to your desired editor." 1>&2
exit 1