--===============2707004137563745536==
Content-Type: multipart/alternative; boundary=000e0cd5918683c63e04addb2ad2

--000e0cd5918683c63e04addb2ad2
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

I submitted my patch here ( https://bugs.kde.org/show_bug.cgi?id=3D235545  =
 ).
Let me know If it requires more changes or not.

-Jignesh

On Mon, Sep 26, 2011 at 3:32 PM, Aaron J. Seigo <aseigo@kde.org> wrote:

> On Sunday, September 25, 2011 18:39:10 jignesh kakadiya wrote:
> > http://paste.kde.org/126985/ here is the Ksnapshottimer.cpp
>
> please post patches, not full copies of the source files, so that we can
> see
> the changes. `git diff` makes this easy.
>
> some comments:
>
> * please keep indentation and whitespace identical in the file to other
> uses.
> e.g. it should be "if (" not "if(" and the opening curly brace on line 84
> looks like it is misalinged
>
> * an implementation of enterEvent would probably have been enough;
> mouseMoveEvent is only really better if one assumes that after moving the
> lable, it might still be under the mouse ... but in that case moving it
> back
> will have the same problem. enterEvent allows you to avoid the check for
> "is
> in the mouse inside the widget" and so keeps the code simpler
>
> * you don't need two booleans for "onLeftCorner" and "onRightCorner". jus=
t
> one
> is enough. even then, a simple check for "if (x() =3D=3D screenGeom.left(=
))"
> would
> probably be enough to know if it is in the left corner.
>
> * the move call to (0, 0) on line 85 will break for screens whose geometr=
y
> does not start at (0, 0). it should instead be move(screenGeom.topLeft())=
.
> same for the move call on line 78 which uses 0 for the y coordinate.
>
> > > /home/jiggy/kde/src/ksnapshot/snapshottimer.cpp:76:49: error: invalid
> use
> > > of incomplete type =E2=80=98struct QDesktopWidget=E2=80=99
>
> the file is missing: #include <QDesktopWidget>
>
> --
> Aaron J. Seigo
> humru othro a kohnu se
> GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43
>
> KDE core developer sponsored by Qt Development Frameworks
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>
>

--000e0cd5918683c63e04addb2ad2
Content-Type: text/html; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

I submitted my patch here ( <a href=3D"https://bugs.kde.org/show_bug.cgi?id=
=3D235545">https://bugs.kde.org/show_bug.cgi?id=3D235545</a>=A0=A0 ). Let m=
e know If it requires more changes or not.<br><br>-Jignesh<br><br><div clas=
s=3D"gmail_quote">
On Mon, Sep 26, 2011 at 3:32 PM, Aaron J. Seigo <span dir=3D"ltr">&lt;<a hr=
ef=3D"mailto:aseigo@kde.org">aseigo@kde.org</a>&gt;</span> wrote:<br><block=
quote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc=
 solid;padding-left:1ex;">
<div class=3D"im">On Sunday, September 25, 2011 18:39:10 jignesh kakadiya w=
rote:<br>
&gt; <a href=3D"http://paste.kde.org/126985/" target=3D"_blank">http://past=
e.kde.org/126985/</a> here is the Ksnapshottimer.cpp<br>
<br>
</div>please post patches, not full copies of the source files, so that we =
can see<br>
the changes. `git diff` makes this easy.<br>
<br>
some comments:<br>
<br>
* please keep indentation and whitespace identical in the file to other use=
s.<br>
e.g. it should be &quot;if (&quot; not &quot;if(&quot; and the opening curl=
y brace on line 84<br>
looks like it is misalinged<br>
<br>
* an implementation of enterEvent would probably have been enough;<br>
mouseMoveEvent is only really better if one assumes that after moving the<b=
r>
lable, it might still be under the mouse ... but in that case moving it bac=
k<br>
will have the same problem. enterEvent allows you to avoid the check for &q=
uot;is<br>
in the mouse inside the widget&quot; and so keeps the code simpler<br>
<br>
* you don&#39;t need two booleans for &quot;onLeftCorner&quot; and &quot;on=
RightCorner&quot;. just one<br>
is enough. even then, a simple check for &quot;if (x() =3D=3D screenGeom.le=
ft())&quot; would<br>
probably be enough to know if it is in the left corner.<br>
<br>
* the move call to (0, 0) on line 85 will break for screens whose geometry<=
br>
does not start at (0, 0). it should instead be move(screenGeom.topLeft()).<=
br>
same for the move call on line 78 which uses 0 for the y coordinate.<br>
<div class=3D"im"><br>
&gt; &gt; /home/jiggy/kde/src/ksnapshot/snapshottimer.cpp:76:49: error: inv=
alid use<br>
</div>&gt; &gt; of incomplete type =E2=80=98struct QDesktopWidget=E2=80=99<=
br>
<br>
the file is missing: #include &lt;QDesktopWidget&gt;<br>
<font color=3D"#888888"><br>
--<br>
</font><div><div></div><div class=3D"h5">Aaron J. Seigo<br>
humru othro a kohnu se<br>
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA =A0EE75 D6B7 2EB1 A7F1 DB43<br>
<br>
KDE core developer sponsored by Qt Development Frameworks<br>
</div></div><br>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href=3D"mailto:Plasma-devel@kde.org">Plasma-devel@kde.org</a><br>
<a href=3D"https://mail.kde.org/mailman/listinfo/plasma-devel" target=3D"_b=
lank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br>
<br></blockquote></div><br>

--000e0cd5918683c63e04addb2ad2--

--===============2707004137563745536==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

--===============2707004137563745536==--