Hi,
Alle sabato 28 maggio 2011, Christopher Reichert ha scritto:
> +#include
you don't need this
> +m_model = new QFileSystemModel( this );
please use a KDirModel
> +m_docdataDir =
> KStandardDirs().localkdedir().append("share/apps/okular/docdata"); +
this is doubly wrong!
please make use of the KStandardDirs of the okular kpart (ok, you cannot
access to it right now, so for now use the one in KGlobal), and use
KStandardDirs::saveLocation() for the apps path (IIRC it should be used
somewhere in part.cpp or document.cpp)
> +// Hide file size and type
> +m_dlg->MetaFileList->hideColumn( 1 );
> +m_dlg->MetaFileList->hideColumn( 2 );
you have a proxy model, so make that proxy do the column hiding job (not
sure it is needed that way with KDirModel)
> +m_dlg->MetaFileList->setColumnWidth( 0, 350);
why this hardcoded value?
> +
> +//m_dlg->MetaFileList->header()->setResizeMode(
> QHeaderView::Interactive );
> +//m_dlg->MetaFileList->header()->resize(10,20);
no need for commented code in new code
> +KGuiItem yesButton( i18n( "Yes" ), QString(),
> + i18n( "This is a tooltip" ),
> + i18n( "This is a WhatsThis help text." ) );
unused?
> +if ( KMessageBox::questionYesNo( 0, i18n( "Are you sure you want
> to do this?\n\n"
missing a parent widget ("this" can be enough) for the message box
> + "Warning: deleting
> meta data will result\n"
> +"in the loss of
> bookmarks and annotations." ),
there are no bookmarks in .xml files
> + i18n( "Warning " ),
trailing space in i18n() string text, and a bit too generic
> + KStandardGuiItem::yes(),
put a custom gui item with the "delete" text (or dig whether
KStandardGuiItem has one)
> + i18n( "Do ask me again" )) ==
> KMessageBox::Yes )
this would be a dangerous operation so
- add the dangerous operation to the questionYesNo() call
- do not add the "do not ask me" string
> +if ( KMessageBox::questionYesNo( 0, i18n( "Are you sure you want
> to do this?\n\n"
> + "Warning: deleting
> meta data will result\n"
> + "in the loss of
> bookmarks and annotations." ),
> + i18n( "Warning " ),
> + KStandardGuiItem::yes(),
> + KStandardGuiItem::no(),
> + i18n( "Do ask me again" )) ==
> KMessageBox::Yes )
same notes as above
> +QDirIterator it(m_docdataDir, QDir::Files |
> QDir::NoDotAndDotDot );
> +QDir dir(m_docdataDir);
> +while ( it.hasNext() )
> +{
> +dir.remove( it.next() );
> +}
this must remove only the xml files, not any arbitrary file which could
be there
> +if ( role == Qt::DisplayRole )
switch for the implemented roles
> +QRegExp rx("*.xml");
> +rx.setPatternSyntax(QRegExp::Wildcard);
> +
> +if ( rx.exactMatch( modifiedData.toString() ))
as it is, a regexp is just overkill, QString::endsWith() does the same
> +// beautify its display to the users
> +modifiedData =
> modifiedData.toString().remove(QRegExp(".xml$"));
> +modifiedData =
> modifiedData.toString().remove(QRegExp("^[0-9]+\."));
pretty inefficient; you're getting the string out of the variant, do a
removal of some text using two regexps (which are not needed), and then
converting to qvariant again; just get the string out once, do the
replacements and reassign back to the variant
> +if (role == Qt::SizeHintRole) {
> +if ( orientation == Qt::Horizontal )
> +{
> +switch (section)
> +{
> +// 0 == File Name; 3 == Modified
> +// --this sizes the actual Header size so it looks
> proper +case 0: case 3:
> +//return QSize(20,20);
> +default:
> +return QVariant();
> +}
> +}
> +}
i don't think you need to play with the size hint of the header
elements, just leave whatever the style does
> +else if ( role == Qt::DisplayRole ) {
> +if ( orientation == Qt::Horizontal )
> +{
> +switch (section)
> +{
> +case 0:
> +return i18n("File Name");
> +case 3:
> +return i18n("Modified");
> +default:
> +return QVariant();
> +}
> +}
> +}
check again whether this is still needed with KDirModel
> +#ifndef _DLGMETADATA_H
> +#define _DLGMETADATA_H
OKULAR_CONF_DLGMETADATA_H would be better
> +clas
