Edwin Leuven wrote:
> Georg Baum wrote:
>> I had a closer look, and IMHO it is only partly an improvement. You
>> remove the functionality to set the borders of whole columns/rows.
>
> i think that it is possible to set borders of whole cols/rows. just
> select them and set/unset them.
And how is that implemented? I do not understand how you can remove the
wholerow/wholecolumn arguments of some LyXTabular methods without making
this impossible.
> afaics my changes make the dialog and toolbar act in a consistent way
> which i think is what people expect.
Yes, and I agree with that.
>> If you rework your patch to not remove this functionality then it
>> would be OK with me. It would be even better if it would be possible
>> to set/reset whole column/row lines in the dialog, but since the
>> dialog is horribly broken anwyway this can also wait.
>
> can you give me an example what you are missing? i didn't follow the one
> above. (i might be wrong but there is no table you cannot make after the
> patch that you could make before.)
I did not try the patch, but from looking at it I saw that you removed the
last argument of the topLine() etc. methods. That means that existing
functionality is removed. Do you know what that means in
InsetTabular::copySelection()?
> the only thing that is different with my patch is that when the cursor
> is in a multicolumn cell toggling applies to that cell's borders.
No, you also changed copying.
> more in general i think that it makes sense to have the toggling apply
> to what is selected:
>
> cursor in cell -> toggle cell
> selection -> toggle selection
>
> want to change the whole row/col? -> select row/col and toggle
That makes sense. As I wrote, I am in favour of the toggling part of your
patch. What I do not like is that you remove the boolean flags of the
LyXTabular methods. I do not understand why these are not used for setting
the lines of whole rows/columns. That probably means some duplicate code. I
really would like to preserve these arguments and reuse these methods later
for changing whole columns/rows.
> the strange thing is that this comment is in the frontend whereas what
> you are saying should be handled by the core...
As I wrote: can of worms.
>> What do you mean? You can set the lines in the UI per cell. If all
>> lines in a row are set this results in a \hline, otherwise in one or
>> more \cline. What else has \cline and \hline to do with the user
>> interface?
>
> how do you do that? i don't manage to have a horizontal line that spans
> only a few columns unless i make the cell multicolumns cells...
Correct. I think now I understand what you mean: It is not possible to
change the top/bottom line of just one cell if it is not a multicolumn
cell. That is a strange limitation that should be removed. Again: Can of
worms.
BTW, you forgot to remove some enum values. Here is an updated version of
your patch.
Georg
Index: src/insets/insettabular.C
===================================================================
--- src/insets/insettabular.C (Revision 15932)
+++ src/insets/insettabular.C (Arbeitskopie)
@@ -108,10 +108,6 @@ TabularFeature tabularFeature[] =
{ LyXTabular::VALIGN_TOP, "valign-top" },
{ LyXTabular::VALIGN_BOTTOM, "valign-bottom" },
{ LyXTabular::VALIGN_MIDDLE, "valign-middle" },
- { LyXTabular::M_TOGGLE_LINE_TOP, "m-toggle-line-top" },
- { LyXTabular::M_TOGGLE_LINE_BOTTOM, "m-toggle-line-bottom" },
- { LyXTabular::M_TOGGLE_LINE_LEFT, "m-toggle-line-left" },
- { LyXTabular::M_TOGGLE_LINE_RIGHT, "m-toggle-line-right" },
{ LyXTabular::M_ALIGN_LEFT, "m-align-left" },
{ LyXTabular::M_ALIGN_RIGHT, "m-align-right" },
{ LyXTabular::M_ALIGN_CENTER, "m-align-center" },
@@ -843,28 +839,20 @@ bool InsetTabular::getStatus(LCursor & c
status.setOnOff(tabular.isMultiColumn(cur.idx()));
break;
- case LyXTabular::M_TOGGLE_LINE_TOP:
- flag = false;
case LyXTabular::TOGGLE_LINE_TOP:
- status.setOnOff(tabular.topLine(cur.idx(), flag));
+ status.setOnOff(tabular.topLine(cur.idx()));
break;
- case LyXTabular::M_TOGGLE_LINE_BOTTOM:
- flag = false;
case LyXTabular::TOGGLE_LINE_BOTTOM:
- status.setOnOff(tabular.bottomLine(cur.idx(), flag));
+ status.setOnOff(tabular.bottomLine(cur.idx()));
break;
- case LyXTabular::M_TOGGLE_LINE_LEFT:
- flag = false;
case LyXTabular::TOGGLE_LINE_LEFT:
- status.setOnOff(tabular.leftLine(cur.idx(), flag));
+ status.setOnOff(tabular.leftLine(cur.idx()));
break;
- case LyXTabular::M_TOGGLE_LINE_RIGHT:
- flag = false;
case LyXTabular::TOGGLE_LINE_RIGHT:
- status.setOnOff(tabular.rightLine(cur.idx(), flag));
+ status.setOnOff(tabular.rightLine(cur.idx()));
break;
case LyXTabular::M_ALIGN_LEFT:
@@ -1477,54 +1465,35 @@ void InsetTabular::tabularFeatures(LCurs
cur.idx() = tabular.getCellNumber(row, column);
break;
- case LyXTabular::M_TOGGLE_LINE_TOP:
- flag = false;
case LyXTabular::TOGGLE_LINE_TOP: {
- bool lineSet = !tabular.topLine(cur.idx(), flag);
+ bool lineSet = !tabular.topLine(cur.idx());
for (row_type i = sel_row_start; i <= sel_row_end; ++i)
for (col_type j = sel_col_start; j <= sel_col_end; ++j)
- tabular.setTopLine(
- tabular.getCellNumber(i, j),
- lineSet, flag);
+ tabular.setTopLine(tabular.getCellNumber(i, j), lineSet);
break;
}
- case LyXTabular::M_TOGGLE_LINE_BOTTOM:
- flag = false;
case LyXTabular::TOGGLE_LINE_BOTTOM: {
- bool lineSet = !tabular.bottomLine(cur.idx(), flag);
+ bool lineSet = !tabular.bottomLine(cur.idx());
for (row_type i = sel_row_start; i <= sel_row_end; ++i)
for (col_type j = sel_col_start; j <= sel_col_end; ++j)
- tabular.setBottomLine(
- tabular.getCellNumber(i, j),
- lineSet,
- flag);
+ tabular.setBottomLine(tabular.getCellNumber(i, j), lineSet);
break;
}
- case LyXTabular::M_TOGGLE_LINE_LEFT:
- flag = false;
case LyXTabular::TOGGLE_LINE_LEFT: {
- bool lineSet = !tabular.leftLine(cur.idx(), flag);
+ bool lineSet = !tabular.leftLine(cur.idx());
for (row_type i = sel_row_start; i <= sel_row_end; ++i)
for (col_type j = sel_col_start; j <= sel_col_end; ++j)
- tabular.setLeftLine(
- tabular.getCellNumber(i,j),
- lineSet,
- flag);
+ tabular.setLeftLine(tabular.getCellNumber(i,j), lineSet);
break;
}
- case LyXTabular::M_TOGGLE_LINE_RIGHT:
- flag = false;
case LyXTabular::TOGGLE_LINE_RIGHT: {
- bool lineSet = !tabular.rightLine(cur.idx(), flag);
+ bool lineSet = !tabular.rightLine(cur.idx());
for (row_type i = sel_row_start; i <= sel_row_end; ++i)
for (col_type j = sel_col_start; j <= sel_col_end; ++j)
- tabular.setRightLine(
- tabular.getCellNumber(i,j),
- lineSet,
- flag);
+ tabular.setRightLine(tabular.getCellNumber(i,j), lineSet);
break;
}
@@ -1776,9 +1745,9 @@ bool InsetTabular::copySelection(LCursor
while (paste_tabular->rows() > rows)
paste_tabular->deleteRow(rows);
- paste_tabular->setTopLine(0, true, true);
+ paste_tabular->setTopLine(0, true);
paste_tabular->setBottomLine(paste_tabular->getFirstCellInRow(rows - 1),
- true, true);
+ true);
for (col_type i = 0; i < cs; ++i)
paste_tabular->deleteColumn(0);
@@ -1787,9 +1756,8 @@ bool InsetTabular::copySelection(LCursor
while (paste_tabular->columns() > columns)
paste_tabular->deleteColumn(columns);
- paste_tabular->setLeftLine(0, true, true);
- paste_tabular->setRightLine(paste_tabular->getLastCellInRow(0),
- true, true);
+ paste_tabular->setLeftLine(0, true);
+ paste_tabular->setRightLine(paste_tabular->getLastCellInRow(0), true);
odocstringstream os;
OutputParams const runparams;
Index: src/frontends/qt4/QTabularDialog.C
===================================================================
--- src/frontends/qt4/QTabularDialog.C (Revision 15932)
+++ src/frontends/qt4/QTabularDialog.C (Arbeitskopie)
@@ -230,28 +230,28 @@ void QTabularDialog::borderUnset_clicked
void QTabularDialog::leftBorder_changed()
{
- form_->controller().toggleLeftLine();
+ form_->controller().set(LyXTabular::TOGGLE_LINE_LEFT);
form_->changed();
}
void QTabularDialog::rightBorder_changed()
{
- form_->controller().toggleRightLine();
+ form_->controller().set(LyXTabular::TOGGLE_LINE_RIGHT);
form_->changed();
}
void QTabularDialog::topBorder_changed()
{
- form_->controller().toggleTopLine();
+ form_->controller().set(LyXTabular::TOGGLE_LINE_TOP);
form_->changed();
}
void QTabularDialog::bottomBorder_changed()
{
- form_->controller().toggleBottomLine();
+ form_->controller().set(LyXTabular::TOGGLE_LINE_BOTTOM);
form_->changed();
}
Index: src/frontends/qt4/QTabular.C
===================================================================
--- src/frontends/qt4/QTabular.C (Revision 15932)
+++ src/frontends/qt4/QTabular.C (Arbeitskopie)
@@ -114,20 +114,19 @@ void QTabular::update_borders()
LyXTabular::idx_type const cell = controller().getActiveCell();
bool const isMulticolumnCell = tabular.isMultiColumn(cell);
+ dialog_->borders->setTop(tabular.topLine(cell));
+ dialog_->borders->setBottom(tabular.bottomLine(cell));
+ dialog_->borders->setLeft(tabular.leftLine(cell));
+ dialog_->borders->setRight(tabular.rightLine(cell));
+
if (!isMulticolumnCell) {
dialog_->borders->setLeftEnabled(true);
dialog_->borders->setRightEnabled(true);
- dialog_->borders->setTop(tabular.topLine(cell, true));
- dialog_->borders->setBottom(tabular.bottomLine(cell, true));
- dialog_->borders->setLeft(tabular.leftLine(cell, true));
- dialog_->borders->setRight(tabular.rightLine(cell, true));
// repaint the setborder widget
dialog_->borders->update();
return;
}
- dialog_->borders->setTop(tabular.topLine(cell));
- dialog_->borders->setBottom(tabular.bottomLine(cell));
// pay attention to left/right lines: they are only allowed
// to set if we are in first/last cell of row or if the left/right
// cell is also a multicolumn.
Index: src/frontends/controllers/ControlTabular.h
===================================================================
--- src/frontends/controllers/ControlTabular.h (Revision 15932)
+++ src/frontends/controllers/ControlTabular.h (Arbeitskopie)
@@ -46,12 +46,6 @@ public:
/// set a parameter
void set(LyXTabular::Feature, std::string const & arg = std::string());
- /// borders
- void toggleTopLine();
- void toggleBottomLine();
- void toggleLeftLine();
- void toggleRightLine();
-
void setSpecial(std::string const & special);
void setWidth(std::string const & width);
Index: src/frontends/controllers/ControlTabular.C
===================================================================
--- src/frontends/controllers/ControlTabular.C (Revision 15932)
+++ src/frontends/controllers/ControlTabular.C (Arbeitskopie)
@@ -83,42 +83,6 @@ bool ControlTabular::useMetricUnits() co
}
-void ControlTabular::toggleTopLine()
-{
- if (tabular().isMultiColumn(getActiveCell()))
- set(LyXTabular::M_TOGGLE_LINE_TOP);
- else
- set(LyXTabular::TOGGLE_LINE_TOP);
-}
-
-
-void ControlTabular::toggleBottomLine()
-{
- if (tabular().isMultiColumn(getActiveCell()))
- set(LyXTabular::M_TOGGLE_LINE_BOTTOM);
- else
- set(LyXTabular::TOGGLE_LINE_BOTTOM);
-}
-
-
-void ControlTabular::toggleLeftLine()
-{
- if (tabular().isMultiColumn(getActiveCell()))
- set(LyXTabular::M_TOGGLE_LINE_LEFT);
- else
- set(LyXTabular::TOGGLE_LINE_LEFT);
-}
-
-
-void ControlTabular::toggleRightLine()
-{
- if (tabular().isMultiColumn(getActiveCell()))
- set(LyXTabular::M_TOGGLE_LINE_RIGHT);
- else
- set(LyXTabular::TOGGLE_LINE_RIGHT);
-}
-
-
void ControlTabular::setSpecial(string const & special)
{
if (tabular().isMultiColumn(getActiveCell()))
Index: src/tabular.C
===================================================================
--- src/tabular.C (Revision 15933)
+++ src/tabular.C (Arbeitskopie)
@@ -662,31 +662,30 @@ LyXTabular::idx_type LyXTabular::numberO
}
-bool LyXTabular::topLine(idx_type const cell, bool const wholerow) const
+bool LyXTabular::topLine(idx_type const cell) const
{
- if (!wholerow && isMultiColumn(cell) &&
- !(use_booktabs && row_of_cell(cell) == 0))
+ if (isMultiColumn(cell) &&
+ !(use_booktabs && row_of_cell(cell) == 0))
return cellinfo_of_cell(cell).top_line;
return row_info[row_of_cell(cell)].top_line;
}
-bool LyXTabular::bottomLine(idx_type const cell, bool wholerow) const
+bool LyXTabular::bottomLine(idx_type const cell) const
{
- if (!wholerow && isMultiColumn(cell) &&
+ if (isMultiColumn(cell) &&
!(use_booktabs && isLastRow(cell)))
return cellinfo_of_cell(cell).bottom_line;
return row_info[row_of_cell(cell)].bottom_line;
}
-bool LyXTabular::leftLine(idx_type cell, bool wholecolumn) const
+bool LyXTabular::leftLine(idx_type cell) const
{
if (use_booktabs)
return false;
- if (!wholecolumn && isMultiColumn(cell) &&
- (isFirstCellInRow(cell) || isMultiColumn(cell-1)))
- {
+ if (isMultiColumn(cell) &&
+ (isFirstCellInRow(cell) || isMultiColumn(cell-1))) {
if (cellinfo_of_cell(cell).align_special.empty())
return cellinfo_of_cell(cell).left_line;
return prefixIs(ltrim(cellinfo_of_cell(cell).align_special), "|");
@@ -697,13 +696,12 @@ bool LyXTabular::leftLine(idx_type cell,
}
-bool LyXTabular::rightLine(idx_type cell, bool wholecolumn) const
+bool LyXTabular::rightLine(idx_type cell) const
{
if (use_booktabs)
return false;
- if (!wholecolumn && isMultiColumn(cell) &&
- (isLastCellInRow(cell) || isMultiColumn(cell + 1)))
- {
+ if (isMultiColumn(cell) &&
+ (isLastCellInRow(cell) || isMultiColumn(cell + 1))) {
if (cellinfo_of_cell(cell).align_special.empty())
return cellinfo_of_cell(cell).right_line;
return suffixIs(rtrim(cellinfo_of_cell(cell).align_special), "|");
@@ -891,9 +889,9 @@ void LyXTabular::setWidthOfCell(idx_type
int width = 0;
int add_width = 0;
- if (rightLine(cell_info[row][column1].cellno, true) &&
+ if (rightLine(cell_info[row][column1].cellno) &&
column1 < columns_ - 1 &&
- leftLine(cell_info[row][column1+1].cellno, true))
+ leftLine(cell_info[row][column1+1].cellno))
{
add_width = WIDTH_OF_LINE;
}
@@ -1025,37 +1023,37 @@ void LyXTabular::setAllLines(idx_type ce
}
-void LyXTabular::setTopLine(idx_type cell, bool line, bool wholerow)
+void LyXTabular::setTopLine(idx_type cell, bool line)
{
row_type const row = row_of_cell(cell);
- if (wholerow || !isMultiColumn(cell))
+ if (!isMultiColumn(cell) || (use_booktabs && row == 0))
row_info[row].top_line = line;
else
cellinfo_of_cell(cell).top_line = line;
}
-void LyXTabular::setBottomLine(idx_type cell, bool line, bool wholerow)
+void LyXTabular::setBottomLine(idx_type cell, bool line)
{
- if (wholerow || !isMultiColumn(cell))
+ if (!isMultiColumn(cell) || (use_booktabs && isLastRow(cell)))
row_info[row_of_cell(cell)].bottom_line = line;
else
cellinfo_of_cell(cell).bottom_line = line;
}
-void LyXTabular::setLeftLine(idx_type cell, bool line, bool wholecolumn)
+void LyXTabular::setLeftLine(idx_type cell, bool line)
{
- if (wholecolumn || !isMultiColumn(cell))
+ if (!isMultiColumn(cell) && !use_booktabs)
column_info[column_of_cell(cell)].left_line = line;
else
cellinfo_of_cell(cell).left_line = line;
}
-void LyXTabular::setRightLine(idx_type cell, bool line, bool wholecolumn)
+void LyXTabular::setRightLine(idx_type cell, bool line)
{
- if (wholecolumn || !isMultiColumn(cell))
+ if (!isMultiColumn(cell) && !use_booktabs)
column_info[right_column_of_cell(cell)].right_line = line;
else
cellinfo_of_cell(cell).right_line = line;
@@ -1912,8 +1910,8 @@ int LyXTabular::TeXCellPreamble(odocstre
} else {
if (leftLine(cell) &&
(isFirstCellInRow(cell) ||
- (!isMultiColumn(cell - 1) && !leftLine(cell, true) &&
- !rightLine(cell - 1, true))))
+ (!isMultiColumn(cell - 1) && !leftLine(cell) &&
+ !rightLine(cell - 1))))
{
os << '|';
}
Index: src/tabular.h
===================================================================
--- src/tabular.h (Revision 15933)
+++ src/tabular.h (Arbeitskopie)
@@ -72,14 +72,6 @@ public:
///
VALIGN_MIDDLE,
///
- M_TOGGLE_LINE_TOP,
- ///
- M_TOGGLE_LINE_BOTTOM,
- ///
- M_TOGGLE_LINE_LEFT,
- ///
- M_TOGGLE_LINE_RIGHT,
- ///
M_ALIGN_LEFT,
///
M_ALIGN_RIGHT,
@@ -205,13 +197,13 @@ public:
row_type rows_arg);
/// Returns true if there is a topline, returns false if not
- bool topLine(idx_type cell, bool wholerow = false) const;
+ bool topLine(idx_type cell) const;
/// Returns true if there is a topline, returns false if not
- bool bottomLine(idx_type cell, bool wholerow = false) const;
+ bool bottomLine(idx_type cell) const;
/// Returns true if there is a topline, returns false if not
- bool leftLine(idx_type cell, bool wholecolumn = false) const;
+ bool leftLine(idx_type cell) const;
/// Returns true if there is a topline, returns false if not
- bool rightLine(idx_type cell, bool wholecolumn = false) const;
+ bool rightLine(idx_type cell) const;
///
bool topAlreadyDrawn(idx_type cell) const;
@@ -246,13 +238,13 @@ public:
///
void setAllLines(idx_type cell, bool line);
///
- void setTopLine(idx_type cell, bool line, bool wholerow = false);
+ void setTopLine(idx_type cell, bool line);
///
- void setBottomLine(idx_type cell, bool line, bool wholerow = false);
+ void setBottomLine(idx_type cell, bool line);
///
- void setLeftLine(idx_type cell, bool line, bool wholecolumn = false);
+ void setLeftLine(idx_type cell, bool line);
///
- void setRightLine(idx_type cell, bool line, bool wholecolumn = false);
+ void setRightLine(idx_type cell, bool line);
///
void setAlignment(idx_type cell, LyXAlignment align,
bool onlycolumn = false);
