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.
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.
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.
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).
I use a pair to store two bools. Should std::pair only be used when
the two are different types? 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).
If I'm on the right track with this patch, I will incorporate feedback
on how to handle multirow and how to handle track changes, and I will
extend to "move column".
Any comments on syntax/style or expected functionality would be appreciated.
Thank you,
Scott
From ce20a0b41299dfe02d0472da3e80f568ed9bf082 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Fri, 7 Dec 2012 02:17:15 -0500
Subject: [PATCH 9/9] Move row in tabular with alt-<up/down>
This patch is not meant to be final.
---
src/insets/InsetTabular.cpp | 91 ++++++++++++++++++++++++++++++++++++++++++-
src/insets/InsetTabular.h | 11 ++++++
2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index 24f43fb..1d9d7ba 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -114,6 +114,8 @@ TabularFeature tabularFeature[] =
{ Tabular::DELETE_COLUMN, "delete-column", false },
{ Tabular::DUPLICATE_ROW, "duplicate-row", false },
{ Tabular::DUPLICATE_COLUMN, "duplicate-column", false },
+ { Tabular::MOVE_ROW_DOWN, "move-row-down", false },
+ { Tabular::MOVE_ROW_UP, "move-row-up", false },
{ Tabular::SET_LINE_TOP, "set-line-top", true },
{ Tabular::SET_LINE_BOTTOM, "set-line-bottom", true },
{ Tabular::SET_LINE_LEFT, "set-line-left", true },
@@ -769,6 +771,61 @@ void Tabular::insertRow(row_type const row, bool copy)
}
+void Tabular::moveRow(row_type row, Tabular::RowDirection const direction)
+{
+ if (direction == Tabular::UP)
+ row = row -1;
+
+ swap(cell_info[row], cell_info[row + 1]);
+
+ updateIndexes();
+
+ // Note that the names refer to the positions after the swap.
+ map< pair<row_type, col_type>, pair<bool, bool> > rMap;
+ map< pair<row_type, col_type>, pair<bool, bool> > rplus1Map;
+
+ // Could potentially do everything in one loop, but would have to be
+ // careful with multicolumn cells. Currently the first loop reads the
+ // borders and the second loop writes them.
+ for (col_type c = 0; c < ncols(); ++c) {
+
+ // Cells part of a multicolumn will write to the same idx.
+ {
+ idx_type const i = cellIndex(row, c);
+ rMap[make_pair(row, c)] =
+ make_pair(topLine(i), bottomLine(i));
+ }
+ {
+ idx_type const j = cellIndex(row + 1, c);
+ rplus1Map[make_pair(row + 1, c)] =
+ make_pair(topLine(j), bottomLine(j));
+ }
+ }
+
+ for (col_type c = 0; c < ncols(); ++c) {
+ idx_type const i = cellIndex(row, c);
+ idx_type const j = cellIndex(row + 1, c);
+ {
+ pair<bool, bool> const j_lines =
+ rplus1Map[make_pair(row + 1, c)];
+ setTopLine(i, j_lines.first);
+ setBottomLine(i, j_lines.second);
+ }
+ {
+ pair<bool, bool> const i_lines =
+ rMap[make_pair(row, c)];
+ setTopLine(j, i_lines.first);
+ setBottomLine(j, i_lines.second);
+ }
+
+ if (buffer().params().trackChanges) {
+ cellInfo(i).inset->setChange(Change(Change::INSERTED));
+ cellInfo(j).inset->setChange(Change(Change::INSERTED));
+ }
+ }
+}
+
+
void Tabular::deleteColumn(col_type const col)
{
// Not allowed to delete last column
@@ -4171,6 +4228,14 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest
& cmd)
// break;
// }
+ case LFUN_PARAGRAPH_MOVE_DOWN:
+ tabularFeatures(cur, Tabular::MOVE_ROW_DOWN);
+ break;
+
+ case LFUN_PARAGRAPH_MOVE_UP:
+ tabularFeatures(cur, Tabular::MOVE_ROW_UP);
+ break;
+
case LFUN_LAYOUT_TABULAR:
cur.bv().showDialog("tabular");
break;
@@ -4403,6 +4468,14 @@ bool InsetTabular::getStatus(Cursor & cur, FuncRequest
const & cmd,
status.clear();
return true;
+ case Tabular::MOVE_ROW_DOWN:
+ status.setEnabled(!cur.selection() && tabular.nrows() >
tabular.cellRow(cur.idx()) + 1);
+ break;
+
+ case Tabular::MOVE_ROW_UP:
+ status.setEnabled(!cur.selection() &&
tabular.cellRow(cur.idx()) > 0);
+ break;
+
case Tabular::SET_TABULAR_WIDTH:
status.setEnabled(!tabular.rotate &&
!tabular.is_long_tabular
&& tabular.tabular_valignment ==
Tabular::LYX_VALIGN_MIDDLE);
@@ -4728,6 +4801,12 @@ bool InsetTabular::getStatus(Cursor & cur, FuncRequest
const & cmd,
return cell(cur.idx())->getStatus(cur, cmd, status);
}
+ case LFUN_PARAGRAPH_MOVE_DOWN:
+ return getStatus(cur, FuncRequest(LFUN_INSET_MODIFY, "tabular
move-row-down"), status);
+
+ case LFUN_PARAGRAPH_MOVE_UP:
+ return getStatus(cur, FuncRequest(LFUN_INSET_MODIFY, "tabular
move-row-up"), status);
+
// disable in non-fixed-width cells
case LFUN_PARAGRAPH_BREAK:
// multirow does not allow paragraph breaks
@@ -5358,6 +5437,16 @@ void InsetTabular::tabularFeatures(Cursor & cur,
cur.idx() = tabular.cellIndex(row, column);
break;
+ case Tabular::MOVE_ROW_DOWN:
+ tabular.moveRow(row, Tabular::DOWN);
+ cur.idx() = tabular.cellIndex(row + 1, column);
+ break;
+
+ case Tabular::MOVE_ROW_UP:
+ tabular.moveRow(row, Tabular::UP);
+ cur.idx() = tabular.cellIndex(row - 1, column);
+ break;
+
case Tabular::SET_LINE_TOP:
case Tabular::TOGGLE_LINE_TOP: {
bool lineSet = (feature == Tabular::SET_LINE_TOP)
@@ -5433,7 +5522,7 @@ void InsetTabular::tabularFeatures(Cursor & cur,
tabular.rightLine(cur.idx()));
break;
}
- // we have a selection so this means we just add all this
+ // we have a selection so this means we just add all these
// cells to form a multicolumn cell
idx_type const s_start = cur.selBegin().idx();
row_type const col_start = tabular.cellColumn(s_start);
diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h
index dd79a8f..2886486 100644
--- a/src/insets/InsetTabular.h
+++ b/src/insets/InsetTabular.h
@@ -136,6 +136,10 @@ public:
///
DUPLICATE_COLUMN,
///
+ MOVE_ROW_DOWN,
+ ///
+ MOVE_ROW_UP,
+ ///
SET_LINE_TOP,
///
SET_LINE_BOTTOM,
@@ -331,6 +335,11 @@ public:
CAPTION_ANY
};
+ enum RowDirection {
+ UP = 0,
+ DOWN
+ };
+
class ltType {
public:
// constructor
@@ -453,6 +462,8 @@ public:
///
void insertRow(row_type row, bool copy);
///
+ void moveRow(row_type row, Tabular::RowDirection direction);
+ ///
void appendColumn(col_type column);
///
void deleteColumn(col_type column);
--
1.7.9.5
