On Wed, Nov 7, 2012 at 9:45 PM, Pino Toscano <span dir="ltr"><<a href="mailto:pino@kde.org" target="_blank">pino@kde.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi,<br>
<br>
Alle mercoledì 7 novembre 2012, Aleix Pol ha scritto:<br>
<div>> On Wed, Nov 7, 2012 at 12:49 AM, Pino Toscano <<a href="mailto:pino@kde.org" target="_blank">pino@kde.org</a>> wrote:<br>
</div><div>> > Alle lunedì 5 novembre 2012, Aleix Pol ha scritto:<br>
> >> Some months ago I pushed this project quite intensively so that<br>
> >> right now we have a stable KCM for editing the GTK settings. This<br>
> >> is being used already in some distributions, so I think it should<br>
> >> be moved to extragear/base.<br>
> >><br>
> >> So here I open the process for kde:kde-gtk-config to move in<br>
> >> extragear. Any thoughts?<br>
> ><br>
> > Sure, here are mine:<br>
> ><br>
> > - please convert the two QDialog in .ui files to QWidget and put<br>
> > them in KDialog, so they have a simplier layout and they don't<br>
> > have to relayout (and retranslate) buttons<br>
><br>
> Is that really a big problem? I don't really like the jumping from<br>
> and to Q/K classes.<br>
<br>
</div>Where would be this "jumping from and to"?<br>
Using KDialog for dialogs makes them follow the KDE style and settings,<br>
and avoid retranslating standard texts.<br></blockquote><div>Ok, not using QDialog anymore...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - I've just simplified some ui strings, but there are still few<br>
> > more which use the qt rich text format, forcing a font face and<br>
> > size; please apply the font enlarging at runtime on the font<br>
> > active at that time<br>
><br>
> Yep..<br>
<br>
</div>Meaning it is going to be fixed? :)<br></blockquote><div>Yes, it should be fixed. I left one in gui.ui which is in a tooltip and I think it's ok like that.</div><div>It's only like this because we wanted to have links in it, if it's wrong, we can change that of course.and I'll </div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - QMessageBox -> KMessageBox<br>
><br>
> done<br>
<br>
</div>Still there...?<br></blockquote><div>Not anymore.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - related to the above, there's the job of ThreadAnalisysTheme::run<br>
> > and ThreadAnalisysThemeIcon::run, which currently spawn `tar` to<br>
> > extract the archive... what about porting them to KArchive (so you<br>
> > can potentially rely on all the formats supported by that)?<br>
<br>
</div>`tar` usage still there.<br></blockquote><div>not anymore!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - AppearanceGTK2::installedThemes and<br>
> > AppearanceGTK3::installedThemes need to be ported to<br>
> > KStandardDirs, yes<br>
><br>
> done<br>
<br>
</div> foreach(const QString& themesDir, KGlobal::dirs()->findDirs \<br>
("xdgdata-apps", "../themes")) {<br>
This is a bit ich-y, but at the moment I'm not sure how to do it<br>
properly.<br></blockquote><div>I have no idea either, if anybody has an idea, please don't hesitate to suggest it :).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - port AppearanceGTK3::defaultConfigFile to<br>
> > KStandardDirs::localxdgconfdir<br>
><br>
> done<br>
<br>
</div>I don't think localxdgconfdir returns an empty directory.<br>
<div><br>
> > - IconThemesModel really needs to use KStandardDirs instead of<br>
> > looking for XDG envvars and looking for paths on its own... or,<br>
> > even, just use KIconLoader/KIconTheme...<br>
><br>
> true, changed<br>
<br>
</div>Why not just use KIconLoader/KIconTheme to simplify even more?<br></blockquote><div>Because the code we have now just works and I don't have much experience with KIcon* classes.</div><div>I know it's a lousy explanation, but I think that it could become a waste of time...</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > - the differences between gtkproxies/preview.c and<br>
> > gtk3proxies/preview3.c seem minimal; is there really no way to have<br>
> > just one source with at most 1-2 «#if GTK_IS_VERSION(...)» blocks?<br>
> > also, doesn't glib/gtk offer some kind of functions to parse<br>
> > command line arguments (similar to getopt, KCmdLineArgs, etc)?<br>
><br>
> Probably, on the other hand, it just works now and I don't think it<br>
> would add much value to spend time improving that, as it's only for<br>
</div>[...]? :)<br></blockquote><div>If nobody has a big problem with having the preview codes duplicated, I'd prefer to leave it like that.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- there are icons in .ui files which are badly set, see:<br>
$ grep '<pixmap>' src/ui/*<br>
so better remove them from .ui files and set them via code.<br></blockquote><div>removed. It was already being set from the code.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- instead of have ThreadErase::run execute a KIO job synchronously, why<br>
not just rely on the async result of the job?<br></blockquote><div>Well it's a thread and the thread needs to wait for the job to finish for finishing the thread itself, so it would end up being synchronous anyway.</div>
<div><br></div><div>I ported the EraseThread to KJob anyway, because it made sense there.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- AbstractAppearance::hasProperty should be const, and just check<br>
QString::isEmpty?<br></blockquote><div>True, fixed</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><font color="#888888"><br>
--<br>
Pino Toscano<br>
</font></span></blockquote></div><br>