On Fri, Nov 14, 2008 at 05:54:49PM +0100, Pavel Sanda wrote:
> Hello Tommaso,
> 
> we have agreed that it would be good to merge search branch into trunk,
> so the work continue there. For the proper merge we need review the
> current patch bit by bit.

For one, I'd like to call it 'AdvancedSearch' or such.

'SearchAdv' always makes me wonder why I should search advertisement.

> +     if (! inset().asInsetMath())
> +             return false;
> +     InsetMathHull * i = dynamic_cast<InsetMathHull *>
> +             (inset().asInsetMath());
> +     return (i && i->getType() == hullRegexp);

Style: Spacing and parantheses.

> +
> +SearchAdvDialog::SearchAdvDialog(
> +     GuiView & parent
> + ) : DockView(parent, "Find LyX", "Find LyX Dialog", 
> Qt::RightDockWidgetArea),


        SearchAdvDialog::SearchAdvDialog(GuiView & parent)
                : DockView(parent, "Find LyX", "Find LyX Dialog", 
Qt::RightDockWidgetArea),

> + ** in theBuffeerList() destruction loop at application exit
> + **/
> +SearchAdvDialog::~SearchAdvDialog() {

{ on new line. Mor of those below.


> +void SearchAdvDialog::findAdv(bool casesensitive,
> +             bool matchword, bool backwards,
> +             bool expandmacros, bool ignoreformat)

I wonder whether we should start using a structure for the parameters
instead of an ever growing list of bools...

> +                     lyxerr << "searchString up to here: " << 
> lyx::to_utf8(os.str()) << std::endl;

Is lyx:: needed?

Also, we changed to  'using namespace std;' recently.

>  GuiWorkArea * GuiView::workArea(Buffer & buffer)
>  {
> +     if (currentWorkArea()
> +         && &currentWorkArea()->bufferView().buffer() == &buffer)
> +             return (GuiWorkArea *) currentWorkArea();

C style cast?

> +#include <iostream>
> +#include <sstream>
> +#include "TexRow.h"
> +#include "OutputParams.h"
> +#include "output_latex.h"
> +
> +#include <boost/regex.hpp>
> +
> +namespace lyx {
> +
> +using support::compare_no_case;
> +using support::uppercase;
> +using support::lowercase;
> +using support::split;
> +
> +using std::advance;
> +
> +namespace {
> +
> +typedef std::vector<std::pair<std::string, std::string> > Escapes;
> +
> +/// A map of symbols and their escaped equivalent needed within a regex.
> +Escapes const & get_regexp_escapes() {
> +     static Escapes escape_map;
> +     if (escape_map.empty()) {
> +             escape_map.push_back(std::pair<std::string, std::string>("\\", 
> "\\\\"));
> +             escape_map.push_back(std::pair<std::string, std::string>("^", 
> "\\^"));
> +             escape_map.push_back(std::pair<std::string, std::string>("$", 
> "\\$"));
> +             escape_map.push_back(std::pair<std::string, std::string>("{", 
> "\\{"));
> +             escape_map.push_back(std::pair<std::string, std::string>("}", 
> "\\}"));
> +             escape_map.push_back(std::pair<std::string, std::string>("[", 
> "\\["));
> +             escape_map.push_back(std::pair<std::string, std::string>("]", 
> "\\]"));
> +             escape_map.push_back(std::pair<std::string, std::string>("(", 
> "\\("));
> +             escape_map.push_back(std::pair<std::string, std::string>(")", 
> "\\)"));
> +             escape_map.push_back(std::pair<std::string, std::string>("+", 
> "\\+"));
> +             escape_map.push_back(std::pair<std::string, std::string>("*", 
> "\\*"));
> +             escape_map.push_back(std::pair<std::string, std::string>(".", 
> "\\."));
> +     }

Using a struct might reduced line noise here.

> +
> +/** Return the position of the closing brace matching the open one at s[pos],
> + ** or s.size() if not found.
> + **/
> +size_t find_matching_brace(std::string const & s, size_t pos) {
> +     BOOST_ASSERT(s[pos] == '{');

LASSERT

Andre'

Reply via email to