Editing system files in Linux (as root) with GUI and CLI text editors #3

Question

One year and a half ago I posted the second iteration of this script for a review here:

Editing system files in Linux (as root) with GUI and CLI text editors #2

---

Since then, it has been "hibernated" as I had way too much work, and I would like to ask you for a review of the possible final edit. I made a big effort for it to be final, but we all know there is always some space to improve. Thank you in advance!

As stated in there:

My intention is to POSIX-ly write one generalized function for running various text editors I use for different purposes through sudoedit, i.e. editing files as root safely. Safely = for instance, if a power loss occurs during the file edit; another example could be lost SSH connection, etc.

---

The script follows

#!/bin/sh
# Please, customize these lists to your preference before using this script!
cli_editors='vi nano'
gui_editors='subl xed'
# NON-COMPLIANT: code -w --user-data-dir; I did not find a way to run it through `sudoedit`;
# atom -w --no-sandbox; gedit -w does not work via rooted ssh; pluma does not have -w switch
# USAGE INSTRUCTIONS
# 1. Customize the editor lists at the beginning of this script.
#
# 2. Bash: Source this script in your ~/.bashrc or ~/.bash_aliases with:
# . /full/path/to/sudoedit-enhanced
# Other shells: Generally put it inside your shell's startup config file...
#
# 3. Call an alias, for instance, one CLI, and one GUI editor:
# sunano /path/to/file
# susubl /path/to/file
#
# Explanation: This script works with standard `sudoedit`, but
# it does a bit more checks and allows some GUI editors to be used.
# It needs to be sourced into your shell's environment first.
# Then simply use the pre-defined aliases or define some yourself.
sudoedit_err ()
{
 printf >&2 '%s\n' 'Error in sudoedit_run():'
 printf >&2 '%b\n' "$@"
 exit 1
}
sudoedit_run ()
{
 # print usage
 if [ "$#" -eq 2 ]; then
 printf >&2 '%s\n' 'Usage example: sunano /file1/path /file2/path'
 exit 1
 fi
 # sudo must be installed
 if ! command -v sudo > /dev/null 2>&1; then
 sudoedit_err "'sudo' is required by this function. This is because" \
 "'sudoedit' is part of 'sudo'\`s edit feature (sudo -e)"
 fi
 # the first argument must be an editor type
 case "1ドル" in
 ('cli'|'gui') ;;
 (*) sudoedit_err "'1ドル' is unrecognized editor type, expected 'cli' or 'gui'." ;;
 esac
 # store editor's type and name and move these two out of argument array
 editor_type=1ドル; editor_name=2ドル; shift 2
 # find and store path to this editor
 editor_path=$( command -v "$editor_name" 2> /dev/null )
 # check if such editor = executable path exists
 if ! [ -x "$editor_path" ]; then
 sudoedit_err "The '$editor_name' editor is not properly installed on this system."
 fi
 # `sudoedit` does not work with symlinks!
 # translating symlinks to normal file paths using `readlink` if available
 if command -v readlink > /dev/null 2>&1; then
 for file in "$@"; do
 if [ -L "$file" ]; then
 if ! file=$( readlink -f "$file" ); then
 sudoedit_err "readlink -f $file failed."
 fi
 fi
 set -- "$@" "$file"
 shift
 done
 fi
 if [ "$editor_type" = gui ]; then
 # 1. GUI editors will "sit" on the terminal until closed thanks to the wait option
 # 2. Various editors errors might flood the terminal, so we redirect all output,
 # into prepared temporary file in order to filter possible "sudoedit: file unchanged" lines.
 if tmpfile=$( mktemp /tmp/sudoedit_run.XXXXXXXXXX ); then
 SUDO_EDITOR="$editor_path -w" sudoedit -- "$@" > "$tmpfile" 2>&1
 grep 'sudoedit:' "$tmpfile"
 rm "$tmpfile"
 else
 sudoedit_err "mktemp /tmp/sudoedit_run.XXXXXXXXXX failed."
 fi
 else
 # 1. CLI editors do not cause problems mentioned above.
 # 2. This is a generic and proper way using `sudoedit`;
 # Running the editor with one-time SUDO_EDITOR setup.
 SUDO_EDITOR="$editor_path" sudoedit -- "$@"
 fi
}
sudoedit_sub ()
{
 ( sudoedit_run "$@" )
}
# Editor aliases generators:
# - avoid generating editors aliases for which editor is not installed
# - avoid generating editors aliases for which already have an alias
for cli_editor in $cli_editors; do
 if command -v "$cli_editor" > /dev/null 2>&1; then
 # shellcheck disable=SC2139,SC2086
 if [ -z "$( alias su$cli_editor 2> /dev/null)" ]; then
 alias su$cli_editor="sudoedit_sub cli $cli_editor"
 fi
 fi
done
for gui_editor in $gui_editors; do
 if command -v "$gui_editor" > /dev/null 2>&1; then
 # shellcheck disable=SC2139,SC2086
 if [ -z "$( alias su$gui_editor 2> /dev/null)" ]; then
 alias su$gui_editor="sudoedit_sub gui $gui_editor"
 fi
 fi
done
unset cli_editors gui_editors

Answer 1

Aside from your self-review, I noticed a few (very minor) things I thought may be worth pointing out

First, a design opinion rather than a comment on the code. I'm not sure putting the lists of editors in variables in the file is the best approach - it has its advantages, but allowing it to be managed separately in configuration files may have advantages. Were I a user of this script I would prefer to be told to go edit something like a $XDG_CONFIG_DIRS/sudoedit-enhanced file over being encouraged to edit the script itself

It feels a bit weird to have sudoedit_run print sunano as a usage example regardless of whether nano is desired, or available. Would it be at all realistic to offer up one of the aliases that were successfully defined?

While you can usually assume /tmp exists, the user may prefer to store their temp files elsewhere. Consider having the mktemp call instead use the --tmpdir flag to create a file relative to the $TMPDIR set by the user - not many users will care, but those who do will probably have very strong feelings about it

The alias generators act a bit weird on editors whose names contain spaces - which binary files can even if they rarely do. Right now, as you store each editor as a word in a space-separated string you really can't worry about what to do with them, but if you were to some day end up refactoring something in such a way that it becomes possible to do something funky like cli_editor="per se", consider how a line like su$cli_editor="sudoedit_run cli $cli_editor" would react to that (it's sometimes but not always an error, and there might be side effects). Quoting the alias name, to force a failure due to an invalid alias name (at least in bash, not sure if other shells permit spaces in alias names) might be preferable

sudoedit_run also has a subtle bug in how it checks for an editor - command -v will report the name of shell builtins in favor of commands. If someone were to do something silly like listing echo as an editor, command -v echo would return echo. This would lead to it working just fine as long as there's a file called echo in your current directory, but get reported as not properly installed otherwise. It's enough of a corner case that it's almost certainly not worth caring about, but still

Answer 2

1. The print error function is only useful to a point, I have rather removed it and used direct in-place constructs instead, so this function has been removed:

   (削除)

   sudoedit_err () { ... ; exit 1; }
   

   (削除ここまで)

   and the code had changed to this:

   printf >&2 '%s\n' '...'
   return 1
   

   So, it is now without the useless headline, and also we eliminated the exit keyword completely. In all of the code return is used now. Also there is now no need for '%b\n' in printf as there are no escape sequences like Tab or New line.

2. Thanks to the above change it was possible to remove the subshell function:

   (削除)

   sudoedit_sub ()
   {
    ( sudoedit_run "$@" )
   }
   

   (削除ここまで)

3. The print usage routine now returns 0 exit code, as it seems more logical to me if someone just hits the alias without any argument like with nano:

   sunano
   

4. New opening routine added to address the issue when someone would try to hit sudoedit_run with 0 or 1 argument:

   if [ "$#" -le 1 ]; then
    printf >&2 '%s\n' 'Implementation example: sudoedit_run cli nano /path/to/file'
    return 1
   fi
   

5. Finally, there was an unseen error in the GUI editor routine, where it would always return 0 exit code, no matter how sudoedit would end up, this is now corrected:

   if [ "$editor_type" = 'gui' ]; then
    # 1. GUI editors will "sit" on the terminal until closed thanks to the wait option
    # 2. Various editors errors might flood the terminal, so we redirect all output,
    # into prepared temporary file in order to filter possible "sudoedit: file unchanged" lines.
    if tmpfile=$( mktemp /tmp/sudoedit_run.XXXXXXXXXX ); then
    SUDO_EDITOR="$editor_path -w" sudoedit -- "$@" > "$tmpfile" 2>&1
    exit_code=$?
    grep 'sudoedit:' "$tmpfile"
    rm "$tmpfile"
    return "$exit_code"
    else
    printf >&2 '%s\n' "mktemp /tmp/sudoedit_run.XXXXXXXXXX failed."
    return 1
    fi
   else
    ... CLI routine ...
   fi
   

---

The refactored code follows:

#!/bin/sh
#+------------------------------------------------------------------------------+
#| Safe text files editing as `root` |
#| Language: POSIX shell script |
#| Copyright: 2020-2021 Vlastimil Burian |
#| M@il: info[..]vlastimilburian[..]cz |
#| License: GPL 3.0 |
#| Version: 2.1 |
#| GitHub: https://git.io/Jvnzq |
#+------------------------------------------------------------------------------+
# Customize these lists to your preference before using this script!
cli_editors='vi nano'
gui_editors='subl xed'
# NON-COMPLIANT-GUI:
# - code -w --user-data-dir; I did not find a way to run it through `sudoedit`
# - atom -w --no-sandbox
# - gedit -w does not work via rooted ssh
# - pluma does not have -w switch
# USAGE INSTRUCTIONS
#
# 1. Customize the editor lists at the beginning of this script.
#
# 2. Bash: Source this script in your ~/.bashrc or ~/.bash_aliases with:
# source /full/path/to/sudoedit-enhanced
#
# Other shells: Put it inside your shell's startup config file universally using dot:
# . /full/path/to/sudoedit-enhanced
#
# 3. Call an alias, for instance, one CLI, and one GUI editor:
#
# sunano /path/to/file
# susubl /path/to/file
#
# Explanation: This script works with standard `sudoedit`, but
# it does a bit more checks and allows some GUI editors to be used.
# It also allows symbolic links be translated into ordinary files.
# It needs to be sourced into your shell's environment first.
# Then simply use the pre-defined aliases or define some yourself.
sudoedit_run ()
{
 # print error if called with 0 or 1 argument
 if [ "$#" -le 1 ]; then
 printf >&2 '%s\n' 'Implementation example: sudoedit_run cli nano /path/to/file'
 return 1
 fi
 # print usage if called with 2 arguments
 if [ "$#" -eq 2 ]; then
 printf '%s\n' 'Usage example: sunano /path/to/file1 /path/to/file2'
 return 0
 fi
 # sudo must be installed
 if ! command -v sudo > /dev/null 2>&1; then
 printf >&2 '%s\n' "'sudo' is required by this function. This is because" \
 "'sudoedit' is part of 'sudo'\`s edit feature (sudo -e)."
 return 1
 fi
 # the first argument must be an editor type
 case "1ドル" in
 ('cli'|'gui')
 ;;
 (*)
 printf >&2 '%s\n' "'1ドル' is unrecognized editor type, expected 'cli' or 'gui'."
 return 1
 ;;
 esac
 # store editor's type and name and move these two out of argument array
 editor_type=1ドル; editor_name=2ドル; shift 2
 # find and store path to this editor
 editor_path=$( command -v "$editor_name" 2> /dev/null )
 # check if such editor = executable path exists
 if ! [ -x "$editor_path" ]; then
 printf >&2 '%s\n' "The '$editor_name' editor is not properly installed on this system."
 return 1
 fi
 # `sudoedit` does not work with symlinks!
 # translating symlinks to normal file paths using `readlink` if available
 if command -v readlink > /dev/null 2>&1; then
 for file in "$@"; do
 if [ -L "$file" ]; then
 if ! file=$( readlink -f "$file" ); then
 printf >&2 '%s\n' "readlink -f $file failed."
 return 1
 fi
 fi
 set -- "$@" "$file"
 shift
 done
 fi
 if [ "$editor_type" = 'gui' ]; then
 # 1. GUI editors will "sit" on the terminal until closed thanks to the wait option
 # 2. Various editors errors might flood the terminal, so we redirect all output,
 # into prepared temporary file in order to filter possible "sudoedit: file unchanged" lines.
 if tmpfile=$( mktemp /tmp/sudoedit_run.XXXXXXXXXX ); then
 SUDO_EDITOR="$editor_path -w" sudoedit -- "$@" > "$tmpfile" 2>&1
 exit_code=$?
 grep 'sudoedit:' "$tmpfile"
 rm "$tmpfile"
 return "$exit_code"
 else
 printf >&2 '%s\n' "mktemp /tmp/sudoedit_run.XXXXXXXXXX failed."
 return 1
 fi
 else
 # 1. CLI editors do not cause problems mentioned above.
 # 2. This is a generic and proper way using `sudoedit`;
 # Running the editor with one-time SUDO_EDITOR setup.
 SUDO_EDITOR="$editor_path" sudoedit -- "$@"
 fi
}
# Editor aliases generators:
# - avoid generating editors aliases for which editor is not installed
# - avoid generating editors aliases for which already have an alias
for cli_editor in $cli_editors; do
 if command -v "$cli_editor" > /dev/null 2>&1; then
 # shellcheck disable=SC2139,SC2086
 if [ -z "$( alias su$cli_editor 2> /dev/null)" ]; then
 alias su$cli_editor="sudoedit_run cli $cli_editor"
 fi
 fi
done
for gui_editor in $gui_editors; do
 if command -v "$gui_editor" > /dev/null 2>&1; then
 # shellcheck disable=SC2139,SC2086
 if [ -z "$( alias su$gui_editor 2> /dev/null)" ]; then
 alias su$gui_editor="sudoedit_run gui $gui_editor"
 fi
 fi
done
unset cli_editors gui_editors cli_editor gui_editor