Re: [Qgit RFC] commit --amend

On Wed, Jul 04, 2007 at 14:44:16 +0200, Marco Costalba wrote:

On 7/4/07, Junio C Hamano [off-list ref] wrote:

quoted

But if a Porcelain like StGIT or Qgit would want to do that kind
of operation for different use case than "amending", it can and
should use plumbing commands, just like the implementation of
"commit --amend" does, with different constraints and error
checks.

I would prefer if there was something between git-commit-tree and git-commit.
There are several steps one has to do for commit, that are the same for most
ways of commit:
 - call pre-commit hook (unless --no-verify)
 - write-tree
 - commit-tree
 - update-ref
 - mv next-index index
 - call post-commit hook (unless --no-verify or unconditionally?)

Would factoring out such script from the end of git-commit.sh be accepted?

Or would it be possible to get just that from git-commit with right options?
Basically I need to replicate the logic with 

I would suggest a name git-commit-index. It would take the index to commit,
index to move in after commit, head to update, list of parents and commit
message on standard input (as commit-tree does).

The other thing is, that of course qgit (or any other frontend) can't start
using it until it's accepted and released with git. So I'll first try to get
it working in qgit and than think about making it a separate plumbing command
in git.

I always prefer qgit to use the highest level commands as possible because:

1- Less error prone
2- Easier to implement

Definitely.

3- More robust to API change
4- Less easy to break by changes in git.

Actually, no. The porcelains are more likely to change than the plumbing.

Having said that, from '-F' option documentation:

-F <file>::
	Take the commit message from the given file.  Use '-' to
	read the message from the standard input.

Jan, what about to use '-' and feed message from stdin?

I actually am, because I am rewriting it to use plumbing, which means
git-write-tree and git-commit-tree directly. And git-commit-tree always reads
commit message from stdin.

Indeed the full signature of run() is:

bool Git::run(SCRef runCmd, QString* runOutput, QObject* receiver, SCRef 
buf)

Where the last parameter 'buf' it's a string that, if not empty, is
passed to the launched program stdin.

... except if I read the code correctly, it will create a temporary file
anyway. The comment in QGit::startProcess says it is because of windows, but
there is nothing to disable it in Unix, so to me it seems temporary file is
used anyway.

I don't know if it is already too late, but I would suggest to stick
to git-commit if possible, I see only downsides in not doing so. But
of course who writes the code decides.

The old code needs rewriting in any case, because if I read it correctly, it
will not commit merges either. Yes, you rarely do it, because git
automatically commits non-conflicting merges, but amending such commits is
much more common.

I would personally most like qgit to do all playing with index on it's own
and than call a single command to commit the index. But commit can't really
do that, because I can't give commit the parent list and I don't like the
idea of reset --soft HEAD^ + reinstate whatever MERGE_HEAD there needs to be
(to not clutter reflog and also to leave the tree is as sensible state as
possible if something goes wrong).

-- 
						 Jan 'Bulb' Hudec [off-list ref]