[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review of new man page of sensible-editor



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.

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

(In particular, bullseye will need "command -v" instead of "which".)

> 
> for candidate in "${VISUAL-}" "${EDITOR-}" "${SELECTED_EDITOR-}"; do
>   if [ -n "$candidate" ] && [ "$(which "$candidate" || true)" != "$p" ]; then

(It isn't running under set -eu, so no need for ${...-} or ||true
here.  Likewise in the foo&&bar||true later.)

>     "$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

(I'd use '' quotes round this one and save a backslash.)

> exit 1

-- 
JBR	with qualifications in linguistics, experience as a Debian
	sysadmin, and probably no clue about this particular package


Reply to: