Tommaso Cucinotta wrote:
Richard Heck ha scritto:
Yes, it doesn't actually commit anything: only svn ci will do that. svn add just registers the files with your local copy, so svn diff, etc, will see them.
In fact, done. Please, find the patched patch at bug 3998.

 http://bugzilla.lyx.org/show_bug.cgi?id=3998

Added missing files in diff (QSearchAdv.{cpp,h}, SearchAdvUi.ui). Cleaned up
commented code.

Looks terrific ;-)

Here's some comments on the code nevertheless:


Index: lib/bind/cua.bind
===================================================================
--- lib/bind/cua.bind   (revisione 19086)
+++ lib/bind/cua.bind   (copia locale)
@@ -62,6 +62,7 @@
 \bind "C-S-M"                        "math-display"

 \bind "C-f"                  "dialog-show findreplace"
+\bind "C-S-f"                        "dialog-show findreplaceadv"


I'd prefer avancedfindreplace personally.


Index: src/BufferView.cpp
===================================================================
--- src/BufferView.cpp  (revisione 19086)

+       case LFUN_WORD_FINDADV:
+               findAdv(this, cmd);

AdvancedFind() IMHO.


+static docstring stringifyFromCursor(DocIterator & cur, MatchStringAdv const & match) {

The convention in LyX sourcecode is to put local functions in the anonymous namespace:

namespace {
docstring stringifyFromCursor(..)
...
}


Index: src/frontends/qt4/QSearchAdv.h

AdvancedSearch.h would be better, we try to move away for Qxxx naming.


+
+protected Q_SLOTS:
+       void findNextClicked();
+       void findPrevClicked();
+       void replaceClicked();
+       void replaceallClicked();
+       void closeClicked();

Instead of those, you could use automatic slots:
+       void on_findNextPB_clicked();

This saves the manual signal/slot connection.


+       ControlSearchAdv * ctrl_;
+       WorkArea * origWorkArea_;       // The work area in which to search
+       GuiWorkArea * searchWorkArea_;  // The work area defining what to search

This GuiWorkArea is a child of QSearchAdvDialog right?


Index: src/frontends/qt4/QSearchAdv.cpp
===================================================================
+       origWorkArea_ = theApp()->currentView()->currentWorkArea();
+ searchWorkArea_ = new GuiWorkArea(frameRect.width(), frameRect.height(), 10, *theApp()->currentView());

Good.

+       searchWorkArea_->setBufferView(&ctrl_->bufferView());
+       searchWorkArea_->connectBuffer(ctrl->buffer());

You will need to simplify this with my branch.

I'll try to look at the Controller code tommorrow.

Good stuff ;-)

Abdel.

Reply via email to