<div class="gmail_quote">On Sun, May 3, 2009 at 10:41 PM, Urs Wolfer <span dir="ltr"><<a href="mailto:uwolfer@kde.org">uwolfer@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Sunday 03 May 2009 22:20:28 Paweł Prażak wrote:<br>
> Some comments:<br>
> > 0001-Intendation-cleanup:<br>
> > Note that such a change makres merging with the original file from KHTML<br>
> > harder, but it's okay for me since it improves readabilty.<br>
><br>
> Hmm, I understand. Is it possible to patch KHTML code to change this (I<br>
> know it's just a detail but I wouldn't want to debug this code ;)<br>
<br>
</div>Yeah, that's a nice idea.<br>
<div class="im"><br>
> 0001-Zoom-actions-changes:<br>
> > * Are you sue the bool var m_zoomTextOnly is always initiated?<br>
><br>
> No, I'm not sure, but all of the settings are initialized the same way as I<br>
> see, so I assumed this is the working way. I don't know if this is a good,<br>
> idea but we can move all bit fields to union with an int and init the whole<br>
> union at once. But I'm not sure if it's necessary.<br>
<br>
</div>Okay then.<br>
<div class="im"><br>
> * "Zoom Normal" is not a very nice text IMHO. What about "Reset zoom"? Or<br>
><br>
> > what<br>
> > do you suggest?<br>
><br>
> "Reset Zoom" or "Zoom Reset" are good. I didn't gave much thought about the<br>
> name.<br>
> Maybe "Default Zoom", but reset is probably better. Is there a convention<br>
> established in KDE?<br>
<br>
</div>Not one I'm aware of... I still like "Reset Zoom" best. Probably ask a native<br>
english speaking person...</blockquote><div><br>Yes, it would be handy, but for the time being lets stick with "Reset Zoom" :)<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<div class="im">><br>
> * Just thought about it: is it the usual way that the user sets an option<br>
><br>
> > for<br>
> > one tab, and it gets saved as a global setting? Probably you could add a<br>
> > new<br>
> > option in the HTML options, and respect that option? Though the issue<br>
> > would be<br>
> > that KHTML would not follow this setting... So not really sure how to<br>
> > handle<br>
> > that. Probably your way is the only really working way. Any thoughts,<br>
> > anyone?<br>
><br>
> FF have global setting as well, so it's not that bad ;) If we make this<br>
> setting per tab how do we save this? per domain? I don't know how other<br>
> people but I only use option to set text only zoom when I'm testing/writing<br>
> the code, so it's not a hot feature anyway. ;)<br>
<br>
</div>Okay :) Let it as it is now.<br>
<div class="im">><br>
> 0002-Code-cleanup:<br>
> > This patch makes merging really harder. I also like nice code styling a<br>
> > lot,<br>
> > but I'm not sure if it is worth for loosing automatic merging...<br>
><br>
> Yes, now I understand why it looks how it looks. Now I see there is no<br>
> point in style cleanup without cleanup upstream.<br>
><br>
> 0003-Added-astyle-script-for-kdelibs-style.-Applyed-sctip.patch:<br>
> > Have you manually reviewed all changes? In some rare cases astyle styling<br>
> > is<br>
> > not the optimal thing.<br>
><br>
> No I didn't, but good to know that astyle isn't perfect. Anyway, I can try<br>
> to rewrite the script to force styling only on files that have no<br>
> imporeted/merged code (but I'm not sure if it makes sense).<br>
<br>
</div>The only files which need needs to be merged are in kdewebkit/settings.<br>
Either you or I need to review all astyle changes before committing.</blockquote><div><br>OK, so I'll blacklist kdewebkit/settings/* in the script and review all astyle changes later. (tomorrow probably)<br><br></div>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Btw, do you have a KDE SVN account? Otherwise I will commit your changes.</blockquote><div><br>Please commit the changes. <br><br>Best Regards,<br>Paweł<br></div></div><br>