git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Shell script cleanups/style changes?

From
David Kastrup <dak@gnu.org>
Date
Aug 2, 2007, 10:44 UTC
Message-ID
<86bqdqkygp.fsf@lola.quinscape.zz>

Hi, I wanted to ask what the general stance towards shell script cleanups and simplifications would be. For example, I find the expr usage quite inscrutable in commit, and there is no necessity of putting "shift" in every case branch instead of once behind it, and a lot of conditionals and other manipulations can be made much easier on the eye by using parameter expansion patterns that are, as far as I can see, available with every reasonable Bourne Shell and clones.

Here is an example context diff (in this case, I find it more readable than unified) to illustrate (untested!, please don't apply without a regular formatted git patch).

Should I bother doing such cleanups as I read up on code, or should I just leave things alone?

diff --git a/git-commit.sh b/git-commit.sh
index d7e7028..bdf20be 100755
*** a/git-commit.sh
--- b/git-commit.sh
***************
*** 97,101 ****
  		no_edit=t
  		log_given=t$log_given
  		logfile="$1"
- 		shift
  		;;
--- 97,100 ----
***************
*** 102,107 ****
  	-F*|-f*)
  		no_edit=t
  		log_given=t$log_given
! 		logfile=`expr "z$1" : 'z-[Ff]\(.*\)'`
! 		shift
  		;;
--- 101,105 ----
  	-F*|-f*)
  		no_edit=t
  		log_given=t$log_given
! 		logfile="${1#-?}"
  		;;
***************
*** 108,113 ****
  	--F=*|--f=*|--fi=*|--fil=*|--file=*)
  		no_edit=t
  		log_given=t$log_given
! 		logfile=`expr "z$1" : 'z-[^=]*=\(.*\)'`
! 		shift
  		;;
--- 106,110 ----
  	--F=*|--f=*|--fi=*|--fil=*|--file=*)
  		no_edit=t
  		log_given=t$log_given
! 		logfile="${1#*=}"
  		;;
***************
*** 114,117 ****
  	-a|--a|--al|--all)
  		all=t
- 		shift
  		;;
--- 111,113 ----
***************
*** 118,127 ****
  	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
! 		force_author=`expr "z$1" : 'z-[^=]*=\(.*\)'`
! 		shift
  		;;
  	--au|--aut|--auth|--autho|--author)
  		case "$#" in 1) usage ;; esac
  		shift
  		force_author="$1"
- 		shift
  		;;
--- 114,121 ----
  	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
! 		force_author="${1#*=}"
  		;;
  	--au|--aut|--auth|--autho|--author)
  		case "$#" in 1) usage ;; esac
  		shift
  		force_author="$1"
  		;;
***************
*** 128,144 ****
  	-e|--e|--ed|--edi|--edit)
  		edit_flag=t
- 		shift
  		;;
  	-i|--i|--in|--inc|--incl|--inclu|--includ|--include)
  		also=t
- 		shift
  		;;
  	--int|--inte|--inter|--intera|--interac|--interact|--interacti|\
  	--interactiv|--interactive)
  		interactive=t
- 		shift
  		;;
  	-o|--o|--on|--onl|--only)
  		only=t
- 		shift
  		;;
--- 122,134 ----
***************
*** 145,159 ****
  	-m|--m|--me|--mes|--mess|--messa|--messag|--message)
  		case "$#" in 1) usage ;; esac
- 		shift
  		log_given=m$log_given
! 		if test "$log_message" = ''
! 		then
! 		    log_message="$1"
! 		else
! 		    log_message="$log_message
  
! $1"
! 		fi
  		no_edit=t
- 		shift
  		;;
--- 135,142 ----
  	-m|--m|--me|--mes|--mess|--messa|--messag|--message)
  		case "$#" in 1) usage ;; esac
  		log_given=m$log_given
! 		log_message="${log_message}${log_message:+
  
! }$1"
  		no_edit=t
  		;;
***************
*** 160,172 ****
  	-m*)
  		log_given=m$log_given
! 		if test "$log_message" = ''
! 		then
! 		    log_message=`expr "z$1" : 'z-m\(.*\)'`
! 		else
! 		    log_message="$log_message
  
! `expr "z$1" : 'z-m\(.*\)'`"
! 		fi
  		no_edit=t
- 		shift
  		;;
--- 143,149 ----
  	-m*)
  		log_given=m$log_given
! 		log_message="${log_message}${log_message:+
  
! }${1#-m}"
  		no_edit=t
  		;;
***************
*** 173,185 ****
  	--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
  		log_given=m$log_given
! 		if test "$log_message" = ''
! 		then
! 		    log_message=`expr "z$1" : 'z-[^=]*=\(.*\)'`
! 		else
! 		    log_message="$log_message
  
! `expr "z$1" : 'zq-[^=]*=\(.*\)'`"
! 		fi
  		no_edit=t
- 		shift
  		;;
--- 150,156 ----
  	--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
  		log_given=m$log_given
! 		log_message="${log_message}${log_message:+
  
! }${1#*=}"
  		no_edit=t
  		;;
***************
*** 186,197 ****
  	-n|--n|--no|--no-|--no-v|--no-ve|--no-ver|--no-veri|--no-verif|\
  	--no-verify)
  		verify=
- 		shift
  		;;
  	--a|--am|--ame|--amen|--amend)
  		amend=t
  		use_commit=HEAD
- 		shift
  		;;
  	-c)
  		case "$#" in 1) usage ;; esac
--- 157,166 ----
***************
*** 199,203 ****
  		log_given=t$log_given
  		use_commit="$1"
  		no_edit=
- 		shift
  		;;
--- 168,171 ----
***************
*** 204,213 ****
  	--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
  	--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
  	--reedit-messag=*|--reedit-message=*)
  		log_given=t$log_given
! 		use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
  		no_edit=
- 		shift
  		;;
  	--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
  	--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
--- 172,180 ----
  	--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
  	--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
  	--reedit-messag=*|--reedit-message=*)
  		log_given=t$log_given
! 		use_commit="${1#*=}"
  		no_edit=
  		;;
  	--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
  	--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
***************
*** 217,223 ****
  		log_given=t$log_given
  		use_commit="$1"
  		no_edit=
- 		shift
  		;;
  	-C)
  		case "$#" in 1) usage ;; esac
--- 184,189 ----
***************
*** 225,229 ****
  		log_given=t$log_given
  		use_commit="$1"
  		no_edit=t
- 		shift
  		;;
--- 191,194 ----
***************
*** 230,239 ****
  	--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
  	--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
  	--reuse-message=*)
  		log_given=t$log_given
! 		use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
  		no_edit=t
- 		shift
  		;;
  	--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
  	--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
--- 195,203 ----
  	--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
  	--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
  	--reuse-message=*)
  		log_given=t$log_given
! 		use_commit="${1#*=}"
  		no_edit=t
  		;;
  	--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
  	--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
***************
*** 242,273 ****
  		log_given=t$log_given
  		use_commit="$1"
  		no_edit=t
- 		shift
  		;;
  	-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
  		signoff=t
- 		shift
  		;;
  	-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)
  		case "$#" in 1) usage ;; esac
  		shift
  		templatefile="$1"
  		no_edit=
- 		shift
  		;;
  	-q|--q|--qu|--qui|--quie|--quiet)
  		quiet=t
- 		shift
  		;;
  	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
  		verbose=t
- 		shift
  		;;
  	-u|--u|--un|--unt|--untr|--untra|--untrac|--untrack|--untracke|\
  	--untracked|--untracked-|--untracked-f|--untracked-fi|--untracked-fil|\
  	--untracked-file|--untracked-files)
  		untracked_files=t
- 		shift
  		;;
  	--)
  		shift
--- 206,231 ----
***************
*** 280,285 ****
--- 238,244 ----
  		break
  		;;
  	esac
+ 	shift
  done
  case "$edit_flag" in t) no_edit= ;; esac
  
***************
*** 437,448 ****
  
  if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
  then
! 	if test "$TMP_INDEX"
! 	then
! 		GIT_INDEX_FILE="$TMP_INDEX" "$GIT_DIR"/hooks/pre-commit
! 	else
! 		GIT_INDEX_FILE="$USE_INDEX" "$GIT_DIR"/hooks/pre-commit
! 	fi || exit
  fi
  
  if test "$log_message" != ''
--- 396,403 ----
  
  if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
  then
!     GIT_INDEX_FILE="${TMP_INDEX:-${USE_INDEX}}" "$GIT_DIR"/hooks/pre-commit \
!     || exit
  fi
  
  if test "$log_message" != ''

-- 
David Kastrup
Next: Robert Schiele
Message 1 of 42 in “Shell script cleanups/style changes?”
  1. David KastrupAug 2, 2007
  2. Robert SchieleAug 2, 2007
  3. David KastrupAug 2, 2007
  4. Robert SchieleAug 2, 2007
  5. Johannes SchindelinAug 2, 2007
  6. Robert SchieleAug 2, 2007
  7. Uwe Kleine-KönigAug 3, 2007
  8. Bradford SmithAug 2, 2007
  9. Sam RavnborgAug 2, 2007
  10. Junio C HamanoAug 2, 2007
  11. Nguyen Thai Ngoc DuyAug 2, 2007
  12. David KastrupAug 2, 2007
  13. Junio C HamanoAug 2, 2007
  14. David KastrupAug 2, 2007
  15. Junio C HamanoAug 2, 2007
  16. Junio C HamanoAug 2, 2007
  17. David KastrupAug 2, 2007
  18. Robert SchieleAug 2, 2007
  19. David KastrupAug 2, 2007
  20. Junio C HamanoAug 2, 2007
  21. David KastrupAug 2, 2007
  22. Junio C HamanoAug 3, 2007
  23. David KastrupAug 3, 2007
  24. Junio C HamanoAug 3, 2007
  25. David KastrupAug 3, 2007
  26. Junio C HamanoAug 3, 2007
  27. David KastrupAug 3, 2007
  28. David KastrupAug 3, 2007
  29. Robert SchieleAug 3, 2007
  30. David KastrupAug 3, 2007
  31. Robert SchieleAug 2, 2007
  32. David KastrupAug 2, 2007
  33. Robert SchieleAug 2, 2007
  34. David KastrupAug 2, 2007
  35. Robert SchieleAug 3, 2007
  36. David KastrupAug 3, 2007
  37. Junio C HamanoAug 3, 2007
  38. Robert SchieleAug 2, 2007
  39. Florian WeimerAug 4, 2007
  40. Robert SchieleAug 4, 2007
  41. Junio C HamanoAug 4, 2007
  42. David KastrupAug 2, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.