Re: Shell script cleanups/style changes?

From: David Kastrup <dak@gnu.org>
Date: Aug 2, 2007, 20:57 UTC
Message-ID: <85odhpzmbo.fsf@lola.goethe.zz>
In-Reply-To: <7vlkctvfk9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> David Kastrup <dak@gnu.org> writes:
>
>> 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.
>
> As to Bourne-ness of the shell script, please realize that your
> maintainer is very old fashioned ;-), but is willing to be taught
> new tricks within reason.

Most of the "new tricks" I try on bash, dash and ash.

> We try to limit ourselves to -, =, ?, + (and their colon "if
> empty" variants when it really make sense) in parameter
> expansion of shell variables.  We also use % and # (and their
> "match largest" variants).

You do?

Indeed:

-*- mode: grep; default-directory: "/home/tmp/git/" -*- Grep started at Thu Aug 2 22:47:30

grep -nH -e '\${[a-zA-Z0-9_]*[#%]' *.sh git-am.sh:146: resolvemsg=${1#--resolvemsg=}; shift ;; git-clone.sh:358: destname="refs/$branch_top/${name#refs/heads/}" ;; git-clone.sh:360: destname="refs/$tag_top/${name#refs/tags/}" ;; git-filter-branch.sh:361: ref="${ref#refs/tags/}" git-pull.sh:98: curr_branch=${curr_branch#refs/heads/} git-rebase.sh:93: eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"' git-stash.sh:52: branch=${branch#refs/heads/}

Grep finished (matches found) at Thu Aug  2 22:47:31

I am confused now: a different poster adamantly stated that /bin/sh on Solaris did not support those constructs, and that every functionality of git was working fine for him.

> Non POSIX substitions such as ${parameter/pattern/string} and
> ${parameter:offset} are not to be used.  We do not want to
> depend on bash.

Sure. What about the git-rebase line using $(($end - $msgnum)) ? That's even more risque than ##.

> After 1.5.3 git-commit.sh will hopefully become built-in, so I would
> rather not touch the script.

Too bad: this should mean that $EDITOR can get called from C... I've been glad to see that so far this could be avoided.

> Certainly, the kind of change that is "intended to be style-only but
> somebody needs to make sure it does not introduce regression to
> everybody's shell" is very unwelcome at this point.

Understood. But using ${...#...} and ${...:+...} does not exactly seem to be news in the git code base. Even though we have the claim that Solaris' sh won't deal with the former.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum