[Qgit RFC] commit --amend

[Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

Hello,

I am thinking about adding commit --amend support to Qgit.

However I see three ways to do it, so I'd like to ask whether anybody knows
some reason why any of them is better or worse, which I don't:

 - Add a button for "Amend". This would fit with what is there currently,
   because with stgit it has buttons "New patch" and "Add to top" (refresh),
   which correspond exactly to core git's "commit" and "commit --amend"
   respectively.

   Disadvantage (for current approach too) is, that when amending (resp.
   refreshing) the previous commit message is not there to be edited.

 - Add a radio button the way git-gui does. Switching the button to "amend"
   would load the previous commit message.

   This would be more invasive change, but it would allow editing the commit
   message when amending/refreshing. I fear it will be more user-error-prone
   too, though.

 - Add a separate action to the menu. This action would take over the refresh
   (Add to top) operation when using stgit.
   
   I believe this has lower risk of user errors than the previous option. It
   also has the advantage, that I don't have to touch the disabling logic for
   the commit action. Amending last commit is always possible, even if there
   are no changes, because you might want to edit the message (eg. if you
   forget to sign-off or forget to mention some change or something).

I'll try doing the first option now, unless somebody persuades me that it's
a nonsense.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 6/10/07, Jan Hudec <bulb@ucw.cz> wrote:
> Hello,
>
> I am thinking about adding commit --amend support to Qgit.
>

Good!

> However I see three ways to do it, so I'd like to ask whether anybody knows
> some reason why any of them is better or worse, which I don't:
>
>  - Add a button for "Amend". This would fit with what is there currently,
>    because with stgit it has buttons "New patch" and "Add to top" (refresh),
>    which correspond exactly to core git's "commit" and "commit --amend"
>    respectively.
>

>From 'Edit->commit' menu it is possible to open a dialog to commit
modified files in working dir.

Currently, at the bottom of the dialog there are 4 buttons:

- Settings (to open settings dialog on 'commit options' tab)

- Cancel (to cancel ;-) )

- Sync (to update the index but without writing a new tree)

- Commit (to commit a new revision)

In case the repository is under a StGIT stack the buttons are slightly
different, in particular:

Sync --> becames 'Add to top' (to fold and refresh changes)

Commit --> becames 'New patch' (to create a new patch with selected
modified files)


If I have understood correctly you plan to substitute 'Sync' button
with 'Amend'. Is it correct?

>    Disadvantage (for current approach too) is, that when amending (resp.
>    refreshing) the previous commit message is not there to be edited.
>

Yes but see below.

>  - Add a radio button the way git-gui does. Switching the button to "amend"
>    would load the previous commit message.
>
>    This would be more invasive change, but it would allow editing the commit
>    message when amending/refreshing. I fear it will be more user-error-prone
>    too, though.
>

I agree.

>  - Add a separate action to the menu. This action would take over the refresh
>    (Add to top) operation when using stgit.
>
>    I believe this has lower risk of user errors than the previous option. It
>    also has the advantage, that I don't have to touch the disabling logic for
>    the commit action. Amending last commit is always possible, even if there
>    are no changes, because you might want to edit the message (eg. if you
>    forget to sign-off or forget to mention some change or something).
>

Yes. But amending is an option of commit (also in git) so probably the
amend action will fire the commit dialog anyway and we are back to
previous situation. The only advantage is that we can load the message
of the tip revision as default instead of git-status output as the
current.


> I'll try doing the first option now, unless somebody persuades me that it's
> a nonsense.
>

I think it's the best: 'Sync' button is very seldom used (I think I've
never used it but for testing that it works) and updating the index is
something very plumbing anyway.

What we could add is another button 'Load prev msg' to do what it
says, so we would end up with 5 buttons:

- Settings /Cancel/ Load prev msg / Amend /Commit

I don't see a reason to set 'Load prev msg' as a check button, you may
want to reload the prev msg as many times as you need, (re)clicking
everytime on the button, also for a normal commit where as example you
want to keep the header or part of the subject of the previous
revision.

Please let me know if and where you find something obscure/messy with
the code, I will be happy to help you.


Marco

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]

On Mon, Jun 11, 2007 at 00:10:20 +0200, Marco Costalba wrote:
> On 6/10/07, Jan Hudec <bulb@ucw.cz> wrote:
> >Hello,
> >
> >I am thinking about adding commit --amend support to Qgit.
> >
> 
> Good!
> 
> > - Add a separate action to the menu. This action would take over the 
> > refresh
> >   (Add to top) operation when using stgit.
> >
> >   I believe this has lower risk of user errors than the previous option. 
> >   It
> >   also has the advantage, that I don't have to touch the disabling logic 
> >   for
> >   the commit action. Amending last commit is always possible, even if 
> >   there
> >   are no changes, because you might want to edit the message (eg. if you
> >   forget to sign-off or forget to mention some change or something).
> >
> 
> Yes. But amending is an option of commit (also in git) so probably the
> amend action will fire the commit dialog anyway and we are back to
> previous situation. The only advantage is that we can load the message
> of the tip revision as default instead of git-status output as the
> current.
> 
> 
> >I'll try doing the first option now, unless somebody persuades me that it's
> >a nonsense.
> >
> 
> I think it's the best: 'Sync' button is very seldom used (I think I've
> never used it but for testing that it works) and updating the index is
> something very plumbing anyway.
> 
> What we could add is another button 'Load prev msg' to do what it
> says, so we would end up with 5 buttons:
> 
> - Settings /Cancel/ Load prev msg / Amend /Commit
> 
> I don't see a reason to set 'Load prev msg' as a check button, you may
> want to reload the prev msg as many times as you need, (re)clicking
> everytime on the button, also for a normal commit where as example you
> want to keep the header or part of the subject of the previous
> revision.

I think it would be somewhat complicated unfortunately :-(. The loading of
previous message should be really tied to whether you are about to
amend/refresh or to commit/add new. In this respect this would actually match
the command-line interface, because you say whether you want to amend
*before* you edit the message.

So I am now inclined more to the separate action and trying to go that way.
But the difference between the variants would not be that big.

> Please let me know if and where you find something obscure/messy with
> the code, I will be happy to help you.

I think I mostly understood it now. Thank you.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 6/11/07, Jan Hudec <bulb@ucw.cz> wrote:
>
> So I am now inclined more to the separate action and trying to go that way.
> But the difference between the variants would not be that big.
>

Ok for me. Please, feel free to implement what's the best for you and
I will apply the patch.

Marco

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 6/11/07, Jan Hudec <bulb@ucw.cz> wrote:
>
> I think I mostly understood it now. Thank you.
>

Anyhow I think this could be useful to you:

	/*
	   getAllRefSha() returns the list of sha of a given
	   type, where type is a mask of Git::RefType flags
	   see src/git.h.
	   In this case we want the sha of the current branch
	*/
	QStringList revs = getAllRefSha(CUR_BRANCH);
	
	if (!revs.isEmpty()) {

		// all the sha info is stored in this QGit::Rev
		// class defined in src/common.h
		const Rev* r = revLookup(revs.first());

		// short log (title) is
		r->shortLog();

		// message body is
		r->longLog();

		// etc....
	}

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

On Mon, Jun 11, 2007 at 07:45:51 +0200, Marco Costalba wrote:
> On 6/11/07, Jan Hudec <bulb@ucw.cz> wrote:
> >
> >I think I mostly understood it now. Thank you.
> >
> 
> Anyhow I think this could be useful to you:
> 
> 	/*
> 	   getAllRefSha() returns the list of sha of a given
> 	   type, where type is a mask of Git::RefType flags
> 	   see src/git.h.
> 	   In this case we want the sha of the current branch
> 	*/
> 	QStringList revs = getAllRefSha(CUR_BRANCH);
> 	
> 	if (!revs.isEmpty()) {
> 
> 		// all the sha info is stored in this QGit::Rev
> 		// class defined in src/common.h
> 		const Rev* r = revLookup(revs.first());
> 
> 		// short log (title) is
> 		r->shortLog();
> 
> 		// message body is
> 		r->longLog();
> 
> 		// etc....
> 	}

Thanks.

I got stuck at git-commit --amend -F not working (because it's explicitely
forbidden), so I have to reimplement all of the commit stuff with plumbing
commands. I am in the middle of rewriting it currently. Unfortunately the
plumbing is a little too low-level.

However, I am currently not sure how to handle errors. If the current commit
fails, it will show a message box with it's output, but I can't see where it
is generated. It seems it's somewhere inside MyProcess, so I don't have to do
anything special though, right?

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 7/1/07, Jan Hudec <bulb@ucw.cz> wrote:
> On Mon, Jun 11, 2007 at 07:45:51 +0200, Marco Costalba wrote:
> > On 6/11/07, Jan Hudec <bulb@ucw.cz> wrote:
> > >
>
> However, I am currently not sure how to handle errors. If the current commit
> fails, it will show a message box with it's output, but I can't see where it
> is generated. It seems it's somewhere inside MyProcess, so I don't have to do
> anything special though, right?
>

You have 2 ways to start a git command:

QGit::run() and QGit::runAsync()

The first starts the command and wait for completion, a bool is
returned to indicate success as example

if (!run("git read-tree --reset HEAD"))
	return false;

The second one is use to start a command and return immediately, so
it's used for long commands that should be non-blocking, as example:

if (!runAsync("git diff-index -r -m --patch-with-stat HEAD"))
    return false;

In the latter case success it means the command has been _started_ successfully.


A bool is the only flag returned, because error detect is done at
lower level, in MyProcess::sendErrorMsg() in file myprocess.cpp that
handles the low level of running an external process and is called by
run().

In case of an error MyProcess::sendErrorMsg() is called to inform the
GUI (a popup dialog box) about the error and the stderr output
received.

What is an error ? :-)

It's not so trivial due to different OS and git commands behaviours
regarding stderr and exiting codes, check the comment at the beginning
of MyProcess::on_finished() in myprocess.cpp to see *how* qgit detects
an error has occurred and informs upstream.


So the bottom line is: no, you don't have to do anything special. The
returned 'false' value from run() call is for your use only, if
needed, you don't have to propagate upstream to let user be informed.

Hope this helps.

Marco


P.S: Why 'git-commit --amend -F' it's explicitely forbidden?

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]

On Sun, Jul 01, 2007 at 18:09:37 +0200, Marco Costalba wrote:
> [...]
> QGit::run() and QGit::runAsync()
> [...]
>
> A bool is the only flag returned, because error detect is done at
> lower level, in MyProcess::sendErrorMsg() in file myprocess.cpp that
> handles the low level of running an external process and is called by
> run().
>
> In case of an error MyProcess::sendErrorMsg() is called to inform the
> GUI (a popup dialog box) about the error and the stderr output
> received.

Thanks. That's the bit I didn't see in MyProcess. Ok, so I'll simply return
from the function when some of the run()s return false.

> [...]
> Hope this helps.

Thanks.

> P.S: Why 'git-commit --amend -F' it's explicitely forbidden?

The logic for preparing the commit message seems to be incompatible in
git-commit.sh for some reason and whoever wrote it fobode those flags
together instead of making it compatible. In any case, the error is reported
in git-commit.sh:291-296:

case "$log_given" in
tt*)
	die "Only one of -c/-C/-F/--amend can be used." ;;
*tm*|*mt*)
	die "Option -m cannot be combined with -c/-C/-F/--amend." ;;
esac

And in any case the git policy is, that frontends should not be using
porcelain commands. What I would really like to see is something between
git-commit and git-commit-tree, that would:
 - run pre-commit hook
 - write-tree
 - commit-tree
 - update-ref
 - rerere
 - run post-commit hook
But it does not exist and git-gui obviously does that manually, so I did it
manually as well. Now I have to test it.

The other thing that didn't work (or at least from reading the code it seems
it couldn't) is commiting merges, because when commiting merge, filename
arguments are forbidden on git-commit.

Commiting merges will require some special handling, because you have to at
least commit all files that were merged. And you can't re-create pristine
index with read-tree the way normal commit does it either.

Unfortunately I don't fully understand -- and remember the right commands for
working with it -- the multi-stage index.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Junio C Hamano]

Jan Hudec <bulb@ucw.cz> writes:

>> P.S: Why 'git-commit --amend -F' it's explicitely forbidden?

The reasoning goes like this (here, I am not particularly trying
to justify it, but am merely explaining the original reasoning
and intended use case as a historical background):

    To amend means to remove the tip commit, replace with a new
    one, possibly but not necessarily with a different tree from
    the removed one.

    Since you are "amending", the spirit of the commit you are
    going to create, in order to replace the old one, ought to
    be the same as the one being replaced.

    - You may be only adding a change that you forgot to add
      before making the previous commit (in which case your tree
      is slightly different, but what you are going to say in
      the commit log message is exactly the same), or

    - you may found a typo in the commit log message and trying
      to fix it (in which case your tree is identical but the
      commit log message would be slightly different).

    In either case, the resulting commit log message would be
    very similar to the existing one, so the tool helps you by
    letting re-use and re-edit the commit log message instead of
    forcing you to re-type it.

There is no room for -F, -c, nor -m to make sense for these use
cases, and giving them to "commit --amend" is most likely a user
error, and diagnozed as such, because "commit --amend" is an
end-user level Porcelain program.

If you are popping one commit and replacing with a totally
**unrelated** commit, that is not what --amend is about.  What
you are doing is "reset --soft HEAD^" followed by "add <something>"
followed by "commit".

At the mechanical level, you could argue that --amend is doing
the same thing.  After all, that reset/add/commit sequence is
exactly what is done by --amend internally.

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.

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 7/4/07, Junio C Hamano <gitster@pobox.com> wrote:
> Jan Hudec <bulb@ucw.cz> writes:
>
> >> P.S: Why 'git-commit --amend -F' it's explicitely forbidden?
>
> The reasoning goes like this (here, I am not particularly trying
> to justify it, but am merely explaining the original reasoning
> and intended use case as a historical background):
>

Thanks for your explanation, of course ;-) I think you will agree that...

>
> There is no room for -F, -c, nor -m to make sense for these use
> cases, and giving them to "commit --amend" is most likely a user
> error, and diagnozed as such, because "commit --amend" is an
> end-user level Porcelain program.
>

Why an 'end-user' should _erroneusly_ write '-F' option to 'git commit
--amend' ?

An error IMHO could occur if user *forgets* to write something, not if
he intentionally writes a very specific '-F' option, in that case I
would say user knows what it writes.

And speaking about errors one can always write 'git reset HEAD^' with
worst results.

Probably you agree it's very _artificial_ try to guess what is in the
head of the user and what is not, especially if this guess is made by
a tool.

So I would dare to say this could be a good occasion to remove that
illusory and obscure check.

> 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 always prefer qgit to use the highest level commands as possible because:

1- Less error prone
2- Easier to implement
3- More robust to API change
4- Less easy to break by changes in git.


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?

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.

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.

Marco

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 3523 bytes --]

On Wed, Jul 04, 2007 at 14:44:16 +0200, Marco Costalba wrote:
> On 7/4/07, Junio C Hamano <gitster@pobox.com> wrote:
>> 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 <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Junio C Hamano]

Jan Hudec <bulb@ucw.cz> writes:

> On Wed, Jul 04, 2007 at 14:44:16 +0200, Marco Costalba wrote:
>> On 7/4/07, Junio C Hamano <gitster@pobox.com> wrote:
>>> 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).

Judging from the existing users (am, commit, filter-branch, and
merge), I would say "write-tree then commit-tree then
update-ref" sequence could be made to a new built-in plumbing if
you really wanted to.  "mv next-index index" and calling of
hooks definitely do _not_ belong there, though, since the hook
that needs to be called are different, and the caller of such a
plumbing command is very well equipped to do them by themselves
anyway.

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 7/4/07, Jan Hudec <bulb@ucw.cz> wrote:
>
> > 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.
>

Well, changing internal API does not break compatibility, _modifying_
user commands behaviour yes and you need to release a new version for
this.

If for "change" you mean adding stuff then, yes, I agree with you, but
adding stuff is not a problem.

> > 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.
>

Yes you are right, but the file is redirected to process stdin by the call

proc->setStandardInputFile()

just below the comment you reported. I can assure you it works because
to read file names it is used "git diff-tree -r -C --stdin" without
problems.


Sorry for mt late reply, but I'm abroad this week and can access the
email only seldom.


Marco

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]

Hello,

On Mon, Jun 11, 2007 at 07:45:51 +0200, Marco Costalba wrote:
> On 6/11/07, Jan Hudec <bulb@ucw.cz> wrote:
>> I think I mostly understood it now. Thank you.
>
> Anyhow I think this could be useful to you:
>
> 	/*
> 	   getAllRefSha() returns the list of sha of a given
> 	   type, where type is a mask of Git::RefType flags
> 	   see src/git.h.
> 	   In this case we want the sha of the current branch
> 	*/
> 	QStringList revs = getAllRefSha(CUR_BRANCH);
> 	
> 	if (!revs.isEmpty()) {
>
> 		// all the sha info is stored in this QGit::Rev
> 		// class defined in src/common.h
> 		const Rev* r = revLookup(revs.first());
>
> 		// short log (title) is
> 		r->shortLog();
>
> 		// message body is
> 		r->longLog();
>
> 		// etc....
> 	}

I thought I should be using something more explicit. Like getRefSha("HEAD",
ANY_REF, false) -- only to find that it wouldn't work. That means that this
code would not work either. Why? Well, because HEAD does not have to be
a symbolic ref. If you check out anything else than branch (which you can),
HEAD will be set to the SHA1 directly and if you commit in such state (which
you also can), the HEAD will be different from anything in refs/.

Therefore I'll either have to always ask git via run("git-rev-parse HEAD",
head), add HEAD into the map, or store HEAD somewhere in the Git object.
Which do you think makes most sense?

(Note: Yes, I noted that getRefSha("HEAD", ANY_REF, true) should work, but of
course that is the run("git-rev-parse HEAD") case.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 7/5/07, Jan Hudec <bulb@ucw.cz> wrote:

>
> Therefore I'll either have to always ask git via run("git-rev-parse HEAD",
> head), add HEAD into the map, or store HEAD somewhere in the Git object.
> Which do you think makes most sense?
>

Asking git when you need it and keep HEAD value only for the minimum
time required to execute the commit command.

- HEAD is very 'volatile'

- commit is _not_ performance critical.

- commit, being a write operation, is instead bugs/misbehaviour
critical (a big point to use an high level "git-commit" BTW)

- asking git with getRefSha("HEAD", ANY_REF, true)  is very quick and
in any case much quicker then the whole commit dance.

- someone can always change the repo behind you, qgit is not the only
interface to git ;-) does exist also the command line.


Marco

Re: [Qgit RFC] commit --amend

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

On Fri, Jul 06, 2007 at 10:12:50 +0200, Marco Costalba wrote:
> On 7/5/07, Jan Hudec <bulb@ucw.cz> wrote:
>> Therefore I'll either have to always ask git via run("git-rev-parse HEAD",
>> head), add HEAD into the map, or store HEAD somewhere in the Git object.
>> Which do you think makes most sense?
>
> Asking git when you need it and keep HEAD value only for the minimum
> time required to execute the commit command.
>
> - HEAD is very 'volatile'
>
> - commit is _not_ performance critical.
>
> - commit, being a write operation, is instead bugs/misbehaviour
> critical (a big point to use an high level "git-commit" BTW)
>
> - asking git with getRefSha("HEAD", ANY_REF, true)  is very quick and
> in any case much quicker then the whole commit dance.

Yes. It is also much faster to write in code, but...

> - someone can always change the repo behind you, qgit is not the only
> interface to git ;-) does exist also the command line.

And the commit in qgit should better fail loudly if that happens, because the
list of files or something else might no longer make sense.

There is actually just one thing I need the head for -- passing it as 3rd
argument to git-update-ref. That should be done purely as safety measure --
if the value does not match, the command will fail. And for that safety
measure to be useful, I need value of the head at the time user openend the
commit dialog, NOT the time user clicked on commit button.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qgit RFC] commit --amend

[Marco Costalba]

On 7/8/07, Jan Hudec <bulb@ucw.cz> wrote:
> On Fri, Jul 06, 2007 at 10:12:50 +0200, Marco Costalba wrote:
> > On 7/5/07, Jan Hudec <bulb@ucw.cz> wrote:
> >> Therefore I'll either have to always ask git via run("git-rev-parse HEAD",
> >> head), add HEAD into the map, or store HEAD somewhere in the Git object.
> >> Which do you think makes most sense?
> >
> > Asking git when you need it and keep HEAD value only for the minimum
> > time required to execute the commit command.
> >
> > - HEAD is very 'volatile'
> >
> > - commit is _not_ performance critical.
> >
> > - commit, being a write operation, is instead bugs/misbehaviour
> > critical (a big point to use an high level "git-commit" BTW)
> >
> > - asking git with getRefSha("HEAD", ANY_REF, true)  is very quick and
> > in any case much quicker then the whole commit dance.
>
> Yes. It is also much faster to write in code, but...
>
> > - someone can always change the repo behind you, qgit is not the only
> > interface to git ;-) does exist also the command line.
>
> And the commit in qgit should better fail loudly if that happens, because the
> list of files or something else might no longer make sense.
>
> There is actually just one thing I need the head for -- passing it as 3rd
> argument to git-update-ref. That should be done purely as safety measure --
> if the value does not match, the command will fail. And for that safety
> measure to be useful, I need value of the head at the time user openend the
> commit dialog, NOT the time user clicked on commit button.
>

It it has "commit dialog" life span I would suggets to save in a
"commit dialog" object member data.

Marco