On 12/08/2012 02:11 AM, Scott Kostyshak wrote:
This patch implements a "move row" feature for tabular. The purpose is
to provide a useful behavior in tabular that is consistent with
PARAGRAPH_MOVE_UP and PARAGRAPH_MOVE_DOWN so that the user can do, for
example, alt-<up> to move a row up.
I don't use tables much, but this looks useful.
If there is any selection, the LFUN is disabled. This is consistent
with how PARAGRAPH_MOVE_UP works in other contexts.
"move row" moves only the left and right borders of a cell along with
the row and does not move the top and bottom borders. This seems a
little inconsistent to me from an OOP point of view (all borders are
part of the CellData object), but I think it is intuitive to the user
(left and right borders can be viewed as part of the cell whereas top
and bottom borders can be viewed as part of the tabular structure).
Any thoughts on this?
I'm not sure what the expected behavior is when multirows are involved.
Suppose that the user is going to do a move row down (PARAGRAPH_MOVE_DOWN).
In particular:
(1) What if the current row contains a cell that is part of a multirow?
(2) What if not (1) but the row beneath it has a cell that is part of
a multirow?
I see two solutions:
(a) In cases (1) and (2) disable the LFUN.
(b) In case (1) move all rows that are part of the multirow along with
the current row under the row that is below. In case (2) move the
current row beneath all of the rows that are part of the multirow.
If that were the end of the story, I think I would favor solution (b).
However, things get trickier.
(3) What if the row beneath it has a cell that is part of a multirow
and a different cell starts a new multirow? (imagine a chain of
multirows where when one ends another one in a different cell begins)
move-row could move the current row to beneath where the chain ends. I
don't think this would be too hard to implement, but my question is --
is this intuitive to the user? I'm not sure.
This sounds as if it will get insanely complicated, and I can imagine
cases in which there would not be anything sensible to do. (E.g., every
row except the last has a cell that starts a multirow.) So I'd suggest
disabling the LFUN or, perhaps better, enabling it, but popping an error
dialog that says, "Cannot move row since it contains multirow".
Otherwise, people will just be confused about why it works sometimes and
not other times.
Change tracking seems to set the changes, but "next change" does not
work (it just highlight's the column and accept change does nothing).
I have no experience with change tracking and am unsure what to do.
I'm not experienced with this either. I'd post separately about it.
More picky, technical questions:
Is my bracket-scoping poor style? I do this to make sure i don't use
an i where I should use a j (e.g. avoid copy paste errors now and in
the future).
Hmm. I have no opinion about this and can see the point of it. But I'll
guess other people will have strong opinions, since you never see this
in the LyX code. So maybe use it during development and remove it for
what you commit?
I use a pair to store two bools. Should std::pair only be used when
the two are different types?
No, that is fine.
Instead of pair, I could use a two
element vector or two element array or a struct. I guess that the
reason I like pair over a vector is because it provides more clear and
compact syntax (although uniform initialization in C++11 will change
that I guess).
The vector is definitley wrong, since that suggests you're prepared to
add and remove stuff. You could use a struct, but a pair is probably
some kind of struct anyway.
Richard
