Unless someone disagrees i'll commit this tomorrow

This looks reasonable to me, but since Luigi is the one that did most of the last work i'll let him do the actual approval for the patch :)

Yep, this looks more like it :)

In D15355#342864, @yurchor wrote:

Just to be sure if I'm moving in an acceptable direction (resizing does not work for some reason yet). Screenshot:

I feel like having two classes of each is not the best of the ideas, since it seems the only difference in one you use 10 and in the other 100 in notifyValueChanged.

In D16090#343204, @gregormi wrote:

On both of the Amazon links, I only get Amazon login screens. It is not possible to review this information.

(In contrast, the Benevity link leads to information accessible to the public. The description about KDE sounds good there.)

even FSF does at https://www.fsf.org/about/ways-to-donate/

They have included a disclaimer: "As an independent small publisher, the FSF discourages people from shopping at Amazon (plus, Amazon supports DRM)". Can we add something like that as well? Because I am worried about linking to Amazon.

Have you asked sysadmin if by any chance they have a backup from that old site?

I still maintain this is wrong, and that there is no need for the second i18ncp since we don't support anywhere else different plurals for decimal numbers so we shouldn't here either, and as far as I know we don't support differnt plurals because all languages just use the same form for decimal numbers.

That's almost good, but the flag name should be more general, think like for example you would remove the manage bool and convert it to a flag too, so that if it the future someone needs to add a new flag, they can add it to that enum and don't need a new parameter.

*personally* I'd hate it to be the default, since clearing all the open files to get an empty one where i can write is something i do several times a day.

Oh you're doing this manually?

You fixed one and broke the other :D

In D16137#341984, @kadabash wrote:

I have to admit, I don't know how to implement a version with flags correctly.

Opening an image (Via File->Open) doesn't work (which i realize in the non ported version doesn't work either, but the error is different so you may want to have a look anyway.

With the new code i get 0,10 increments in Hue instead 0,03 but still far away from the 1,08 i get with the old version

In D16121#341752, @shlomif wrote:

Thanks! I see it here now - https://bugs.kde.org/show_bug.cgi?id=399123 - there was no link to the bug. The fix seems sane.

I'm unconvinced whether this is actually better, but it's somehow similar to what poppler does, so maybe it makes sense?

It's just that, create a folder named autotest and put the tests there :D

In D16121#341203, @shlomif wrote:

Can you give a deal number / screenshot / screencast where kpat moves a non-top card to the foundation automatically without this patch?

@broulik so you were getting the

./kioslave/kamera.cpp:796:        error(KIO::ERR_SLAVE_DEFINED, i18n("Could not read. Reason: %1",

often? Or is it somehow an inherited i18n call?

Adding new parameters is unfortunately Binary Incompatible so this can't go in this way, for it to work you'd need to add a new function with all the parameters and then call one from the other.

Add an autotests folder?

Pushed https://websvn.kde.org/?view=revision&revision=r1526304

If you can fix this correctly once in KConfigDialog instead of adding bad magic numbers everywhere, fixing it in KConfigDialog seems to be a no brainer (maybe with an flag option to not change behaviour for people that don't want that)

I'm not thrilled about all these dynamic_cast< UnarFlavour * > in the ifs.

ah my fault was based on the other revision

629 is a random number, random numbers are bad.

In D15972#337295, @ltoscano wrote:

("KDE Applications" is not meant to be used as tag for reviews, it's used to collect the projects; the last one who did significant changes to kfind is @broulik )

I haven't tested it, but code seems sensible, so if you have tested it and are confident it works, i'd say go for it

This doesn't make sense to me, this code should not be triggered in the case that is higlighted in this bug since AutoFit in that very long document should never be a case of "the scrollbar vs no scrollbar fight", so what we need to figure out is why the if is being triggered and protect against it, not fixing what is inside the if.

Have you double checked with the civicrm consultant (forgot his name), that this is OK? Last i remember there was some incompatibility between the sepa stuff and the new civicrm versions.

In D14631#331827, @emateli wrote:

@aacid Would be great if you'd point me towards making them installable. Only the dialog itself should be exported.

The patch doesn't seem to apply here, is this on top of master?

The ported dialog looks bad, both because the layout of the new widget is not where you would want it to be and because it has much less functionality than the native dialog.

I don't like qrc, are you saying that the only way kxmlgui works on MacOs is by not installing files?

So there's unar and lsar? What's the difference? Can you add some more text to the summary explaining it?

Cool looks defenitely easier to understand, please remove the unneded include and commit :)