Re: [Qgit RFC] commit --amend

From: Marco Costalba <mcostalba@gmail.com>
Date: Jun 10, 2007, 22:10 UTC
Message-ID: <e5bfff550706101510x6d685944ja70c9d9dbb3668f6@mail.gmail.com>
In-Reply-To: <20070610150839.GG4084@efreet.light.src>

On 6/10/07, Jan Hudec <bulb@ucw.cz> wrote:

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

Good!

Show 8 quoted lines

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

Show 7 quoted lines

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

Show 9 quoted lines

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