Merge lp:~srazi/qpdfview/small-patches into lp:qpdfview

[Razi Alavizadeh]

Hi Adam,

Some explanations:
Commit Message: "Show text related context-menu's actions only when selected rectangle contains text."
- I think showing "Copy text" when just image is available is confusing.

Commit Message: "Correctly determines word when searching through DjVu document."
- For example searching (book: "Megginson R. An introduction to Banach space theory") for "Banach" now can find "Hahn-Banach", "Banach's". (1229 results improved to 1382 results!)
Indeed, I found this issue when I implemented an extended search dock. It's completed but I didn't push it here because you said there are enough features for new release. (but if you want to review it then I'll push it tonight)

Best Regards,
Razi.

[Adam Reichold (adamreichold)]

Hello Razi,

I don't have time to review and merge right now, but the changes look correct and I will merge as soon as I can and definitely before the next release (since functional analysis without the Hahn-Banach theorem won't get you very far). Just a small thing, could you revert the usage of "QUrl::fromLocalFile" since that is not available on Qt 4.6 which we are currently still targeting. Thanks!

Best regards, Adam.

review: Approve

[Adam Reichold (adamreichold)]

Hello again,

sorry I was wrong, it was "QUrl::isLocalFile" that was introduced only in Qt 4.8. So I'll take that fix in as well.

Best regards, Adam.

[Razi Alavizadeh (srazi)]

Hello Adam,
> I don't have time to review and merge right now, but the changes look correct and I will merge as soon as I can and definitely before the next release (since functional analysis without the Hahn-Banach theorem won't get you very far). Just a small thing, could you revert the usage of "QUrl::fromLocalFile" since that is not available on Qt 4.6 which we are currently still targeting. Thanks!
It feels good to see a man of computer's world knows about mathematics's world.

Sorry again unrelated discussions:
1- I suggest that the HelpDialog should be non-modal, then user can read it and test application without closing it.
2- I want to save firstPage state to database (indeed I wrote the patch) but I don't understand the following part of preparePerFileSettings_v2()?

    if(Settings::instance()->mainWindow().restorePerFileSettings())
    {
        query.exec("DELETE FROM perfilesettings_v2 WHERE filePath IN (SELECT filePath FROM perfilesettings_v2 ORDER BY lastUsed DESC LIMIT -1 OFFSET 1000)");
    }
    else
    {
        query.exec("DELETE FROM perfilesettings_v2");
    }

Best Regards,
Razi.

[Adam Reichold (adamreichold)]

Hello Razi,

Am 19.09.2014 um 22:31 schrieb S. Razi Alavizadeh:
> Hello Adam,
>> I don't have time to review and merge right now, but the changes look correct and I will merge as soon as I can and definitely before the next release (since functional analysis without the Hahn-Banach theorem won't get you very far). Just a small thing, could you revert the usage of "QUrl::fromLocalFile" since that is not available on Qt 4.6 which we are currently still targeting. Thanks!
> It feels good to see a man of computer's world knows about mathematics's world.
>
> Sorry again unrelated discussions:

Again, the mailing list would be best. :-)

> 1- I suggest that the HelpDialog should be non-modal, then user can read it and test application without closing it.

I agree, the change sounds sensible. Can you write up the patch as I
will be travelling until Sunday evening (CEST).

> 2- I want to save firstPage state to database (indeed I wrote the patch) but I don't understand the following part of preparePerFileSettings_v2()?
>
> if(Settings::instance()->mainWindow().restorePerFileSettings())
> {
> query.exec("DELETE FROM perfilesettings_v2 WHERE filePath IN (SELECT filePath FROM perfilesettings_v2 ORDER BY lastUsed DESC LIMIT -1 OFFSET 1000)");
> }
> else
> {
> query.exec("DELETE FROM perfilesettings_v2");
> }

It is our probably not very elegant way to keep only the 1000 least
recently used entries of the per-file settings table (or completely
clear it if the user disabled the option). The number 1000 is pretty
arbitrary and was chosen conservatively and could propbably be improved.
More elegant SQL to keep only the last N entries would of course also be
welcome.

> Best Regards,
> Razi.

Best regards, Adam.

=== modified file 'sources/djvumodel.cpp'
--- sources/djvumodel.cpp	2014-03-30 09:43:56 +0000
+++ sources/djvumodel.cpp	2014-09-17 14:08:17 +0000
@@ -519,6 +519,8 @@
     QRectF rect;
     int index = 0;
 
+    const QTransform transform = QTransform::fromScale(72.0 / m_resolution, 72.0 / m_resolution);
+
     while(!words.isEmpty())
     {
         miniexp_t textExp = words.takeFirst();
@@ -531,7 +533,9 @@
             {
                 const QString word = QString::fromUtf8(miniexp_to_str(miniexp_nth(5, textExp)));
 
-                if(text.indexOf(word, index, matchCase ? Qt::CaseSensitive : Qt::CaseInsensitive) == index)
+                index = word.indexOf(text, index, (matchCase ? Qt::CaseSensitive : Qt::CaseInsensitive));
+
+                if (index != -1)
                 {
                     const int xmin = miniexp_to_int(miniexp_nth(1, textExp));
                     const int ymin = miniexp_to_int(miniexp_nth(2, textExp));
@@ -540,16 +544,11 @@
 
                     rect = rect.united(QRectF(xmin, m_size.height() - ymax, xmax - xmin, ymax - ymin));
 
-                    index += word.length();
-
-                    while(text.length() > index && text.at(index).isSpace())
-                    {
-                        ++index;
-                    }
-
-                    if(text.length() == index)
-                    {
-                        results.append(rect);
+                    index += text.length();
+
+                    if (!word.at(index).isLetter())
+                    {
+                        results.append(transform.mapRect(rect));
 
                         rect = QRectF();
                         index = 0;
@@ -573,13 +572,6 @@
 
     ddjvu_miniexp_release(m_parent->m_document, pageTextExp);
 
-    const QTransform transform = QTransform::fromScale(72.0 / m_resolution, 72.0 / m_resolution);
-
-    for(int index = 0; index < results.size(); ++index)
-    {
-        results[index] = transform.mapRect(results[index]);
-    }
-
     return results;
 }
 

=== modified file 'sources/mainwindow.cpp'
--- sources/mainwindow.cpp	2014-09-10 18:57:50 +0000
+++ sources/mainwindow.cpp	2014-09-17 14:08:17 +0000
@@ -910,7 +910,7 @@
 
 void MainWindow::on_openContainingFolder_triggered()
 {
-    QDesktopServices::openUrl(currentTab()->fileInfo().absolutePath());
+    QDesktopServices::openUrl(QUrl::fromLocalFile(currentTab()->fileInfo().absolutePath()));
 }
 
 void MainWindow::on_refresh_triggered()

=== modified file 'sources/pageitem.cpp'
--- sources/pageitem.cpp	2014-09-16 21:07:02 +0000
+++ sources/pageitem.cpp	2014-09-17 14:08:17 +0000
@@ -756,29 +756,27 @@
 {
     QMenu menu;
 
-    const QAction* copyTextAction = menu.addAction(tr("Copy &text"));
+    QAction* copyTextAction = menu.addAction(tr("Copy &text"));
     QAction* selectTextAction = menu.addAction(tr("&Select text"));
     const QAction* copyImageAction = menu.addAction(tr("Copy &image"));
     const QAction* saveImageToFileAction = menu.addAction(tr("Save image to &file..."));
 
-    selectTextAction->setVisible(QApplication::clipboard()->supportsSelection());
+    const QString text = m_page->text(m_transform.inverted().mapRect(m_rubberBand));
+
+    selectTextAction->setVisible(QApplication::clipboard()->supportsSelection() && !text.isEmpty());
+    copyTextAction->setVisible(!text.isEmpty());
 
     const QAction* action = menu.exec(screenPos);
 
     if(action == copyTextAction || action == selectTextAction)
     {
-        const QString text = m_page->text(m_transform.inverted().mapRect(m_rubberBand));
-
-        if(!text.isEmpty())
-        {
-            if(action == copyTextAction)
-            {
-                QApplication::clipboard()->setText(text);
-            }
-            else
-            {
-                QApplication::clipboard()->setText(text, QClipboard::Selection);
-            }
+        if(action == copyTextAction)
+        {
+            QApplication::clipboard()->setText(text);
+        }
+        else
+        {
+            QApplication::clipboard()->setText(text, QClipboard::Selection);
         }
     }
     else if(action == copyImageAction || action == saveImageToFileAction)