On Wed, Feb 21, 2007 at 03:12:24PM +0100, Stefan Schimanski wrote:
> Hi all!
>
> I wrote here recently about the sometimes hard to use math macros in
> LyX:
>
> http://www.mail-archive.com/[email protected]/msg53740.html
>
> Attached is an experimental patch against the 1.5 branch to make
> inline editing of macros possible, i.e. _without_ jumping back and
> forth between the LyX macro preview and the unfolded version with the
> #1, #2, ... lines.
>
> The main idea is the following: If a macro is expanded, the
> MathMacroArguments are replaced by MathMacroArgumentValues. These
> have pointers to the MathMacro cells to (only) pass through draw and
> metric calls to the cell. Hence the MathMacroArgumentValues will
> behave like the cell visually.
>
> This works quite nice, even if arguments appear multiple times in the
> macro definition. Also the cursor movement is acceptable. It just
> follows the order of argument appearances in the macro template.
>
> I am not entirely sure whether the memory management is correct. It
> would help if somebody who knows this in LyX could take a look over
> it. Especially I wonder if the cell pointers can become invalid by
> deleting the cell, but keeping the MathMacroArgumentValue object,
> hence leading to a segfault.
>
> Waiting for some comments
> Schimmi
Here we go.
> Index: InsetMathMacro.h
> ===================================================================
> --- InsetMathMacro.h (Revision 17273)
> +++ InsetMathMacro.h (Arbeitskopie)
> @@ -20,8 +20,25 @@
>
>
> namespace lyx {
> -
> -
> +
> + /// This class is the value of a macro argument, technically
> + /// a wrapper of the cells of MathMacro.
> + class MathMacroArgumentValue : public InsetMathDim {
> + public:
> + ///
> + MathMacroArgumentValue( MathArray *value );
> + ///
> + MathMacroArgumentValue( const MathMacroArgumentValue & );
> + ///
> + bool metrics(MetricsInfo & mi, Dimension & dim) const;
> + ///
> + void draw(PainterInfo &, int x, int y) const;
> +
> + private:
> + virtual std::auto_ptr<InsetBase> doClone() const;
> + MathArray *value_;
> + };
This is not needed outside InsetMathMacro.C. So move it there.
Note that we are very picky wrt to code formatting. Please have
a look at the rest of the code and follow the unwritten rules there
strictly.
This would e.g. be
MathMacroArgumentValue(MathArray * value);
MathMacroArgumentValue(MathMacroArgumentValue const &);
I'll attach a changed version of your patch. We should start from there.
Andre'
Index: InsetMathMacro.C
===================================================================
--- InsetMathMacro.C (revision 17284)
+++ InsetMathMacro.C (working copy)
@@ -33,6 +33,56 @@
using std::vector;
+/// This class is the value of a macro argument, technically
+/// a wrapper of the cells of MathMacro.
+class MathMacroArgumentValue : public InsetMathDim {
+public:
+ ///
+ MathMacroArgumentValue(MathArray * value) : value_(value) {}
+ ///
+ MathMacroArgumentValue(MathMacroArgumentValue const &);
+ ///
+ bool metrics(MetricsInfo & mi, Dimension & dim) const;
+ ///
+ void draw(PainterInfo &, int x, int y) const;
+
+private:
+ virtual std::auto_ptr<InsetBase> doClone() const;
+ MathArray *value_;
+};
+
+
+auto_ptr<InsetBase> MathMacroArgumentValue::doClone() const
+{
+ return auto_ptr<InsetBase>(new MathMacroArgumentValue(*this));
+}
+
+
+MathMacroArgumentValue::MathMacroArgumentValue(MathMacroArgumentValue const & x)
+ : value_(x.value_)
+{
+}
+
+
+bool MathMacroArgumentValue::metrics(MetricsInfo & mi, Dimension & dim) const
+{
+ value_->metrics(mi, dim);
+ metricsMarkers2(dim);
+ if (dim_ == dim)
+ return false;
+ dim_ = dim;
+ return true;
+}
+
+
+void MathMacroArgumentValue::draw(PainterInfo & pi, int x, int y) const
+{
+ value_->draw(pi, x, y);
+}
+
+
+
+
MathMacro::MathMacro(docstring const & name, int numargs)
: InsetMathNest(numargs), name_(name)
{}
@@ -63,24 +113,14 @@
{
if (!MacroTable::globalMacros().has(name())) {
mathed_string_dim(mi.base.font, "Unknown: " + name(), dim);
- } else if (editing(mi.base.bv)) {
- // FIXME UNICODE
- asArray(MacroTable::globalMacros().get(name()).def(), tmpl_);
- LyXFont font = mi.base.font;
- augmentFont(font, from_ascii("lyxtex"));
- tmpl_.metrics(mi, dim);
- // FIXME UNICODE
- dim.wid += mathed_string_width(font, name()) + 10;
- // FIXME UNICODE
- int ww = mathed_string_width(font, from_ascii("#1: "));
- for (idx_type i = 0; i < nargs(); ++i) {
- MathArray const & c = cell(i);
- c.metrics(mi);
- dim.wid = max(dim.wid, c.width() + ww);
- dim.des += c.height() + 10;
+ } else {
+ // create MathMacroArgumentValue object pointing to the cells of the macro
+ vector<MathArray> values(nargs());
+ for(size_t i = 0; i != nargs(); ++i) {
+ MathMacroArgumentValue value(const_cast<MathArray *>(&cells_[i]));
+ values[i].insert(0, MathAtom(value.clone().release()));
}
- } else {
- MacroTable::globalMacros().get(name()).expand(cells_, expanded_);
+ MacroTable::globalMacros().get(name()).expand(values, expanded_);
expanded_.metrics(mi, dim);
}
metricsMarkers2(dim);
@@ -96,26 +136,6 @@
if (!MacroTable::globalMacros().has(name())) {
// FIXME UNICODE
drawStrRed(pi, x, y, "Unknown: " + name());
- } else if (editing(pi.base.bv)) {
- LyXFont font = pi.base.font;
- augmentFont(font, from_ascii("lyxtex"));
- int h = y - dim_.ascent() + 2 + tmpl_.ascent();
- pi.pain.text(x + 3, h, name(), font);
- int const w = mathed_string_width(font, name());
- tmpl_.draw(pi, x + w + 12, h);
- h += tmpl_.descent();
- Dimension ldim;
- string t = "#1: ";
- mathed_string_dim(font, name(), ldim);
- for (idx_type i = 0; i < nargs(); ++i) {
- MathArray const & c = cell(i);
- h += max(c.ascent(), ldim.asc) + 5;
- c.draw(pi, x + ldim.wid, h);
- char_type str[] = { '#', '1', ':', '\0' };
- str[1] += static_cast<char_type>(i);
- pi.pain.text(x + 3, h, str, font);
- h += max(c.descent(), ldim.des) + 5;
- }
} else {
expanded_.draw(pi, x, y);
}
Index: InsetMathMacro.h
===================================================================
--- InsetMathMacro.h (revision 17284)
+++ InsetMathMacro.h (working copy)
@@ -20,8 +20,7 @@
namespace lyx {
-
-
+
/// This class contains the data for a macro.
class MathMacro : public InsetMathNest {
public:
