The official name of the hash function is SHA-1. I tried to make the usage
more consistent in the documentation, the comments and the messages (it
was variously sha1, SHA1, etc). Also some comment reformatting while I was
at it. The resulting git passes all tests.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513

-

[ message continues ]

From: Horst H. von Brand <vonbrand@inf.utfsm.cl> - unquoted

Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
---
 Documentation/git-receive-pack.txt |   14 ++++++------
 Documentation/git-rev-parse.txt    |    8 +++---
 Documentation/git-show-branch.txt  |    4 +-
 Documentation/git-show-index.txt   |    2 +-
 Documentation/git-show-ref.txt     |    4 +-
 Documentation/git-svn.txt          |    6 ++--
 Documentation/git-tag.txt          |    2 +-
 Documentation/git-unpack-file.txt  |    2 +-
 Documentation/git-update-index.txt |   14 ++++++------
 Documentation/git-verify-pack.txt  |    4 +-
 Documentation/git-verify-tag.txt   |    2 +-
 Documentation/git.txt              |    4 +-
 Documentation/glossary.txt         |    8 +++---
 Documentation/pretty-formats.txt   |   16 +++++++-------
 Documentation/tutorial-2.txt       |   16 +++++++-------
 builtin-apply.c                    |   18 ++++++++++------
 builtin-blame.c                    |   20 ++++++++++++------
 builtin-cat-file.c                 |    5 ++-
 builtin-commit-tree.c              |    3 +-
 builtin-describe.c                 |    3 +-
 builtin-diff-tree.c                |    9 ++++---
 builtin-diff.c                     |    6 +++-
 builtin-for-each-ref.c             |    6 +++-
 builtin-init-db.c                  |    2 +-
 builtin-log.c                      |    5 ++-
 builtin-name-rev.c                 |    2 +-
 builtin-pack-objects.c             |   21 +++++++++++--------
 builtin-prune.c                    |    2 +-
 builtin-push.c                     |    4 ++-
 builtin-read-tree.c                |    6 ++++-
 builtin-reflog.c                   |    3 +-
 builtin-rev-list.c                 |    6 +++-
 builtin-rm.c                       |    8 ++++--
 builtin-shortlog.c                 |    2 +-
 builtin-show-branch.c              |   19 ++++++++++++-----
 builtin-show-ref.c                 |    8 +++++-
 builtin-unpack-objects.c           |    2 +-
 builtin-update-index.c             | ...
[ message continues ]

A good cleanup and correction.  We've apparently been a little lax.
Its a noble and worthy goal to make the correction and I applaud

However I cannot help but feel that this hunk is unrelated to the
theme of this extremely large patch.  I don't know how Junio feels,
but this late in the 1.5.0 series I'm a little leary of a 1600+
line patch which is changing so much code, even if its something
as trivial as the above hunk.

-- 
Shawn.
-

[ message continues ]

Maybe the patch could be restricted to documentation fixes only for now?


Nicolas
-

[ message continues ]

That's what I tried to do. But just changing the documentation without
changing (some) of the messages and so on just gets you worse
inconsistency.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513
-

[ message continues ]

I'm not at all against updating output messages to match the
documentation.  Indeed, its good to make them consistent.

It just would be a lot easier to review the patch if it wasn't 1600+
lines, _especially_ when some of the hunks really aren't related
to the theme of the patch.

Of course with the amount of time I've now spent writing email
saying how hard it is to review the original patch, I could have just
read through the entire thing and given it another set of eyeballs.
Argh, another great misuse of my time.  :-)

-- 
Shawn.
-

[ message continues ]

The patch should probably only change sha1 to SHA-1 and not reformat
the initialisation of _usage arrays or the comments (new line before
first line of comment).  If the reformatting is desired it should be a
separate patch imho.  Here is what I found:

sha1 -> SHA-1, dunno what to do about the sha2 and sha3 :)
There is also a space at eol.

The following have a space at eol and/or spaces instead of tabs for
-

[ message continues ]

I do agree the original patch conflates many different things,
and it would be nicer to do this clean-up as separate pieces.

* Code and comment reformatting.

  I agree that multi-line comment should begin with "/*\n",
  the comment sentences should begin with an indent and "* ",
  and the comment block should end with an indent and "*/\n".

  But this obviously belongs to a separate clean-up.

* The official name of these 40-hexdigit thingy we use to name
  objects is "object name" (see Documentation/glossary.txt).

  Taking an example from this hunk from 'update' hook
  documentation:

    @@ -30,12 +30,12 @@ and executable, it is called with three parameters:

            $GIT_DIR/hooks/update refname sha1-old sha1-new

    +The refname parameter is relative to $GIT_DIR; e.g. for the master
    +head this is "refs/heads/master".  The two sha1 are the object names
    +for the refname before and after the update.  Note that the hook is
    +called before the refname is updated, so either sha1-old is 0{40}
    +(meaning there is no such ref yet), or it should match what is
    +recorded in refname.

  I would prefer "the two object names are for the refname before...".

* Some commands take any object name, while some others only
  take committish.  For example, this hunk for show-branch:

    @@ -29,7 +29,7 @@ no <rev> nor <glob> is given on the command line.
     OPTIONS
     -------
     <rev>::
    -	Arbitrary extended SHA1 expression (see `git-rev-parse`)
    +	Arbitrary extended SHA-1 expression (see `git-rev-parse`)
            that typically names a branch HEAD or a tag.

     <glob>::

  is not Horst's fault but this needs to name a committish, so
  rephrasing it to "an arbitrary object name" is not even correct
  (let alone spellfixing SHA-1).

* The name of the hash function we currently happen to use, in
  order to come up with an "object name", is SHA-1 not SHA1.

  Currently we say sha1 and sha-1 interchangeably, but if we aim
  for ...
[ message continues ]

FYI:

As you know I've got some patches that fix use of literal numbers for the hash 
sizes instead of SOME_CONSTANT.  I've further got one that does the same for 
the literal uses of "refs/" et al.  I'm holding off on these until after 1.5 

As a further to the above cleanups, I'm also planning to fix all the sha1 
named variables to be "hash" or "object" or something.  It strikes me that 

The reasons for wanting this are, I hope, obvious.  The variables (and 
parameters) accept object-names not SHA-1 hashes.   The fact that the objects 
are named after a SHA-1 isn't relevant to users; and shouldn't be relevant 
for the variable names, simply to promote abstraction from what the actual 
hash function is.

I mention it here because it seems to fit with this cleanup theme.  Am I still 
correct that you would want this sort of thing post-1.5?  Is it even a 
reasonable goal to have?


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com
-

[ message continues ]