Nikhil Marathe schrieb am Samstag 05 Juni 2010:
> Hi Mathias,
>
> That looks like a good patch to me. A few minor annoyances
>
> At lines 10, 125, 138, could you please put each argument on a
> separate line rather than extending the line so much.
done
> line 218, please add a comment explaining why you are removing the slash.
actually the line was already there, I just fixed the indentation. I guess
it's just to look better.
> And could you give a high level overview of what you mean by
> "UrlResolver::privilegedItem it wasn't needed anyway and I found a
> good
> workaround for that". That would make a good commit message to so that
> someone looking at the code could get a good idea of why it is the way
> it is.
well, in priviledgedItem KUrl::host() was called and it was the only place
where the KUrl was actually used. everywhere else, a QString was sufficient.
the workaround puts a dot at the end of the entered text and looks if there is
a match with the whole url. it works also if the dot is already entered, this
wasn't the case with the previous implementation.
with the attached patch, I improved the startup time with many bookmarks.
there is still an issue, that BookmarkProvider::slotBookmarksChanged(...) is
called to often, especially when editing a bookmark, every time a character
changes, this slot is called. it would be better to call it only after the
bookmark editor is closed, but I'm not familiar with the bookmark code. could
someone have a look at it?
additionally I added a few comments, removed tow functions, because they are
now not neccessary anymore and as a side effect I fixed a bookmark bug, where
the bookmakrs in the root group are not found until the bookmark button is
clicked.
regards,
mathias
diff --git a/src/bookmarks/bookmarksmanager.cpp b/src/bookmarks/bookmarksmanager.cpp
index 1341038..f3e543e 100644
--- a/src/bookmarks/bookmarksmanager.cpp
+++ b/src/bookmarks/bookmarksmanager.cpp
@@ -204,8 +204,6 @@ QAction * BookmarkMenu::actionForBookmark(const KBookmark &bookmark)
}
else
{
- UrlSearchItem urlSearchItem(UrlSearchItem::Bookmark, bookmark.url().prettyUrl() , bookmark.fullText(), QDateTime(), 1, bookmark.description(), QString());
- Application::bookmarkProvider()->completionObject()->addItem(urlSearchItem);
return new KBookmarkAction(bookmark, owner(), this);
}
}
@@ -301,6 +299,8 @@ BookmarkProvider::BookmarkProvider(QObject *parent)
// setup menu
m_owner = new BookmarkOwner(this);
connect(m_owner, SIGNAL(openUrl(const KUrl&, const Rekonq::OpenType &)), this, SIGNAL(openUrl(const KUrl&, const Rekonq::OpenType &)));
+
+ updateCompletionObject();
kDebug() << "Loading Bookmarks Manager... DONE!";
}
@@ -337,11 +337,14 @@ void BookmarkProvider::removeToolBar(KToolBar *toolbar)
void BookmarkProvider::slotBookmarksChanged(const QString &group, const QString &caller)
{
+ //TODO: check why this is called every time a character is changed in the bookmark editor
+ //this should only be called after the bookmark editor is closed
+ //check how konqueror does this
+ //alternative: call at least updateCompletionObject only after this isn't called for a certain time, maybe 1 second
+
Q_UNUSED(group)
Q_UNUSED(caller)
-
- m_completion->clear();
-
+
foreach(KToolBar *bookmarkToolBar, m_bookmarkToolBars)
{
if (bookmarkToolBar)
@@ -350,7 +353,8 @@ void BookmarkProvider::slotBookmarksChanged(const QString &group, const QString
fillBookmarkBar(bookmarkToolBar);
}
}
- //TODO: also change completion object
+
+ updateCompletionObject();
}
@@ -438,43 +442,37 @@ AwesomeUrlCompletion *BookmarkProvider::completionObject() const
}
-QString BookmarkProvider::titleForBookmarkUrl(QString url)
+void BookmarkProvider::updateCompletionObject(KBookmarkGroup* bookmarkGroup)
{
- QString title = "";
- KBookmarkGroup bookGroup = Application::bookmarkProvider()->rootGroup();
- if (bookGroup.isNull())
+ //with 10000 bookmarks, this takes about 4 seconds
+
+ KBookmarkGroup rootBookmarkGroup = rootGroup();
+ if(bookmarkGroup == 0)
{
- return title;
+ bookmarkGroup = &rootBookmarkGroup;
}
-
- KBookmark bookmark = bookGroup.first();
- while (!bookmark.isNull() && title.isEmpty())
+
+ if(*bookmarkGroup == rootBookmarkGroup)
{
- title = titleForBookmarkUrl(bookmark, url);
- bookmark = bookGroup.next(bookmark);
+ m_completion->clear();
}
-
- return title;
-}
-
-
-QString BookmarkProvider::titleForBookmarkUrl(const KBookmark &bookmark, QString url)
-{
- QString title = "";
- if (bookmark.isGroup())
+
+ for (KBookmark bookmark = bookmarkGroup->first(); !bookmark.isNull(); bookmark = bookmarkGroup->next(bookmark))
{
- KBookmarkGroup group = bookmark.toGroup();
- KBookmark bm = group.first();
- while (!bm.isNull() && title.isEmpty())
+ if(bookmark.isGroup())
{
- title = titleForBookmarkUrl(bm, url); // it is .bookfolder
- bm = group.next(bm);
+ updateCompletionObject(&bookmark.toGroup());
+ }
+ else if(!bookmark.isSeparator())
+ {
+ UrlSearchItem urlSearchItem(UrlSearchItem::Bookmark,
+ bookmark.url().prettyUrl(),
+ bookmark.fullText(),
+ QDateTime(),
+ 1, //TODO: check how to get the visit count
+ bookmark.description(),
+ QString());
+ m_completion->addItem(urlSearchItem);
}
}
- else if (!bookmark.isSeparator() && bookmark.url() == url)
- {
- title = bookmark.fullText();
- }
-
- return title;
}
diff --git a/src/bookmarks/bookmarksmanager.h b/src/bookmarks/bookmarksmanager.h
index dfebebd..d33f79e 100644
--- a/src/bookmarks/bookmarksmanager.h
+++ b/src/bookmarks/bookmarksmanager.h
@@ -235,8 +235,6 @@ public:
*/
AwesomeUrlCompletion *completionObject() const;
- QString titleForBookmarkUrl(QString url);
-
signals:
/**
* @short This signal is emitted when an url has to be loaded
@@ -266,7 +264,7 @@ public slots:
private:
void fillBookmarkBar(KToolBar *toolBar);
- QString titleForBookmarkUrl(const KBookmark &bookmark, QString url);
+ void updateCompletionObject(KBookmarkGroup *bookmarkGroup = 0);
KBookmarkManager *m_manager;
BookmarkOwner *m_owner;
diff --git a/src/history/historymanager.cpp b/src/history/historymanager.cpp
index d531c18..97e9489 100644
--- a/src/history/historymanager.cpp
+++ b/src/history/historymanager.cpp
@@ -133,7 +133,11 @@ void HistoryManager::addHistoryEntry(const QString &url)
// Add item to completion object
QString _url(url);
_url.remove(QRegExp("^http://|/$"));
- UrlSearchItem urlSearchItem(UrlSearchItem::History, _url, QString(), item.dateTime, 1, QString(), QString());
+ UrlSearchItem urlSearchItem(UrlSearchItem::History,
+ _url,
+ QString(),
+ item.dateTime,
+ 1);
m_completion->addItem(urlSearchItem);
}
@@ -267,7 +271,10 @@ void HistoryManager::removeHistoryEntry(const KUrl &url, const QString &title)
}
// Remove item from completion object
- UrlSearchItem urlSearchItem(UrlSearchItem::History, item.url, item.title, item.dateTime, 0, QString(), QString());
+ UrlSearchItem urlSearchItem(UrlSearchItem::History,
+ item.url,
+ item.title,
+ item.dateTime);
m_completion->removeItem(urlSearchItem);
}
@@ -373,7 +380,18 @@ void HistoryManager::load()
// Add item to completion object
//QString _url = item.url;
//_url.remove(QRegExp("^http://|/$"));
- UrlSearchItem urlSearchItem(UrlSearchItem::History, item.url, item.title, item.dateTime, 1, QString(), QString());
+ UrlSearchItem urlSearchItem(UrlSearchItem::History,
+ item.url,
+ item.title,
+ item.dateTime,
+ 1);
+ //this might increase startup time if there are many history items,
+ //because addItem(...) checks for double items
+ //with about 15000 history items and about 7000 after the double check,
+ //this increases the startup time by about 1.5 seconds
+ //10000 bookmark items with no doubel entries increase the startup time by about 4 seconds, btw
+ //TODO: check if it is possible to change the history file data format,
+ //so that there can't be any double entry
m_completion->addItem(urlSearchItem);
}
if (needToSort)
diff --git a/src/urlbar/urlresolver.cpp b/src/urlbar/urlresolver.cpp
index 7131766..ee0bad9 100644
--- a/src/urlbar/urlresolver.cpp
+++ b/src/urlbar/urlresolver.cpp
@@ -100,9 +100,6 @@ UrlSearchList UrlResolver::orderedSearchItems()
// There are 8 remaining: if bookmarkResults + historyResults <= 8, catch all, else
// catch first 4 results from the two resulting lists :)
- QTime myTime;
- myTime.start();
-
UrlSearchList list;
if( _typedString == QL1S("about:") )
@@ -261,7 +258,6 @@ UrlSearchList UrlResolver::orderedSearchItems()
}
list = list + historyList + commonList + bookmarksList;
- qWarning() << "orderedSearchItems leave: " << " elapsed: " << myTime.elapsed();
return list;
}
@@ -331,6 +327,7 @@ UrlSearchItem UrlResolver::privilegedItem(UrlSearchList* list)
return item;
}
}
+
return UrlSearchItem();
}
@@ -351,27 +348,22 @@ AwesomeUrlCompletion::~AwesomeUrlCompletion()
void AwesomeUrlCompletion::addItem(const UrlSearchItem& itemToAdd)
{
- bool match = false;
- QTime myTime;
- myTime.start();
- for(int i = 0; i < m_items.count(); i++)
+ //check if item is already in list; the items are equal if the url and the title are equal
+ int nIndex = m_items.indexOf(itemToAdd);
+ if(nIndex != -1)
{
- //check if item is already in list; the items are equal if the url and the title are equal
- if(m_items[i] == itemToAdd)
+ //TODO: check what to do if comment or bookmarkPath are different
+ if(m_items[nIndex] < itemToAdd)
{
- match = true;
- //TODO: check what to do if comment or bookmarkPath are different
- if(m_items[i] < itemToAdd)
- {
- m_items[i].visitDateTime = itemToAdd.visitDateTime;
- }
- m_items[i].visitCount += itemToAdd.visitCount;
+ m_items[nIndex].visitDateTime = itemToAdd.visitDateTime;
}
+ m_items[nIndex].visitCount += itemToAdd.visitCount;
}
- if(!match)
+ else
{
m_items.append(itemToAdd);
}
+
m_resetCompletion = true;
}
@@ -450,6 +442,7 @@ UrlSearchList AwesomeUrlCompletion::substringCompletion(const QString& completio
tempList.append(i);
}
}
+
m_lastCompletionString = completionString;
m_filteredItems = tempList;
return m_filteredItems;
diff --git a/src/urlbar/urlresolver.h b/src/urlbar/urlresolver.h
index 8322814..b0bc8bb 100644
--- a/src/urlbar/urlresolver.h
+++ b/src/urlbar/urlresolver.h
@@ -81,21 +81,21 @@ public:
bookmarkPath(QString())
{};
- UrlSearchItem(const int &_type,
- const QString &_url,
- const QString &_title = QString(),
- const QDateTime &visitDateTime = QDateTime(),
- const int &visitCount = 0,
- const QString &description = QString(),
- const QString &bookmarkPath = QString()
+ UrlSearchItem(const int &_type,
+ const QString &_url,
+ const QString &_title = QString(),
+ const QDateTime &_visitDateTime = QDateTime(),
+ const int &_visitCount = 0,
+ const QString &_description = QString(),
+ const QString &_bookmarkPath = QString()
)
: type(_type),
url(_url),
title(_title),
- visitDateTime(visitDateTime),
- visitCount(visitCount),
- description(description),
- bookmarkPath(bookmarkPath)
+ visitDateTime(_visitDateTime),
+ visitCount(_visitCount),
+ description(_description),
+ bookmarkPath(_bookmarkPath)
{};
inline bool operator==(const UrlSearchItem &i) const
_______________________________________________
rekonq mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/rekonq
