Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run

2013-11-09 Thread David Stephen Hubner

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113779/#review43314
---

Ship it!


Looks good to me

- David Stephen Hubner


On Nov. 10, 2013, 12:20 a.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113779/
> ---
> 
> (Updated Nov. 10, 2013, 12:20 a.m.)
> 
> 
> Review request for kde-workspace and David Stephen Hubner.
> 
> 
> Bugs: 327382
> http://bugs.kde.org/show_bug.cgi?id=327382
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> ReadPipe() doesn't return 0 as expected in the case that the command is not 
> found. but the length of sh's output which is "command not found" in this 
> case. This is because popen() does not fail if the command is not found, 
> because it _can_ run "sh". (according to the man page, popen calls "/bin/sh 
> -c command")
> To fix this, ReadPipe() should check the return code of the call to pclose() 
> (see "man pclose"), and return 0 if this is not equal to 0.
> 
> 
> Diffs
> -
> 
>   kinfocenter/Modules/opengl/opengl.cpp 7df2b17 
> 
> Diff: http://git.reviewboard.kde.org/r/113779/diff/
> 
> 
> Testing
> ---
> 
> Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the 
> user's path.
> Without this patch, 3D Accelerator will be shown as "unknown" (because lspci 
> cannot be run, with this patch it works as intended.
> I also tried with lspci in /usr/bin/ (i.e. in the path) and completely 
> removed, worked as expected (correct information in the former case, 
> "unknown" in the latter).
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>



Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run

2013-11-09 Thread Wolfgang Bauer

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113779/
---

Review request for kde-workspace and David Stephen Hubner.


Bugs: 327382
http://bugs.kde.org/show_bug.cgi?id=327382


Repository: kde-workspace


Description
---

ReadPipe() doesn't return 0 as expected in the case that the command is not 
found. but the length of sh's output which is "command not found" in this case. 
This is because popen() does not fail if the command is not found, because it 
_can_ run "sh". (according to the man page, popen calls "/bin/sh -c command")
To fix this, ReadPipe() should check the return code of the call to pclose() 
(see "man pclose"), and return 0 if this is not equal to 0.


Diffs
-

  kinfocenter/Modules/opengl/opengl.cpp 7df2b17 

Diff: http://git.reviewboard.kde.org/r/113779/diff/


Testing
---

Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the 
user's path.
Without this patch, 3D Accelerator will be shown as "unknown" (because lspci 
cannot be run, with this patch it works as intended.
I also tried with lspci in /usr/bin/ (i.e. in the path) and completely removed, 
worked as expected (correct information in the former case, "unknown" in the 
latter).


Thanks,

Wolfgang Bauer



QDialog on stack+exec and dbus quit crash is no more

2013-11-09 Thread Albert Astals Cid
So in the EBN checks we are checking for places where we create QDailog on the 
stack and then call exec and suggesting to replace them with a QPointer 
QDialog, see 
http://ebn.kde.org/krazy/reports/kde-4.x/kdegames/kmines/index.html and 
http://blogs.kde.org/node/3919

So people are trying to do that improvements (we got patches for kmines to fix 
that).

But I don't think that's a problem anymore. I tried to crash it by doing what 
the blog says and it doesn't crash, and after having a look at the 
KApplication::quit code that calls QCoreApplication::quit() code I think that 
this has been fixed in Qt since QCoreApplication::quit doesn't "destroy" 
anything, just makes the event loops quit, so stuff is deleted properly.

Can anyone confirm my analysis?

And if so we should remove the check from krazy/ebn.

Cheers,
  Albert


Re: Can we make api.kde.org search better?

2013-11-09 Thread Albert Astals Cid
El Dissabte, 9 de novembre de 2013, a les 16:14:16, Allen Winter va escriure:
> On Saturday, November 09, 2013 01:27:12 PM Mark Gaiser wrote:
> > On Sat, Nov 9, 2013 at 12:42 PM, Martin Klapetek
> > 
> >  wrote:
> > > On Fri, Nov 8, 2013 at 3:47 PM, Allen Winter  wrote:
> > >> > Do you guys know if we can make the search better?
> > >> 
> > >> Try now.
> > >> Michael and I made some improvements.
> > > 
> > > Much much better, thank you!!
> > 
> > It looks like it finds more indeed! Awesome job!
> > Can it be tweaked even further to also find for example "kio::stat"?
> > It doesn't find that and i'm sure it exists :)
> 
> kio::stat is a function, not a class
> 
> currently the api.kde.org Search only looks for classes (and namespaces, I
> think)
> 
> but yes, we certainly should add the ability to search for functions too.
> 
> this is all very old stuff and I haven't looked at in depth for a long time.
> I didn't write it either.

Noone's blaming you ;-) Having the thing look into function names (and maybe 
categorizing the results in classes/functions) makes sense to me, if we can 
get it, better :-)

Cheers,
  Albert


Re: Can we make api.kde.org search better?

2013-11-09 Thread Allen Winter
On Saturday, November 09, 2013 01:27:12 PM Mark Gaiser wrote:
> On Sat, Nov 9, 2013 at 12:42 PM, Martin Klapetek
>  wrote:
> > On Fri, Nov 8, 2013 at 3:47 PM, Allen Winter  wrote:
> >>
> >> > Do you guys know if we can make the search better?
> >> >
> >> Try now.
> >> Michael and I made some improvements.
> >
> >
> > Much much better, thank you!!
> 
> It looks like it finds more indeed! Awesome job!
> Can it be tweaked even further to also find for example "kio::stat"?
> It doesn't find that and i'm sure it exists :)
> 
kio::stat is a function, not a class

currently the api.kde.org Search only looks for classes (and namespaces, I 
think)

but yes, we certainly should add the ability to search for functions too.

this is all very old stuff and I haven't looked at in depth for a long time.
I didn't write it either.





Re: Can we make api.kde.org search better?

2013-11-09 Thread Mark Gaiser
On Sat, Nov 9, 2013 at 12:42 PM, Martin Klapetek
 wrote:
> On Fri, Nov 8, 2013 at 3:47 PM, Allen Winter  wrote:
>>
>> > Do you guys know if we can make the search better?
>> >
>> Try now.
>> Michael and I made some improvements.
>
>
> Much much better, thank you!!

It looks like it finds more indeed! Awesome job!
Can it be tweaked even further to also find for example "kio::stat"?
It doesn't find that and i'm sure it exists :)

Cheers,
Mark


Re: Can we make api.kde.org search better?

2013-11-09 Thread Martin Klapetek
On Fri, Nov 8, 2013 at 3:47 PM, Allen Winter  wrote:

> > Do you guys know if we can make the search better?
> >
> Try now.
> Michael and I made some improvements.
>

Much much better, thank you!!

Cheers
-- 
Martin Klapetek | KDE Developer


Re: Can we make api.kde.org search better?

2013-11-09 Thread Albert Astals Cid
El Divendres, 8 de novembre de 2013, a les 09:47:47, Allen Winter va escriure:
> On Saturday, November 02, 2013 12:15:15 AM Albert Astals Cid wrote:
> > Hi, yesterday i was talking with someone about the desktop file class we
> > have in KDE, he tried to find its api by going to
> > 
> >   api.kde.org
> > 
> > and typing
> > 
> >   desktop
> > 
> > in the Search bar.
> > 
> > If you do that
> > 
> >   http://api.kde.org/index.php?miss=1&class=desktop
> > 
> > all you get is
> > 
> >No results found!
> > 
> > Do you guys know if we can make the search better?
> 
> Try now.
> Michael and I made some improvements.

Much better now :-)

Now if the KF5 made http://api.kde.org/frameworks/kdelibs-apidocs/ work that'd 
be awesome :D

Cheers,
  Albert