> 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.
OK.
> > > + // 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?
:P
> > > +void PanelStack::filterChanged(QString const & search) {
> > > + bool enable_all = search.length() == 0;
>
> Perhaps clearer:
>
> bool enable_all = search.isEmpty();
I forgot to fix this.
> > > + // 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.
I think it's useful for future coders.
> 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.
I wanted to add it, I put it in the namespace lyx::frontend, is that correct?
> > > +void PanelStack::resetSearch() {
> > > + this->search_->setText("");
> > > +}
>
> void PanelStack::resetSearch()
> {
> search_->setText(QString());
> }
OK.
> > >
> ===================================================================
> > > --- 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 ;-}
I think I'll write a tool to check the LyX code and see how frequently the
alphabetic order is respected. I can't see a reason for doing that. However,
fixed.
Style and form comments are always welcome but please give a little attention to
the content too:
- Alternative ideas to highlight matching widgets? Currently they become red.
- Ideas on how to make the research faster? In the next patch I'll try to add a
little delay.
Thanks.
venom00
