Re: [PATCH QGit RFC] Fix "Save patch..." on a commit range

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Jun 16, 2009, 18:48 UTC
Message-ID: <200906162048.00849.markus.heidelberg@web.de>
In-Reply-To: <e5bfff550906152310m1dc92bf7vf73fdb8f925e2fb7@mail.gmail.com>

Marco Costalba, 16.06.2009:

Show 8 quoted lines

> On Tue, Jun 16, 2009 at 00:53, Markus
> Heidelberg<markus.heidelberg@web.de> wrote:
> >
> > Yes, it works. Thanks. Would it make sense and be possible to fix it
> > directly in ListView::getSelectedItems()?
> 
> Another reason is that is Git::formatPatch() that needs the rows to be
> ordered, so has a sense to do it there where is needed.

ListView::mouseMoveEvent() needs it as well, I just tested cherry-picking revisions per drag-and-drop and it was applied in the wrong order.

I have never used stgit, but isn't MainImpl::ActPush_activated() and MainImpl::ActPop_activated() affected in the same way?

Show 17 quoted lines

> Of corse if
> other places of QGit requires that the rows returned by
> ListView::getSelectedItems() should be ordered then we can move the
> ordering in ListView::getSelectedItems() as you suggest, or, even
> better, move the ordering in ListView::getSelectedItems() but from
> there call a new helper function Git::sortShaByIndex() that does the
> work and is defined in git.cpp because should use internal information
> like the index and call directly the sha database.
> 
> Yes, a possible cleanup / reformat job could be to:
> 
> - Add a new public helper function Git::sortShaByIndex() defined in git.cpp
> 
> - Call from ListView::getSelectedItems() when we get the list of sha
> user as select
> 
> - Document ListView::getSelectedItems() always returns a list of ordered sha's

I guess this is the right solution.

Show 9 quoted lines

> > BTW, a nice way to see the created git-format-patch command without
> > modifying the sources is to use a directory with spaces, which is not
> > handled correctly by qgit.
> >
> 
> 
> ????
> 
> I cannot test now, but this seems a bug.

I was not clear enough, but yes, it's a bug - with a usable side effect :)

Markus