Hi.
On Wed, Apr 27, 2011 at 02:07:55PM +0200, venom00 wrote:
> > +void GuiDocument::hideView() {
> > + Dialog::hideView();
> > + // Reset the search box
> > + this->docPS->resetSearch();
> > +}
Style nits: { on a separate line for the function body, and this->
seems superfluous.
> > + // Configure tree
> > list_->setRootIsDecorated(false);
> > list_->setColumnCount(1);
> > list_->header()->hide();
> > @@ -43,14 +54,28 @@
> > list_->header()->setStretchLastSection(false);
> > list_->setMinimumSize(list_->viewport()->size());
> >
> > +
> > connect(list_, SIGNAL(currentItemChanged(QTreeWidgetItem*,
Was the extra empty line intended?
> > +void PanelStack::filterChanged(QString const & search) {
> > + bool enable_all = search.length() == 0;
Perhaps clearer:
bool enable_all = search.isEmpty();
> > + // Try to cast to the most common widgets and
> > looks in it's content by each
> > + // It's bad OOP, it would be nice to have a
> > QWidget::toString() overloaded by
> > + // each widget, but this would require to change
> > Qt or subclass each widget.
I think there's no need to add comments mentioning unfeasible approaches.
A helper function or two like
bool matches(QString text)
{
text.remove('&');
return text.contains(search, Qt::CaseInsensitive);
}
might help to make that block a bit more palatable.
> > +void PanelStack::resetSearch() {
> > + this->search_->setText("");
> > +}
void PanelStack::resetSearch()
{
search_->setText(QString());
}
> > ===================================================================
> > --- src/frontends/qt4/PanelStack.h (revisione 38474)
> > +++ src/frontends/qt4/PanelStack.h (copia locale)
> > @@ -16,6 +16,7 @@
> > #include <QWidget>
> > #include <QHash>
> >
> > +class QLineEdit;
> > class QTreeWidget;
> > class QTreeWidgetItem;
> > class QStackedWidget;
Someone before you fumbled with the alphabetic order here ;-}
Andre'
PS: Thanks for that other mail.
