ngraham added inline comments.
INLINE COMMENTS
> annotationwidgets.cpp:153
> + const QString customStampFile = QFileDialog::getOpenFileName(this,
> i18n("Select custom stamp symbol"),
> + QString(), i18n("*.ico *.png *.xpm *.svg *.svgz | Icon Files (*.ico
> *.png *.xpm *.svg *.svgz)") );
> + if ( !customStampFile.isEmpty() )
Are you sure the list of file formats should be localized?
> annotationwidgets.cpp:425
>
> - m_pixmapSelector = new PixmapPreviewSelector( widget );
> + m_pixmapSelector = new PixmapPreviewSelector( widget, true /*
> previewBelow */ );
> formlayout->addRow( i18n( "Stamp symbol:" ), m_pixmapSelector );
If this was an enum, you wouldn't need to add an inline comment explaining what
it does :)
> annotationwidgets.h:35
> public:
> - explicit PixmapPreviewSelector( QWidget * parent = nullptr );
> + explicit PixmapPreviewSelector( QWidget * parent = nullptr, bool
> previewBelow = false );
> virtual ~PixmapPreviewSelector();
Instead of adding a new bool argument, how about an enum instead for clarity so
the code is easy to read?
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D22064
To: simgunz, #okular
Cc: yurchor, ngraham, okular-devel, fbampaloukas, joaonetto, tfella, darcyshen,
aacid
