Re: [PATCH 3/5] Replace $((...)) with expr invocations.

Ralf Wildenhues [off-list ref] writes:

* Ralf Wildenhues wrote on Tue, Nov 06, 2007 at 09:18:09PM CET:

quoted

---
 git-filter-branch.sh       |    4 ++--
 git-rebase--interactive.sh |    8 ++++----
 git-rebase.sh              |    8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

Hmm, maybe this one is overkill.  $((...)) is POSIX, I temporarily
forgot (thanks Benoît!).

I'm unsure whether git targets non-POSIX Bourne shells like Solaris
/bin/sh.  That would however mean replacing stuff like $(cmd) with
`cmd` as well, and from grepping the source it looks like you'd rather
avoid that.

For git, two rough rules are:

 - Most importantly, we never say "It's in POSIX; we'll happily
   screw your system that does not conform."  We live in the
   real world.

 - However, we often say "Let's stay away from that construct,
   it's not even in POSIX".

For shell scripts specifically (not exhaustive):

 - We prefer $( ... ) for command substitution; unlike ``, it
   properly nests.  It should have been the way Bourne spelled
   it from day one, but unfortunately isn't.

 - We use ${parameter-word} and its [-=?+] siblings, and their
   colon'ed "unset or null" form.

 - We use ${parameter#word} and its [#%] siblings, and their
   doubled "longest matching" form.

 - We use Arithmetic Expansion $(( ... )).

 - No "Substring Expansion" ${parameter:offset:length}.

 - No shell arrays.

 - No strlen ${#parameter}.

 - No regexp ${parameter/pattern/string}.

 - We do not use Process Substitution <(list) or >(list).

 - We prefer "test" over "[ ... ]".

 - We do not write noiseword "function" in front of shell
   functions.

Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

Signed-off-by: Johannes Schindelin <redacted>
---

	I long resisted in adding this, as I really believe our code
	base is a very clean one, and a good description of what we
	prefer.

	But it seems that not everybody has the time to study our
	code in depth, beautiful as it is ;-)

	BTW the first to catch the allusion to a certain movie 
	wins a drink with me.

 Documentation/CodingStyle |   87 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 87 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/CodingStyle

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
new file mode 100644
index 0000000..622b80b
--- /dev/null
+++ b/Documentation/CodingStyle

@@ -0,0 +1,87 @@
+As a popular project, we also have some guidelines to keep to the
+code.  For git in general, two rough rules are:
+
+ - Most importantly, we never say "It's in POSIX; we'll happily
+   screw your system that does not conform."  We live in the
+   real world.
+
+ - However, we often say "Let's stay away from that construct,
+   it's not even in POSIX".
+
+As for more concrete guidelines, just imitate the existing code
+(this is a good guideline, no matter which project you are contributing
+to...).  But if you must have some list of rules, here they are.
+
+For shell scripts specifically (not exhaustive):
+
+ - We prefer $( ... ) for command substitution; unlike ``, it
+   properly nests.  It should have been the way Bourne spelled
+   it from day one, but unfortunately isn't.
+
+ - We use ${parameter-word} and its [-=?+] siblings, and their
+   colon'ed "unset or null" form.
+
+ - We use ${parameter#word} and its [#%] siblings, and their
+   doubled "longest matching" form.
+
+ - We use Arithmetic Expansion $(( ... )).
+
+ - No "Substring Expansion" ${parameter:offset:length}.
+
+ - No shell arrays.
+
+ - No strlen ${#parameter}.
+
+ - No regexp ${parameter/pattern/string}.
+
+ - We do not use Process Substitution <(list) or >(list).
+
+ - We prefer "test" over "[ ... ]".
+
+ - We do not write noiseword "function" in front of shell
+   functions.
+
+For C programs:
+
+ - Use tabs to increment, and interpret tabs as taking up to 8 spaces
+
+ - Try to keep to 80 characters per line
+
+ - When declaring pointers, the star sides with the variable name, i.e.
+   "char *string", not "char* string" or "char * string".  This makes
+   it easier to understand "char *string, c;"
+
+ - Do not use curly brackets unnecessarily.  I.e.
+
+	if (bla) {
+		x = 1;
+	}
+
+   is frowned upon.  A gray area is when the statement extends over a
+   few lines, and/or you have a lengthy comment atop of it.
+
+ - Try to make your code understandable.  You may put comments in, but
+   comments invariably tend to stale out when the code they were
+   describing changes.  Often splitting a function into two makes the
+   intention of the code much clearer.
+
+   Double negation is often harder to understand than no negation at
+   all.
+
+   Some clever tricks, like using the !! operator with arithmetic
+   constructs, can be extremely confusing to others.  Avoid them,
+   unless there is a compelling reason to use them.
+
+ - Use the API.  No, really.  We have a strbuf (variable length string),
+   several arrays with the ALLOC_GROW() macro, a path_list for sorted
+   string lists, a hash map (mapping struct objects) named
+   "struct decorate", amongst other things.
+
+ - #include system headers in git-compat-util.h.  Some headers on some
+   systems show subtle breakages when you change the order, so it is
+   best to keep them in one place.
+
+ - if you are planning a new command, consider writing it in shell or
+   perl first, so that changes in semantics can be easily changed and
+   discussed.  Many git commands started out like that, and a few are
+   still scripts.

-- 
1.5.3.5.1597.g7191

Johannes Schindelin wrote:

Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

Sweet. You just saved me 40 minutes of writing one just like that for
company use. I owe you a drink, and then you can tell me what movie
you alluded to ;-)

+
+ - if you are planning a new command, consider writing it in shell or
+   perl first, so that changes in semantics can be easily changed and
+   discussed.  Many git commands started out like that, and a few are
+   still scripts.

I'd skip this part though and just add a pointer to contrib/examples/,
saying something along the lines of

- if you're planning a new command, sneak a peak in contrib/examples/
  for ample study-material of retired git commands implemented in perl
  and shell.

Possibly with s/retired // on that paragraph.

There's nothing particular wrong with writing in C from the start after
all.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

El 7/11/2007, a las 0:17, Johannes Schindelin escribió:

Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

Great idea, Johannes, especially your nice concise summary of  
conventions for shell-scripts (which is an area where we most often  
see list traffic on which way to write things).

Cheers,
Wincent

Wincent Colaiuta wrote:

El 7/11/2007, a las 0:17, Johannes Schindelin escribió:

quoted

Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

Great idea, Johannes, especially your nice concise summary of 
conventions for shell-scripts (which is an area where we most often see 
list traffic on which way to write things).

That was ripped from a mail Junio sent to the list though ;-)

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

On Tue, 2007-11-06 at 17:17, Johannes Schindelin wrote:

+
+ - Do not use curly brackets unnecessarily.  I.e.
+
+	if (bla) {
+		x = 1;
+	}

In my opinion, I think this is a bad guideline.

+   is frowned upon.  A gray area is when the statement extends over a
+   few lines, and/or you have a lengthy comment atop of it.

Or if it is some macro, or any number of vague problem areas.

Again, in my opinion, one should always take the safer
defensive programming tactic and always use braces.
Having them really never produces errors, while omitting
them is often error prone.

Yes, I know that is not a popular opinion by example,
but I'm still allowed to state it. :-)
Feel free to ignore me as well. :-)

jdl

Hi,

On Wed, 7 Nov 2007, Jon Loeliger wrote:

On Tue, 2007-11-06 at 17:17, Johannes Schindelin wrote:

quoted

+
+ - Do not use curly brackets unnecessarily.  I.e.
+
+	if (bla) {
+		x = 1;
+	}

In my opinion, I think this is a bad guideline.

In my opinion, this is a good guideline.

So now what? Let's have another pointless flamewar?

Ciao,
Dscho

On Nov 6, 2007 11:17 PM, Johannes Schindelin [off-list ref] wrote:

Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

Might this file be a place to document exactly which licenses we
accept code under? I.e. is it GPLv2 only or any license compatible
with GPLv2 ?

Currently I see only GPLv2 in core git, but I have a series of patches
which give me a speed-up of two orders of magnitude for some cases of
at least git-status (on my platform, with my repos etc) and so far my
two approaches involve bringing in code which is either public domain
with restrictions (which may be too onerous for us to carry in
mainline) or 3-clause BSD.

Cheers, Mike