On Fri, Jul 01, 2005 at 09:23:59PM +0200, Andre Poenitz wrote: > On Wed, Jun 29, 2005 at 10:48:02PM +0300, Martin Vermeer wrote: > > On Wed, Jun 29, 2005 at 10:46:18AM +0300, Martin Vermeer wrote: > > > > ... > > > > > Here is the patch, armed with comments for posterity. > > > > And here a better one. > > I don't agree with the idea of storing row and column in the cursor. > This duplicates information and will lead to synchronization troubles.
Actually I would have preferred to have _only_ row, col. But there are
dozens of statements in the code like "idx()= ..." with idx() on the
left side, which stop working without idx_. I didn't want to go there
at this point.
(...and about "synchronization troubles": we already have those ;-)
> Apart from that, the cursor is already far too fat to be nice. But
> that's my doing, I know.
Not only... at least it is somewhat understandable now.
> > - else if (s == "copy-row")
> > + else if (s == "copy-row") {
> > + // Here (as later) we save the cursor col/row
> > + // in order to restore it after operation.
> > + cur.idxSave();
> > for (int i = 0, n = extractInt(is); i < n; ++i)
> > copyRow(cur.row());
> > - else if (s == "swap-row")
> > + cur.idxLoad();
>
> With about the same effort one should be able to set idx do the proper
> value directly and be done.
>
> > - row_type const r = cur.row();
> > - col_type const c = cur.col();
> > - addCol(c);
> > - cur.idx() = index(r, c);
> > + cur.idxSave();
> > + addCol(cur.col());
> > + cur.idxLoad();
>
> The price for the line saved is too high IMO.
Hmmm yes, probably. Both fixes are kludges. Both do the job. Reverse and
check in the old patch, if you find it important enough ;-)
- Martin
pgpZ9OZospiO8.pgp
Description: PGP signature
