Re: [PATCH] Fix file mark handling and sort side-effects in git.el

On Sun, Feb 15, 2009 at 9:08 AM, Alexandre Julliard [off-list ref] wrote:

Brent Goodrick [off-list ref] writes:

quoted

My rationale for adding append function calls to the sort calls is to
leave the callers value alone since the caller needs to make use of
the list value in subsequent operations, especially for issuing
messages.

My point is that the callers that need it should take care of it
themselves, instead of forcing a copy even in cases where it's not
necessary. And the copy can most likely be avoided completely by
changing how the success message is printed.

quoted

 > If you mean using git-add-file to do an update-index on an already
 > tracked file, that's not what it's meant to do.

That would be fine in, say, Perforce where once a file is added it
stays added even if the user mades additional edits. I don't agree
that is the best approach in the case the Emacs interface to git in
git.el, since there is that "third" state where I could have added the
file, then edited it, then forgot that I had edited it and proceeded
naively to commit, only to be surprised later that the subsequent edit
to the file was not committed.

The design of git.el is that the index is not exposed directly, it's
treated as an implementation detail. So "add" in git.el is only for
adding an untracked file, it's not for updating the index contents of an
already tracked file; that's an unnecessary operation since git.el uses
the file marks to determine what gets committed.

Ok, now that makes sense to me. Part of the problem here is that there
is no statement in the user manual about git.el's intent to hide the
index. Perhaps something to the effect of "If you are new to using
Emacs but not new to git, then you need to know that bla bla ...".
Otherwise, I think users may get tripped up by this as I was. Was
there a manual in the works for git.el or did I just miss it in recent
checkins?

It does get a bit confusing if you constantly mix command-line and
git.el commands, but you are not supposed to do that, you should be able
to do everything from the git.el buffer. I'm sure hiding the index
offends the git purists, but IMHO it makes things more Emacs-ish and
easier to use, especially if you are used to things like dired or
pcl-cvs or vc-dir with other VC systems.

That makes sense to me from the Emacs standpoint.

However, there is still a minor bug: If you edit two new files (not
known to git), then run M-x git-status, those two new files will show
up as Unknown as expected. But, mark both of them, and type "a" will
show a message in the minibuffer for only one of the files. If you
then look in the *Messages* buffer, you will find that a message for
only one of the files.

However, the *git-status* buffer does properly reflect the two added
files by their state being changed to "Added". Since you may have a
ton of files that are being added, it probably doesn't make a whole
lot of sense to dump a long message into the minibuffer with all of
those names.  By the same token, it doesn't make sense to emit one
message per file either. Instead, would you be willing to change that
message to just state "Added n files" where "n" is the number of files
added?  I would provide a patch, but that patch would necessitate the
fix to the "files" variable being damaged by the sort function, since
the git.el code seems to not even to take that into account (and is
the root cause of why only one message is emitted AFAIK).  The other
alternative is to take the length of the files list before calling the
list-damaging functions, but that just seems so not in the spirit of
current Elisp coding practice.

Thanks,
bg