Re: Cleaning up git user-interface warts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Petr Baudis <pasky@xxxxxxx> 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).

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]