← Back to team overview

qpdfview team mailing list archive

Re: [Question #252130]: Feature request: Visual page numbers

 

Question #252130 on qpdfview changed:
https://answers.launchpad.net/qpdfview/+question/252130

S. Razi Alavizadeh posted a new comment:
Hi,

> first of all: Thanks for your contribution!
- My pleasure :-)

> While we could technically work using patches and pastebin, I would rather prefer to use Launchpad's built-in change submission tool ....
- OK after applying changes that you mentioned above I'll do a pull request. Indeed my problem is with slow and unstable bazaar's client for Windows. Maybe I should try to use and learn its commandline options and commands.

> Also, please try to separate unrelated changes from your commits...
- Certainly

> Now for the patches content, I dislike the fact that SpinBox and
MainWindow get a circular dependency by it (and the coupling seems to
get increased even more by the second patch.)
- What about easily add a subclass of SpinBox within MainWindow class? dislike it also?


> Also just adding an action to the document view context menu (as defined
by MainWindow::on_currentTab_customContextMenuRequested) might be a
simpler solution to setting the visual first page. (Copy-pasting Qt code
to extend widget functionality is a code smell IMHO.)
- I agree, I also didn't like the hack for adding action to context-menu :-D


> I am also not really happy with heuristic encoded in
DocumentView::hasLabelledPages. I would prefer a solution where we
always have a valid page label (even if it was provided by
DocumentView if the backend return an empty string)

- I'm not sure how to handle some special cases, for example:
On a document some pages have non-empty labels and some have [correctly] empty labels and in another document all pages have empty labels what Page::label() should returns? it seems we have to check all page if they have empty label or not and for big document this seems to me a slow task, I'm wrong?

Best Regards,
Razi

-- 
You received this question notification because you are a member of
qpdfview, which is an answer contact for qpdfview.