ENH: Add paperSize global setting #335
Conversation
…ort and print dialog paper size
ReText/window.py
Outdated
| printer.setPaperSize(page_size) | ||
| else: | ||
| QMessageBox.warning(self, '', | ||
| 'Unrecognized PaperSize setting "{0}".'.format( |
There was a problem hiding this comment.
Please make the string translatable (using self.tr), and also use percent formatting (%s) as that is what Qt Linguist understands.
ReText/window.py
Outdated
|
|
||
| def availablePageSizes(self): | ||
| """ List available page sizes. | ||
| """ |
There was a problem hiding this comment.
Maybe use QPrinterInfo::supportedPageSizes() instead of this function? You will need to pass printer info to getPageSizeByName, but that should not be a problem.
There was a problem hiding this comment.
Hmm. supportedPageSizes (possibly same as availablePageSizes ?) consistently returns empty array in my set-up. Perhaps this feature is not quite working on my system or there are some version differences. I'm using python-qt5-5.7-7.fc25.x86_64 on Fedora Linux 4.13.5-100.fc25.x86_64. Leaving as is for now as this seems compatible/working. Feel free to modify/
ReText/window.py
Outdated
| match = list(filter(lambda y:y.lower()==page_size_name.lower(), | ||
| self.availablePageSizes())) | ||
| if match: | ||
| page_size = getattr(QPagedPaintDevice, match[0]) |
There was a problem hiding this comment.
I would use a dict here. Something like:
sizes = {size.name().lower(): size for size in printerInfo.supportedPageSizes()}
requestedSize = sizes.get(pageSizeName.lower())There was a problem hiding this comment.
Hmm. supportedPageSizes (possibly same as availablePageSizes ?) consistently returns empty array in my set-up. Perhaps this feature is not quite working on my system or there are some version differences. I'm using python-qt5-5.7-7.fc25.x86_64 on Fedora Linux 4.13.5-100.fc25.x86_64. Leaving as is for now as this seems compatible/working. Feel free to modify/
ReText/__init__.py
Outdated
| @@ -98,6 +98,7 @@ def getBundledIcon(iconName): | |||
| 'useWebEngine': False, | |||
| 'useWebKit': False, | |||
| 'windowGeometry': QByteArray(), | |||
| 'paperSize': 'A4', | |||
There was a problem hiding this comment.
Why default explicitly to A4? Maybe leave the default empty and use Qt’s defaults?
…r unspecified, and made warning message translatable
ReText/window.py
Outdated
| printer.setPaperSize(pageSize) | ||
| else: | ||
| QMessageBox.warning(self, '', | ||
| self.tr('Unrecognized PaperSize setting "%s".' % |
There was a problem hiding this comment.
The closing ) is in wrong place, this won’t work. The translators need to translate the string with literal %s, not with the substituted value.
It should be self.tr('Unrecognized PaperSize setting "%s".') % globalSettings.paperSize.
ReText/window.py
Outdated
| self.availablePageSizes())) | ||
| if match: | ||
| pageSize = getattr(QPagedPaintDevice, match[0]) | ||
| return pageSize |
There was a problem hiding this comment.
Can you please still use a dictionary? Then this function will be just 3 lines instead of current 8.
lowerCaseNames = {pageSize.lower(): pageSize for pageSize in self.availablePageSizes()}
if pageSizeName.lower() in lowerCaseNames:
return getattr(QPagedPaintDevice, lowerCaseNames[pageSizeName])(And it will still return None if there is no match.)
ReText/window.py
Outdated
| """ Returns a validated PageSize instance | ||
| corresponding to the given name. Returns | ||
| None if the name is not a valid PageSize. | ||
| """ |
There was a problem hiding this comment.
Any reason why the docstring lines are so short?
There was a problem hiding this comment.
Fixed those so they wrap at longer length.
…ixed format of translatable warning message.
|
Yeah, I agree it's super weird that an empty array is being returned. I tried exactly this QPrinterInfo(printer).supportedPageSizes() both inside standardPrinter with the printer there as input and a couple of other places and in an ipython session, but strangely it's still returning [] (an empty array). I'm equally surprised. I can try to dig a bit further, but I'm guessing something strange about python-qt5 (PyQt5) or some other Fedora-specific issue. |
|
Congratulations with your first PR merged! There were minor problems with whitespace, but I fixed them myself in a follow-up commit. |
Adds paperSize global setting which sets both the default paper size for both the quick PDF export and the more interactive print dialog.
Reference issue #205