[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 110670: fixes for qml import paths order
From:       "Commit Hook" <null () kde ! org>
Date:       2013-05-28 16:29:23
Message-ID: 20130528162923.14331.48682 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


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

(Updated May 28, 2013, 4:29 p.m.)


Status
------

This change has been marked as submitted.


Review request for kde-workspace, kwin and Plasma.


Description
-------

QDeclarativeImportDatabase::addImportPath() prepends paths to the list, so paths must \
be added in reverse order (lowest preference -> highest preference).

To further complicate things, only paths not already in the list are added - \
hopefully this behaviour can be corrected in qt at some point. Nevertheless, these \
fixes should give the desired results with or without the further qt fix.


NB: kwin/kcmkwin/kwintabbox/layoutpreview.cpp is also affected by this issue but is \
not yet included in this RR, as it needs extra fixes and proper testing.

NB #2: there may be other files affected outside kde-workspace, but I haven't \
checked.


Branch for this RR can be found at clones/kde-workspace/oliverhenshaw/kde-workspace \
at review/qml-imports-v1 which comprises:

[1/3] Drop unneeded duplicate addImportPath

Let KDeclarative::setupBindings() add the import paths: it too takes
paths from KGlobal::dirs()->findDirs("module", "imports"); it adds paths
in the correct (reverse) order.

[2/3] Replace foreach with java-style iterator

In preparation for reversing the loop.

[3/3] add qml import paths in correct order

addImportPath prepends the path to importPathList so we must add our
paths in reverse order.

Based on the fix for kdeclarative.cpp in kdelibs
400b9f2e9d10386bb175b6123fe0cdaafeaffe61


Diffs
-----

  ksmserver/screenlocker/greeter/greeterapp.cpp \
b70c9d6c005aa66d2f85a42ef4f1dcb04ea44667   ksmserver/shutdowndlg.cpp \
247c8777f060a614436b4f757ba5d588035f3bf5   kwin/clients/aurorae/src/aurorae.cpp \
f7771c4cbee34194dcc8987a789712ab41898ec0   \
kwin/kcmkwin/kwindecoration/kwindecoration.cpp \
591a913c4032885474040e93e75d5f6f91bb9535   kwin/scripting/scripting.cpp \
13a77bc346b09f4b99f782370e8e0873332fad3d   kwin/tabbox/declarative.cpp \
f759512c76a2b589323614b412844a0ce8e4dd54   plasma/screensaver/shell/savercorona.cpp \
9cd207ffc8039cd1e41fdbbf9a4095426824f694 

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


Testing
-------

Compiles and runs. Checked that the resulting importPathList is correct for \
ksmserver/screenlocker/greeter/greeterapp.cpp and for \
kwin/clients/aurorae/src/aurorae.cpp


Thanks,

Oliver Henshaw


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/110670/">http://git.reviewboard.kde.org/r/110670/</a>
  </td>
    </tr>
   </table>
   <br />



<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray \
solid;">  <tr>
  <td>
   <h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been \
marked as submitted.</h1>  </td>
 </tr>
</table>
<br />


<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for kde-workspace, kwin and Plasma.</div>
<div>By Oliver Henshaw.</div>


<p style="color: grey;"><i>Updated May 28, 2013, 4:29 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">QDeclarativeImportDatabase::addImportPath() prepends paths to the list, \
so paths must be added in reverse order (lowest preference -&gt; highest preference).

To further complicate things, only paths not already in the list are added - \
hopefully this behaviour can be corrected in qt at some point. Nevertheless, these \
fixes should give the desired results with or without the further qt fix.


NB: kwin/kcmkwin/kwintabbox/layoutpreview.cpp is also affected by this issue but is \
not yet included in this RR, as it needs extra fixes and proper testing.

NB #2: there may be other files affected outside kde-workspace, but I haven&#39;t \
checked.


Branch for this RR can be found at clones/kde-workspace/oliverhenshaw/kde-workspace \
at review/qml-imports-v1 which comprises:

[1/3] Drop unneeded duplicate addImportPath

Let KDeclarative::setupBindings() add the import paths: it too takes
paths from KGlobal::dirs()-&gt;findDirs(&quot;module&quot;, &quot;imports&quot;); it \
adds paths in the correct (reverse) order.

[2/3] Replace foreach with java-style iterator

In preparation for reversing the loop.

[3/3] add qml import paths in correct order

addImportPath prepends the path to importPathList so we must add our
paths in reverse order.

Based on the fix for kdeclarative.cpp in kdelibs
400b9f2e9d10386bb175b6123fe0cdaafeaffe61</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Compiles and runs. Checked that the resulting importPathList is correct \
for ksmserver/screenlocker/greeter/greeterapp.cpp and for \
kwin/clients/aurorae/src/aurorae.cpp</pre>  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>ksmserver/screenlocker/greeter/greeterapp.cpp <span style="color: \
grey">(b70c9d6c005aa66d2f85a42ef4f1dcb04ea44667)</span></li>

 <li>ksmserver/shutdowndlg.cpp <span style="color: \
grey">(247c8777f060a614436b4f757ba5d588035f3bf5)</span></li>

 <li>kwin/clients/aurorae/src/aurorae.cpp <span style="color: \
grey">(f7771c4cbee34194dcc8987a789712ab41898ec0)</span></li>

 <li>kwin/kcmkwin/kwindecoration/kwindecoration.cpp <span style="color: \
grey">(591a913c4032885474040e93e75d5f6f91bb9535)</span></li>

 <li>kwin/scripting/scripting.cpp <span style="color: \
grey">(13a77bc346b09f4b99f782370e8e0873332fad3d)</span></li>

 <li>kwin/tabbox/declarative.cpp <span style="color: \
grey">(f759512c76a2b589323614b412844a0ce8e4dd54)</span></li>

 <li>plasma/screensaver/shell/savercorona.cpp <span style="color: \
grey">(9cd207ffc8039cd1e41fdbbf9a4095426824f694)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/110670/diff/" style="margin-left: \
3em;">View Diff</a></p>







  </td>
 </tr>
</table>




  </div>
 </body>
</html>



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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic