Here are four versions of essentially the same program:
a wrapper to edit a file under SCCS version control.
The basic task is to use the sccs command to "check out" a file under
version control, and then automatically edit the file. The script will then
be used by "users", aka coders, who may not be particularly advanced UNIX
users. Hence, the need for a wrapper script.
While the basic functionality is the same across all versions , the differences in safety and usability between the first version and the last version are staggering.
The first one is extremely bad: it would be written by someone who has just picked up a book on shellscripting, and has decided, "I'm a programmer now".
The second one is an improvement, showing some consideration to potential users by having safety checks.
The third one is a good, solid version. It's a positive role model for your scripts.
The final one is a full-blown, paranoid, commented program unto itself, with whiz-bang features. This is the way a professional programmer would write it. Don't let that put you off: it's the way you can and should write it too! You might start with a version like the initial dumb one, as the initial step in your code development, just to make sure you have the basic functionality for the task. But after it is shown to work, you should upgrade it to a more reasonable one immediately.
Note: there is a summary of good practices show in this page, at the bottom.
#!/bin/ksh sccs edit $1 if [ "$EDITOR" = "" ] ; then EDITOR=vi fi $EDITOR $1 |
This version makes somewhat of an attempt to be user friendly, by having a check for a user-specified EDITOR setting, and using it if available. However, there are no comments, no error checking, and no help for the user whatsoever!
#!/bin/ksh # Author: Joe Newguy if [ $# -lt 1 ] ; then print "This program will check out a file, or files, with sccs" exit 1 fi # set EDITOR var to "vi" if not already set EDITOR=${EDITOR:-vi} sccs edit $@ $EDITOR $@ |
This is somewhat of a step above the prior version. It accepts multiple files as potential arguments. It's always nice to be flexible about the number of files your scripts can handle. It also has a usage message, if the script is called without arguments. Plus, it's always a good idea to put your name in in, unless you're working for the company of "Me, Myself and I, Inc."
Unfortunately, there is still quite a bit lacking, as you can tell by comparing it to the next version.
#!/bin/ksh # SCCS editing wrapper, version 0.3 # Author - Sys Admin # Usage: see usage() function, below usage(){ print sedit - a wrapper to edit files under SCCS print "usage: sedit file {file2 ...}" } # Set EDITOR var to "vi" if not already set to something EDITOR=${EDITOR:-vi} # Could already be in path, but it doesnt hurt to add it again. # Sorry, I assume solaris machines everywhere: adjust as needed, # if your sccs lives somewheres else SCCSBIN=/usr/ccs/bin if [ ! -x $SCCSBIN/sccs ] ; then print ERROR: sccs not installed on this machine. Cannot continue. usage exit 1 fi PATH=$SCCSBIN:$PATH if [ $# -lt 1 ] ; then usage print ERROR: no files specified exit 1 fi # Yes, I could use "sccs edit $@" and check for a single error, but this # approach allows for finer error reporting for f in $@ ; do sccs edit $f if [ $? -ne 0 ] ; then print ERROR checking out file $f if [ "$filelist" != "" ] ; then print "Have checked out $filelist" fi exit 1 fi filelist="$filelist $f" done $EDITOR $filelist if [ $? -ne 0 ] ; then print ERROR: $EDITOR returned error status exit 1 fi |
This guy has been around the block a few times. He's a responsible sysadmin, who likes to be disaster-prepared. In this case, the most likely "disaster" is 100 calls from developers asking "Why doesnt it work for me?" So when things break, it's a good idea to provide as much information as possible to the user.
Nice things to note:
Compare and contrast the first version of the program, to this one. Then try to make your own scripts be more like this!
#!/bin/ksh # SCCS editing wrapper, version 1.3 # Author - Phil Brown # Usage: see usage() function, below usage(){ print sedit - a wrapper to edit files under SCCS print "Usage: sedit [-c|-C] [-f] file {file2 ...}" print " -c check in file(s) after edit is complete" print " -C check in all files with single revision message" print " -f ignore errors in checkout" } # Set EDITOR var to "vi" if not already set to something EDITOR=${EDITOR:-vi} # Could already be in path, but it doesnt hurt to add it again. # Sorry, I assume solaris machines everywhere: adjust as needed. PATH=$PATH:/usr/ccs/bin if [ ! -x /usr/ccs/bin/sccs ] ; then print ERROR: sccs not installed on this machine. Cannot continue. usage exit 1 fi while getopts "cCfh" arg do case $arg in c) checkin="yes" ;; C) checkinall="yes" ;; f) force="yes" ;; h|*) usage exit 1 ;; esac done shift $(($OPTIND - 1)) if [ $# -lt 1 ] ; then usage print ERROR: no files specified exit 1 fi if [ "$checkinall" != "" ] && [ "$checkin" != "" ] ; then print WARNING: -c and -C used. Will use -C. fi # Yes, I could use "sccs edit $@" and check for a single error, but this # approach allows for finer error reporting. # "$@" is a special construct that catches spaces in filenames. # Note that "$*" is NOT 100% the same thing. for f in "$@" ; do sccs edit "$f" if [ $? -ne 0 ] ; then print ERROR checking out file $f if [ "$force" = "" ] ; then if [ "$filelist" != "" ] ; then print "Have checked out $filelist" fi exit 1 fi # else, -f is in effect. Keep going fi filelist="$filelist $f" done # I would like to use "$filelist", but that does not preserve spaces # in file names $EDITOR "$@" status=$? if [ $status -ne 0 ] ; then print ERROR: $EDITOR returned error status $status exit $status # For editors, it isnt really important to exit with the exact same # error status. However, for more complex programs, and particularly # since this is supposed to be a more or less transparent wrapper # around $EDITOR, it would be important to return the exact same # error status of the program we are wrapping fi if [ "$checkinall" != "" ] ; then # -C option used. re-check in all files at once. sccs delget "$@" if [ $? -ne 0 ] ; then print "ERROR checking in files?" exit 1 fi exit 0 fi if [ "$checkin" != "" ] ; then # -c option used. re-check in each file. for file in $filelist ; do sccs delget $file if [ $? -ne 0 ] ; then print "WARNING: failed to check in $file" fi # do NOT stop after error. Keep trying to check # in any other files done fi |
This guy has been around the block a few times. Heck, he helped BUILD the block ;-)
This was originally my third and final version. It's the way I would really write the script. But I decided it might be a bit daunting to new scripting folks, so I made a new intermediate third step, above.
Additional things beyond the previous version: