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

List:       kde-panel-devel
Subject:    Re: Review Request 118933: PlasmaShell: Do not start krunner
From:       "Marco Martin" <notmart () gmail ! com>
Date:       2014-06-25 11:12:04
Message-ID: 20140625111204.7396.74907 () probe ! kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118933/#review60939
-----------------------------------------------------------


Just to recap why it was decided to make it start by the plasma shell and to actually \
remove the autostart file (part of Ivan's Gsoc last year): krunner being present or \
not is something that depends from the shell ux, and one shell can decide to not have \
krunner, and even stopping it if it's already running (active, for instance) (also, \
the only reason it actually made sense to split the system monitor for it, increasing \
overhead) relying again on autostart files it forces krunner on every shell, having a \
process that may not be used at all.


- Marco Martin


On June 25, 2014, 10:58 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118933/
> -----------------------------------------------------------
> 
> (Updated June 25, 2014, 10:58 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> PlasmaShell: Do not start krunner
> 
> Krunner is automatically started via an autostart file. We do not need plasma to
> start it as well. Without this patch, krunner comes into focus each time
> plasma is restarted since krunner is already running and executing it
> then gives it focus.
> 
> BUG: 336002
> 
> This patch depends on another patch in plasma-workspace/krunner which adds the \
> krunner.desktop file - 
> commit 1b570623b1e8df93f20940654e160b35570172ac
> Author: Vishesh Handa <me@vhanda.in>
> Date:   Wed Jun 25 11:38:49 2014 +0200
> 
> Add a KRunner autostart file
> 
> diff --git a/krunner/CMakeLists.txt b/krunner/CMakeLists.txt
> index 8e625b9..4197827 100644
> --- a/krunner/CMakeLists.txt
> +++ b/krunner/CMakeLists.txt
> @@ -35,6 +35,7 @@ target_link_libraries(krunner
> 
> install(TARGETS krunner ${INSTALL_TARGETS_DEFAULT_ARGS})
> install(FILES ${krunner_dbusAppXML} DESTINATION ${DBUS_INTERFACES_INSTALL_DIR} )
> +install(FILES krunner.desktop DESTINATION ${AUTOSTART_INSTALL_DIR})
> 
> set(CMAKECONFIG_INSTALL_DIR \
> "${CMAKECONFIG_INSTALL_PREFIX}/KRunnerAppDBusInterface") \
>                 ecm_configure_package_config_file(KRunnerAppDBusInterfaceConfig.cmake.in
>                 
> diff --git a/krunner/krunner.desktop b/krunner/krunner.desktop
> new file mode 100644
> index 0000000..2f1f6dc
> --- /dev/null
> +++ b/krunner/krunner.desktop
> @@ -0,0 +1,9 @@
> +[Desktop Entry]
> +Exec=krunner
> +Name=KRunner
> +OnlyShowIn=KDE;
> +Type=Application
> +X-DBUS-StartupType=Unique
> +X-DBUS-ServiceName=org.kde.krunner
> +X-KDE-StartupNotify=false
> +X-KDE-autostart-phase=0
> 
> 
> Diffs
> -----
> 
> desktoppackage/contents/loader.qml c1ac4a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118933/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
> 


[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="https://git.reviewboard.kde.org/r/118933/">https://git.reviewboard.kde.org/r/118933/</a>
  </td>
    </tr>
   </table>
   <br />





 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just to recap why it was \
decided to make it start by the plasma shell and to actually remove the autostart \
file (part of Ivan&#39;s Gsoc last year): krunner being present or not is something \
that depends from the shell ux, and one shell can decide to not have krunner, and \
even stopping it if it&#39;s already running (active, for instance) (also, the only \
reason it actually made sense to split the system monitor for it, increasing \
overhead) relying again on autostart files it forces krunner on every shell, having a \
process that may not be used at all. </pre>
 <br />









<p>- Marco Martin</p>


<br />
<p>On June 25th, 2014, 10:58 a.m. UTC, Vishesh Handa wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://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 Plasma.</div>
<div>By Vishesh Handa.</div>


<p style="color: grey;"><i>Updated June 25, 2014, 10:58 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>


<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;">    PlasmaShell: Do not start krunner

    Krunner is automatically started via an autostart file. We do not need plasma to
    start it as well. Without this patch, krunner comes into focus each time
    plasma is restarted since krunner is already running and executing it
    then gives it focus.

    BUG: 336002

This patch depends on another patch in plasma-workspace/krunner which adds the \
krunner.desktop file -

commit 1b570623b1e8df93f20940654e160b35570172ac
Author: Vishesh Handa &lt;me@vhanda.in&gt;
Date:   Wed Jun 25 11:38:49 2014 +0200

    Add a KRunner autostart file

diff --git a/krunner/CMakeLists.txt b/krunner/CMakeLists.txt
index 8e625b9..4197827 100644
--- a/krunner/CMakeLists.txt
+++ b/krunner/CMakeLists.txt
@@ -35,6 +35,7 @@ target_link_libraries(krunner
 
 install(TARGETS krunner ${INSTALL_TARGETS_DEFAULT_ARGS})
 install(FILES ${krunner_dbusAppXML} DESTINATION ${DBUS_INTERFACES_INSTALL_DIR} )
+install(FILES krunner.desktop DESTINATION ${AUTOSTART_INSTALL_DIR})
 
 set(CMAKECONFIG_INSTALL_DIR \
&quot;${CMAKECONFIG_INSTALL_PREFIX}/KRunnerAppDBusInterface&quot;)  \
                ecm_configure_package_config_file(KRunnerAppDBusInterfaceConfig.cmake.in
                
diff --git a/krunner/krunner.desktop b/krunner/krunner.desktop
new file mode 100644
index 0000000..2f1f6dc
--- /dev/null
+++ b/krunner/krunner.desktop
@@ -0,0 +1,9 @@
+[Desktop Entry]
+Exec=krunner
+Name=KRunner
+OnlyShowIn=KDE;
+Type=Application
+X-DBUS-StartupType=Unique
+X-DBUS-ServiceName=org.kde.krunner
+X-KDE-StartupNotify=false
+X-KDE-autostart-phase=0
</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>desktoppackage/contents/loader.qml <span style="color: \
grey">(c1ac4a4)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/118933/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