From kfm-devel Wed Feb 10 21:43:29 2016 From: Frank Reininghaus Date: Wed, 10 Feb 2016 21:43:29 +0000 To: kfm-devel Subject: Re: Review Request 126904: Avoid checking if the right-clicked folder is in Message-Id: <20160210214329.23891.32880 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kfm-devel&m=145514062004684 --===============4225374682658775932== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Jan. 27, 2016, 9:38 a.m., Thomas Lübking wrote: > > src/dolphincontextmenu.cpp, line 430 > > > > > > As this will likely feed the places panel as well, it needs to be dynamic? > > > > => Could you just operate on a global singleton instance (feeding the panel view as well as used for such queries) instead? > > Emmanuel Pescosta wrote: > The idea is good but the problem is, that the KItemListController takes over the ownership of the model. > So if we really want a singleton places model, we either have to change the controller implemenation or > we need a proxy object for the places model. I think that a singleton object (maybe in KIO) that can be used to manage the Places might make sense. This could make it possible to find out if a URL is in the Places without expensive Solid queries for the devices, and it would also make the code in Dolphin's PlacesItemModel and KFilePlacesModel easier. I have been thinking about this for quite some time, but have not managed to work on it yet. - Frank ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126904/#review91653 ----------------------------------------------------------- On Feb. 10, 2016, 9:38 p.m., Frank Reininghaus wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126904/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2016, 9:38 p.m.) > > > Review request for Dolphin and Thomas Lübking. > > > Repository: dolphin > > > Description > ------- > > This prevents that the setup of the PlacesItemModel queries Solid for the available devices, which can take some time. > > Places can be added multiple times now using the context menu, but this has always been possible with drag and drop anyway. > > See https://forum.kde.org/viewtopic.php?f=223&t=130617 for a dicsussion on this topic. > > > Diffs > ----- > > src/dolphincontextmenu.cpp af283cf > > Diff: https://git.reviewboard.kde.org/r/126904/diff/ > > > Testing > ------- > > The "Add to Places" entry is always enabled now in the context menu. I never observed a noticeable delay when right-clicking folders, but I am confident that this change removes the delay for the users who saw it. > > > Thanks, > > Frank Reininghaus > > --===============4225374682658775932== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126904/

On January 27th, 2016, 9:38 a.m. UTC, Thomas Lübking wrote:

src/dolphincontextmenu.cpp (Diff revision 1)
430 
    PlacesItemModel model;
 430 
    // Creating up a PlacesItemModel to find out if 'url' is one of the Places
 
As this will likely feed the places panel as well, it needs to be dynamic?


=> Could you just operate on a global singleton instance (feeding the panel view as well as used for such queries) instead?

On January 28th, 2016, 8:53 p.m. UTC, Emmanuel Pescosta wrote:

The idea is good but the problem is, that the KItemListController takes over the ownership of the model.
So if we really want a singleton places model, we either have to change the controller implemenation or
we need a proxy object for the places model.
I think that a singleton object (maybe in KIO) that can be used to manage the Places might make sense. This could make it possible to find out if a URL is in the Places without expensive Solid queries for the devices, and it would also make the code in Dolphin's PlacesItemModel and KFilePlacesModel easier. I have been thinking about this for quite some time, but have not managed to work on it yet.

- Frank


On February 10th, 2016, 9:38 p.m. UTC, Frank Reininghaus wrote:

Review request for Dolphin and Thomas Lübking.
By Frank Reininghaus.

Updated Feb. 10, 2016, 9:38 p.m.

Repository: dolphin

Description 

This prevents that the setup of the PlacesItemModel queries Solid for the available devices, which can take some time.


Places can be added multiple times now using the context menu, but this has always been possible with drag and drop anyway.


See https://forum.kde.org/viewtopic.php?f=223&t=130617 for a dicsussion on this topic.

Testing 

The "Add to Places" entry is always enabled now in the context menu. I never observed a noticeable delay when right-clicking folders, but I am confident that this change removes the delay for the users who saw it.

Diffs

  • src/dolphincontextmenu.cpp (af283cf)

View Diff
--===============4225374682658775932==--