Re: Cleaning up git user-interface warts

Petr Baudis [off-list ref] writes:

(vi) Coding issues. This is probably very subjective, but a blocker for
me. I have no issues about C here, but about the shell part of Git.
Well, how to say it... It's just fundamentally incompatible with me. I
*could* do things in/with it, but it's certainly something I wouldn't
_enjoy_ doing _at all_, on a deep level. I think the current shell code
is really hard to read, the ancient constructs are frequently strange at
best, etc. It's surely fine code at functional level and there'll be
people who hate _my_ style of coding and my shell code which isn't
perfect either, but it's just how it is with me.

I've been thinking about revamping the style of shell scripts in
git-core Porcelain-ish for some time, and I have a feeling that
now may be a good time to do so, after one feature release is
out and the list is discussing UI improvements.

But before mentioning the specifics, let me mention one tangent.
I recently installed an OpenBSD bochs (it was actually a qemu
image) without knowing much about the way of the land, and after
adjusting myself to necessary glitches (like "make" being called
"gmake" there), I saw git properly built and pass its selftest.
I was pleasantly surprised when I noticed there was no 'bash' on
the system after all that.

I would like to keep it that way.

I'll list things I would want to and not want to change.
Comments from the list are very appreciated.  You can say things
in two ways:

 * I guarantee that the _default_ shell on all sane platforms we
   care about handle this construct correctly, although it was
   not in the original Bourne.  There is no reason to stay away
   from it these days.

or

 * You've stayed away from this construct but now you say you
   feel it is Ok to use it.  Don't.  It would break with the
   shell on my platform (or "it is a bad practice because of
   such and such reasons").

I do not think many people can say the former with authority
unless you have a portability lab (the company I work for used
to be like that and it was an interesting experience to learn
all about irritating implementation differences).  And "POSIX
says shell should behave that way" is _not_ what I want to hear
about.

But the latter should be a lot easier to say, and would be
appreciated because it would help us avoid regressions.

Things I would want to change:

 - One indent level is one tab and the tab-width is eight
   columns.  Some of our scripts tend to use less than eight
   spaces for indentation to avoid line wrapping.

 - More use of shell functions are fine.   Especially if the
   above change makes lines too long, the logic should be
   refactored.

 - It is so 80-ish to follow certain portability and performance
   wisdom.  The following should go:

   . Use "case" when you do not have to use "if test".

   . Avoid ${parameter##word} and friends and use `expr` instead
     to pick a string apart.

   . Avoid "export name=word", write "name=word; export name"
     instead.

   . Avoid ${parameter:-word} and friends when ${parameter-word}
     would do.

Things I do not want to change:

 - The shell scripts should start with #!/bin/sh, not
   #!/bin/bash (nor even worse "#!/usr/bin/env sh").

 - Shell functions are written as "name () { ... }" without 
   "function" noiseword.

 - 'foo && bar || exit' exits with the error code of what
   failed; no need to say 'exit $?'.

 - String equality check for "test" is a single =, not ==. 

 - Do not use locals.

 - Do not use shell arrays.

 - In general, if something does not behave the same way in ksh,
   bash and dash, don't use it (that does not mean these three
   are special; it just means if something is not even portable
   across these three, it is a definite no-no).

I do not think I need to list other common-sense shell idioms in
the latter category (e.g. 'using "test z$name = zexpected" when
we do not know what $name contains' falls into that).

On Thu, Nov 16, 2006 at 10:49:36PM CET, Junio C Hamano wrote:

I would like to keep it that way.

I agree - I certainly don't want to infect Git with bash dependency.

And "POSIX says shell should behave that way" is _not_ what I want to
hear about.

Actually, which sane platforms we care about have /bin/sh that is NOT
POSIX compatible?

Things I would want to change:

What about [ instead of test? And

	if foo; then

instead of

	if foo
	then

?


Am I the only one who hates

case "$log_given" in
tt*)
        die "Only one of -c/-C/-F can be used." ;;
*tm*|*mt*)
        die "Option -m cannot be combined with -c/-C/-F." ;;
esac

instead of having this stuff in explicit variables and writing out some
explicit boolean expressions? (There _are_ few cases where the case is
cool, but they are rare.)


It would be really great if Git would have something alike the Cogito's
optparse infrastructure. I'm not sure if you can implement it in Bourne
sh with reasonable performance, though...


I think addressing these three particular points would make the scripts
hugely more coder-friendly. (And well, I usually say that coding style
is not *that* important and is frequently overemphasised. But that holds
only to a certain point. ;-)

Things I do not want to change:

..snip all those I agree with..

 - Do not use locals.

It's a pity. :-( Which shell doesn't support them?

It's not that huge a deal, though.

 - Do not use shell arrays.

This is quite a larger deal, I think; but the portability concerns are
very real, I guess. :|

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1