[KPhotoAlbum] Patch for loading only one version of a file into the DB.

Andreas Neustifter andreas.neustifter at gmail.com
Mon Jan 24 18:02:44 GMT 2011


Hi All!

On 4 January 2011 09:58, Jan Kundrát <jkt at gentoo.org> wrote:
> On 01/04/11 09:43, Andreas Neustifter wrote:
>> I'm working on it, please be patient...
>
> Hi Andreas, I didn't meant to sound harsh, so I apologize. I just wanted
> to communicate to Jesper what my plans are. Please take your time :).

No worries, my intention was the same...

On 20 December 2010 00:56, Jan Kundrát <jkt at gentoo.org> wrote:
> On 12/12/10 21:38, Andreas Neustifter wrote:
>> I completely revised my patch and approached this in a whole different
>> manner. The discussion on Aleksi's topics with file version regexes
>> helped a lot with this.
>
> Hi Andreas, I got too angry fighting with git-am when trying to merge
> these patches (looks like [1] is not integrated in my version of Git).
> Could you please push them somewhere, so that I can simply pull? I thing
> there *must* be an easier way than saving patches from Thunderbird,
> applying them one-by-one and writing the commit message once again :(.

Sorry for that, lets discuss the points you raised and I then prepare
a final patch?

>> 0005-Cache-invariant-settings-so-they-are-only-loaded-onc.patch:
>> There are some strings and regexes that are constant throughout the
>> NewImageFinder::loadExtraFiles() loop, this patch pulls them out and
>> creates them only once. As with 0004 useful on its own.
>
> Obligatory question -- what's the overhead of a QRegExp() constructor?
> I'm asking because QSettings() is documented to have very low overhead,
> and it's really good to have optimizations backed up by measurements.

This is only in a small part intended as a runtime optimisation but
also as a readability optimisation because
NewImageFinder::loadExtraFile now is easier and simpler to read. In
generall I find it good to move loop invariant code out of the loop
for readability purpose.

>> 0006-Support-multiple-replacement-regexes-for-file-versio.patch:
>> This patch creates support for several regexes for the original file
>> version detection. Those regexes are separated with semicolons and
>> processes from left to right. The first regex that matches an existing
>> file is used as original file.
>
> If you allow entering multiple strings, please also make the GUI look
> intuitive. Separating regular expression by semicolon isn't intuitive. I
> can't think of anything better that a QTextEdit; I don't believe you'd
> like me to suggest using a QListWidget with some delegate for editing
> the regular expressions, although I'm sure Jesper would be glad to see
> it integrated with the KRegExpEditor :). If you find something
> reasonable, please go ahead and use that.

I thought a little about this and I have not found another (equally
easy to implement) solution. Is it possible to use it like this for
the time being and maybe design a better GUI afterwards?

>> This does not prevent the loading of multiple files but merely stacks
>> them on top of each other.
>
> What happens with other possible stacks? What about replacing the
> previous file with the new one if the regexp matches? I don't really
> like creating stacks in this situation.

I like the idea of just exchanging the pictures but I have always
found the actual image handling code of KPA quite dauting (sorry!). If
someone can point me to some sample code or give pointers to the
relevant functions I would like to make that "stacking vs. exchanging"
optional.

Thanks for the patience and the feedback!

Andi



More information about the Kphotoalbum mailing list