> On Nov. 21, 2016, 8:34 a.m., David Faure wrote: > > filenamesearch/kded/filenamesearchmodule.cpp, line 84 > > <https://git.reviewboard.kde.org/r/129394/diff/1/?file=485431#file485431line84> > > > > Well, if dirUrl looks like > > "filenamesearch:?search=file&url=file:///path/to/file" then dirUrl.path() > > is empty, and this code is incorrect (it should use the query item "url", > > not the path). What am I missing? > > Anthony Fieroni wrote: > This is a big misunderstanding mainly by me. Emitted url should contains > query with new path ? > for (const QString &file : files) { > const QUrl url(file); > if (!url.isLocalFile()) { > continue; > } > const QString urlPath = url.path(); > for (const QUrl &dirUrl : m_searchUrls) { > QUrlQuery urlQuery(dirUrl); > QString str = urlQuery.queryItemValue(QStringLiteral("url")); > if (urlPath.startsWith(QUrl(str).path())) { > QUrl temp; > temp.setScheme(QStringLiteral("filenamesearch")); > urlQuery.removeQueryItem(QStringLiteral("url"); > urlQuery.addQueryItem(QStringLiteral('url"), url); > temp.setQuery(urlQuery); > fileList << temp; > } > } > } > > David Faure wrote: > Maybe, but I'm still in the dark about something. How can KDirLister cope > with listing such URLs? It wants a directory URL and files inside that > directory. Such a filenamesearch URL doesn't look like it's a file inside a > directory, in terms of paths. Ideally I would look into the code to > understand what is being done but I'm short on time. > > Does kio_filenamesearch really return items from listDir(), which have an > empty path too, just like the listed directory? I would assume this breaks > many things in KDirLister. > > Please clarify with the dolphin people (or whoever wrote the > filenamesearch KIO) about the URL structure, then it will be straightforward > to do the URL conversions in this code. > > Anthony Fieroni wrote: > I'm invited Emmanuel, who knows? > https://github.com/KDE/dolphin/blob/1710304e9ba926d2aec4226d00974b826f9bcbc0/src/kitemviews/kfileitemmodel.cpp#L123 > url("filenamesearch:?search=file&url=file:///path/to/file") in slot > https://github.com/KDE/dolphin/blob/1710304e9ba926d2aec4226d00974b826f9bcbc0/src/kitemviews/kfileitemmodel.cpp#L77 > comes results > > Emmanuel Pescosta wrote: > Peter wrote the filenamesearch slave. > > Filenamesearch URLs are only used to initiate a search. The URL contains > the search start-folder, the pattern used for matching and an optional 'check > contents' query item which can be yes or no. The filenamesearch ioslave uses > all this query items to perform the search. It recursively opens each folder > via KCoreDirLister (starting with the start-folder of the filenamesearch URL) > and iterates trough the item list of the directory, every matching item is > then listed via `listEntry` by using the UDSEntry of the matching item (see > 1). So kio_filenamesearch can only return items with an empty path if the > underlying ioslave (local, smb, ftp, ...) returns items with an empty path. > > [1] > https://github.com/KDE/kio-extras/blob/master/filenamesearch/kio_filenamesearch.cpp#L103 > > Anthony Fieroni wrote: > I and David, i guess, it's not clear how KDirlister handles url with > queries, i'm searching for this code, but i don't found it. > > David Faure wrote: > Ah but then KDirLister never sees that URL with a query in it, it's only > used for the app->slave communication (listDir). > (don't look for handling of queries in kdirlister, there isn't any. > kdirlister thinks of dirs with items in them, that's it). > > And to make things more confusing, I wasn't talking about the KDirLister > used by the slave itself (this is rather unusual and could be much more > optimized by using KIO::listDir directly; KDirLister is the backend for > views, it puts items into a cache and keeps watching them to mark them as > dirty so it knows to update the cache when going to that directory again... > all this is overkill for a kioslave who just wants to do a listDir). > > But that's a separate issue. For now let's forget about those > KDirListers. The one that I was talking about was the one on the Dolphin > side, that one that triggers the listDir in filenamesearch itself in the > first place. I think what's happening is that it lists filenamesearch: and in > return gets items with UDS_NAME=".zshrc" and > UDS_URL="file:///home/dfaure/.zshrc" (random example), so it stores it as if > it was filenamesearch:/.zshrc (it's a file inside the listed directory). > Which opens the question about what happens if two search results have the > same filename. > But if this analysis is correct, then the URL that the kded module has to > emit is filenamesearch:/.zshrc. > Enable DEBUG_CACHE in kcoredirlister.cpp to see the structure of the > items in there (I can help analyse the output). > > Anthony Fieroni wrote: > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) url= > QUrl("file:///home/toni/test") keep= false reload= false > kf5.kio.core.dirlister: Items in use: > kf5.kio.core.dirlister: "trash:/" URL: QUrl("trash:/") rootItem: > QUrl("trash:/") autoUpdates refcount: 1 complete: true "with 0 items." > kf5.kio.core.dirlister: "file:///home/toni" URL: > QUrl("file:///home/toni") rootItem: QUrl("file:///home/toni") autoUpdates > refcount: 1 complete: true "with 47 items." > kf5.kio.core.dirlister: Directory data: > kf5.kio.core.dirlister: "trash:/" 0 listers: "" > kf5.kio.core.dirlister: "trash:/" 1 holders: " 0xf8b520" > kf5.kio.core.dirlister: "file:///home/toni" 0 listers: "" > kf5.kio.core.dirlister: "file:///home/toni" 1 holders: " 0xd56250" > kf5.kio.core.dirlister: Jobs: > kf5.kio.core.dirlister: Items in cache: > kf5.kio.core.dirlister: lister: KFileItemModelDirLister(0xd56250) silent= > true > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) url= > QUrl("file:///home/toni") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) > kf5.kio.core.dirlister: Iterating over dirs (QUrl("file:///home/toni")) > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) _url: > QUrl("file:///home/toni") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) item moved into > cache: QUrl("file:///home/toni") > kf5.kio.core.dirlister: Listing directory: QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: Entry now being listed by > (KFileItemModelDirLister(0xd56250)) > kf5.kio.core.dirlister: new entries for QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: Adding item: > QUrl("file:///home/toni/test/lister") > kf5.kio.core.dirlister: in QUrl("file:///home/toni/test") item: > QUrl("file:///home/toni/test/lister") > kf5.kio.core.dirlister: finished listing QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) numJobs: 0 > kf5.kio.core.dirlister: Items in use: > kf5.kio.core.dirlister: "trash:/" URL: QUrl("trash:/") rootItem: > QUrl("trash:/") autoUpdates refcount: 1 complete: true "with 0 items." > kf5.kio.core.dirlister: "file:///home/toni/test" URL: > QUrl("file:///home/toni/test") rootItem: QUrl("file:///home/toni/test") > autoUpdates refcount: 1 complete: true "with 1 items." > kf5.kio.core.dirlister: Directory data: > kf5.kio.core.dirlister: "trash:/" 0 listers: "" > kf5.kio.core.dirlister: "trash:/" 1 holders: " 0xf8b520" > kf5.kio.core.dirlister: "file:///home/toni/test" 0 listers: "" > kf5.kio.core.dirlister: "file:///home/toni/test" 1 holders: " 0xd56250" > kf5.kio.core.dirlister: Jobs: > kf5.kio.core.dirlister: Items in cache: > kf5.kio.core.dirlister: "file:///home/toni" rootItem: > "file:///home/toni" with 47 items. > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) url= > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") keep= false > reload= false > kf5.kio.core.dirlister: Items in use: > kf5.kio.core.dirlister: "trash:/" URL: QUrl("trash:/") rootItem: > QUrl("trash:/") autoUpdates refcount: 1 complete: true "with 0 items." > kf5.kio.core.dirlister: "file:///home/toni/test" URL: > QUrl("file:///home/toni/test") rootItem: QUrl("file:///home/toni/test") > autoUpdates refcount: 1 complete: true "with 1 items." > kf5.kio.core.dirlister: Directory data: > kf5.kio.core.dirlister: "trash:/" 0 listers: "" > kf5.kio.core.dirlister: "trash:/" 1 holders: " 0xf8b520" > kf5.kio.core.dirlister: "file:///home/toni/test" 0 listers: "" > kf5.kio.core.dirlister: "file:///home/toni/test" 1 holders: " 0xd56250" > kf5.kio.core.dirlister: Jobs: > kf5.kio.core.dirlister: Items in cache: > kf5.kio.core.dirlister: "file:///home/toni" rootItem: > "file:///home/toni" with 47 items. > kf5.kio.core.dirlister: lister: KFileItemModelDirLister(0xd56250) silent= > true > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) url= > QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) > kf5.kio.core.dirlister: Iterating over dirs > (QUrl("file:///home/toni/test")) > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) _url: > QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) item moved into > cache: QUrl("file:///home/toni/test") > kf5.kio.core.dirlister: Listing directory: > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: Entry now being listed by > (KFileItemModelDirLister(0xd56250)) > kf5.kio.core.dirlister: new entries for > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: Adding item: > QUrl("file:///home/toni/test/lister") > kf5.kio.core.dirlister: in > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") item: > QUrl("file:///home/toni/test/lister") > kf5.kio.core.dirlister: finished listing > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: KFileItemModelDirLister(0xd56250) numJobs: 0 > kf5.kio.core.dirlister: Items in use: > kf5.kio.core.dirlister: "trash:/" URL: QUrl("trash:/") rootItem: > QUrl("trash:/") autoUpdates refcount: 1 complete: true "with 0 items." > kf5.kio.core.dirlister: > "filenamesearch:?search=lister&url=file:///home/toni/test" URL: > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") rootItem: > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") autoUpdates > refcount: 1 complete: true "with 1 items." > kf5.kio.core.dirlister: Directory data: > kf5.kio.core.dirlister: "trash:/" 0 listers: "" > kf5.kio.core.dirlister: "trash:/" 1 holders: " 0xf8b520" > kf5.kio.core.dirlister: > "filenamesearch:?search=lister&url=file:///home/toni/test" 0 listers: "" > kf5.kio.core.dirlister: > "filenamesearch:?search=lister&url=file:///home/toni/test" 1 holders: " > 0xd56250" > kf5.kio.core.dirlister: Jobs: > kf5.kio.core.dirlister: Items in cache: > kf5.kio.core.dirlister: "file:///home/toni" rootItem: > "file:///home/toni" with 47 items. > kf5.kio.core.dirlister: "file:///home/toni/test" rootItem: > "file:///home/toni/test" with 1 items. > kf5.kio.core.dirlister: ~KCoreDirLister KCoreDirLister(0xd56250) > kf5.kio.core.dirlister: lister: KCoreDirLister(0xd56250) silent= false > kf5.kio.core.dirlister: KCoreDirLister(0xd56250) url= > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: KCoreDirLister(0xd56250) > kf5.kio.core.dirlister: Iterating over dirs > (QUrl("filenamesearch:?search=lister&url=file:///home/toni/test")) > kf5.kio.core.dirlister: KCoreDirLister(0xd56250) _url: > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: KCoreDirLister(0xd56250) item moved into cache: > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") > kf5.kio.core.dirlister: ~KCoreDirLister KCoreDirLister(0xf8b520) > kf5.kio.core.dirlister: lister: KCoreDirLister(0xf8b520) silent= false > kf5.kio.core.dirlister: KCoreDirLister(0xf8b520) url= QUrl("trash:/") > kf5.kio.core.dirlister: KCoreDirLister(0xf8b520) > kf5.kio.core.dirlister: Iterating over dirs (QUrl("trash:/")) > kf5.kio.core.dirlister: KCoreDirLister(0xf8b520) _url: QUrl("trash:/") > kf5.kio.core.dirlister: KCoreDirLister(0xf8b520) item moved into cache: > QUrl("trash:/") > > David Faure wrote: > As I suspected. Look at this: > > kf5.kio.core.dirlister: Adding item: > QUrl("file:///home/toni/test/lister") > kf5.kio.core.dirlister: in > QUrl("filenamesearch:?search=lister&url=file:///home/toni/test") item: > QUrl("file:///home/toni/test/lister") > > KDirLister can't find such an item later on, since its parent dir isn't > file:///home/toni/test. This kind of code breaks then: > > KFileItem *KCoreDirListerCache::findByUrl(const KCoreDirLister > *lister, const QUrl &_u) const > { > QUrl url(_u); > url = url.adjusted(QUrl::StripTrailingSlash); > const QUrl parentDir = url.adjusted(QUrl::RemoveFilename | > QUrl::StripTrailingSlash); > DirItem *dirItem = dirItemForUrl(parentDir); > > I suppose KDirLister could keep yet another hash, mapping from every file > to its parent dir, but that sounds expensive. > > It would be much better if kio_filenamesearch would generate urls like > filenamesearch:/lister for the item called "lister" to be a child of > filenamesearch:/ > (I would add a '/' to the root url for this to work) > > Anthony Fieroni wrote: > If we have file:///home/toni/test/lister and > file:///home/toni/test/old/lister > filenamesearch:/lister filenamesearch:/old/lister ? > > David Faure wrote: > I suppose you don't want to see a subdir in the view, so no. > Rather filenamesearch:/lister_1 with a UDS_DISPLAY_NAME of "lister". Much > like kio_trash does in the same situation. In any case it gets confusing for > the user who has two items called "lister", in the search results, but I hope > dolphin displays the full path somewhere ;-) > > Anthony Fieroni wrote: > in filenamesearch.cpp > foreach (const KFileItem &item, items) { > if (itemValidator(item)) { > KIO::UDSEntry entry = item.entry(); > QString name = item.name(); > entry.insert(KIO::UDSEntry::UDS_NAME, > generateInternalName(name)); > entry.insert(KIO::UDSEntry::UDS_DISPLAY_NAME, name); > listEntry(entry); > } > But in kded module we does't have intenal name i.e. we must keep it in > sync ? > for (const QString &file : files) { > const QUrl url(file); > if (!url.isLocalFile()) { > continue; > } > const QString urlPath = url.path(); > for (const QUrl &dirUrl : m_searchUrls) { > QUrlQuery urlQuery(dirUrl); > QString str = urlQuery.queryItemValue(QStringLiteral("url")); > if (urlPath.startsWith(QUrl(str).path())) { > QUrl temp; > tmp.setScheme(QStringLiteral("filenamesearch")); > tmp.setPath("lister_1"); // <-------------- here how we > know it ? > fileList << tmp; > } > } > } > > Anthony Fieroni wrote: > David, i will ship fix for kded module in 16.08, 16.12 and master since > we are late for right solution.
I suppose the kioslave needs to tell the kded module (via DBus) about the mappings (file URLs -> search URLs), so that kded can store these (QHash) and use them. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129394/#review101007 ----------------------------------------------------------- On Nov. 14, 2016, 11:44 a.m., Anthony Fieroni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129394/ > ----------------------------------------------------------- > > Review request for KDE Frameworks, Anthony Fieroni, David Faure, and Emmanuel > Pescosta. > > > Repository: kio-extras > > > Description > ------- > > Bug is introduced in https://git.reviewboard.kde.org/r/129297/ > When is fixed new kio-extras realease is needed for 16.08 branch. > > > Diffs > ----- > > filenamesearch/kded/filenamesearchmodule.cpp 3f9f582 > > Diff: https://git.reviewboard.kde.org/r/129394/diff/ > > > Testing > ------- > > No big ram usage but still not works as expected. > 1. Perform search in Dolphin > 2. Delete one result item > 3. View must be update, but it's not > > > Thanks, > > Anthony Fieroni > >
