Merge lp:~srazi/qpdfview/page-labels-fixes into lp:qpdfview

Proposed by Razi Alavizadeh
Status: Merged
Merged at revision: 1710
Proposed branch: lp:~srazi/qpdfview/page-labels-fixes
Merge into: lp:qpdfview
Diff against target: 47 lines (+17/-5)
1 file modified
sources/mainwindow.cpp (+17/-5)
To merge this branch: bzr merge lp:~srazi/qpdfview/page-labels-fixes
Reviewer Review Type Date Requested Status
Adam Reichold Approve
Review via email: mp+236940@code.launchpad.net

Description of the change

A small fix and an improvement related to page labels.

To post a comment you must log in.
Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello Razi,

thanks for the merge request. However, I am not sure I understand it correctly. Does it address the problem that the backend could always return English-locale page labels and we would prefer our default page label in this case? If so, wouldn't it be better to fix this once in line 395 of documentview.cpp? (I.e. check if "m_pages.at(number - 1)->label().toInt() != number" before setting "label"?)

Best regards, Adam.

review: Needs Information
Revision history for this message
Razi Alavizadeh (srazi) wrote :

Hello Adam,

Oh... yes, indeed the correct solution instead of first commit (and a part of second commit) is what you said.

Best Regards,
Razi.

Revision history for this message
Adam Reichold (adamreichold) wrote :

Ok, so I'll merge this and make the necessary adjustments...

review: Approve
Revision history for this message
Razi Alavizadeh (srazi) wrote :

OK, thanks.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'sources/mainwindow.cpp'
2--- sources/mainwindow.cpp 2014-09-21 20:29:25 +0000
3+++ sources/mainwindow.cpp 2014-10-02 17:16:12 +0000
4@@ -1596,7 +1596,18 @@
5 }
6
7 *ok = true;
8- return currentTab()->pageLabelFromNumber(val);
9+
10+ const QString& defaultPageLabel = currentTab()->defaultPageLabelFromNumber(val);
11+ const QString& pageLabel = currentTab()->pageLabelFromNumber(val);
12+
13+ if(defaultPageLabel != pageLabel && pageLabel.toInt() != val)
14+ {
15+ return pageLabel;
16+ }
17+ else
18+ {
19+ return defaultPageLabel;
20+ }
21 }
22
23 int MainWindow::currentPage_valueFromText(QString text, bool* ok) const
24@@ -2204,18 +2215,19 @@
25 if(m_tabWidget->currentIndex() != -1)
26 {
27 const int currentPage = currentTab()->currentPage();
28- const int numberOfPages = currentTab()->numberOfPages();
29+ const QString& numberOfPagesAsString = currentTab()->defaultPageLabelFromNumber(currentTab()->numberOfPages());
30
31 const QString& defaultPageLabel = currentTab()->defaultPageLabelFromNumber(currentPage);
32 const QString& pageLabel = currentTab()->pageLabelFromNumber(currentPage);
33
34- if((s_settings->mainWindow().usePageLabel() || currentTab()->hasFrontMatter()) && defaultPageLabel != pageLabel)
35+ if((s_settings->mainWindow().usePageLabel() || currentTab()->hasFrontMatter()) &&
36+ (defaultPageLabel != pageLabel && pageLabel.toInt() != currentPage))
37 {
38- suffix = QString(" (%1 / %2)").arg(currentPage).arg(numberOfPages);
39+ suffix = QString(" (%1 / %2)").arg(defaultPageLabel).arg(numberOfPagesAsString);
40 }
41 else
42 {
43- suffix = QString(" / %1").arg(numberOfPages);
44+ suffix = QString(" / %1").arg(numberOfPagesAsString);
45 }
46 }
47 else

Subscribers

People subscribed via source and target branches