I updated the xml file used for saving current page and current page rotation to store the printing option and print range,
and added new parameters to the document class to store the required information about printing.
BUG: 98192
No Linters Available |
No Unit Test Coverage |
Buildable 4215 | |
Build 4233: arc lint + arc unit |
This new information is saved/remembered per-document and not globally, right?
Given it's in a document.cpp I would assume so, that's probably also where scroll position and what not is persisted
The new information was saved for all documents if Okular was not closed, closing the document only did not reset the printing options.
I reset the printing options when closing the document to avoid printing options from being stored for all opened documents before closing Okular.
If you're not going to save the data, why are you adding it to the docinfo xml?
If this is only for remembering the options while okular is open i don't even think it makes sense storing it into document at all and should just be stored in Part as something like a QMap<Document,PrinterOptions>
I am adding the data to the docinfo xml to remember those data when the file is reopened even after closing Okular the printing data would be remembered for the same file.
But you just said "I reset the printing options when closing the document".
So I'm confused.
Is the plan to remember options between different Okular executions or is it no?
I apologize for my confusing comments.
Yes, the plan is to remember printing options between different executions similar to remembering the document rotation.
The problem in my first patch was that if I have an opened document X and then close this document,
and opened another document Y the printing options of X will be used in Y, so I added this part
d->m_printRangeOption = QPrinter::AllPages; d->m_printFrom = -1; d->m_printTo = -1;
similar to what is done to the rotation:
d->m_rotation = Rotation0;
to restore the printing options to the default values.
This is what I meant by resetting and I apologize again for not being clear.
Ok, understood.
Personally i do not like this feature, the fact that i printed page 2 out of 400 of a document four months ago really means nothing regarding what i want to print now, in fact, most probably page 2 is exactly what i do not want to print.
Also it is not a behaviour the rest of KDE Applications has so it creates confusion amonsgt users why Okular seems to remember some settings but the rest of applications that seemingly have "the same printing dialog" behave different.
Besides that, i think the function names/comments could be improved
/** * Sets the printing range of the current document */ void setPrintRange(int from, int to);
doesn't really have to do anything with setting the printing rahge, all it does is save some value in an xml file.
So probably something like
/** * Sets the last page range used when printing the current document */ void setLastPrintRange(int from, int to);
makes more sense.
Please move as a Merge Request in https://invent.kde.org/kde/okular
We have pre-commit CI and lots of checks including clazy and clang-tidy there so it's a much better place for doing the review/approval/merge of the code.