Am 08.12.2012 08:09, schrieb Scott Kostyshak:
While studying InsetTabular{.cpp,.h}, I have some suggestions and questions.
In the 0001 patch attached, am I missing something or can we remove
cur.selection in the conditions?
I would leave them in. It makes sense to me that we don#t accept a "down" selection if only the
current cell is selected.
In 0003, can I rename "copy-column" and "copy-row" to
"duplicate-column" and "duplicate-row"? The features do not just copy
but they also insert. The both together seem more of a "duplicate"
functionality.
OK, if you like to. But for me "copy row " implies that the row content will appear somewhere else.
So it is either a cut+paste or copy+paste. In the first case it is not a duplication. But I don't
have a string opinion about the naming of the LFUN. But if you do the change, please also update our
LFUN list and the LFUN documentation LyX-file.
There are also a few things I did not change but that I find confusing:
Why isn't "col_type" not named "col_num" and row_type "row_num"?
Does the "_type" emphasize that it's a typedef?
I don't know. This must have historical reasons.
- I find it unintuitive that a cell that is at the beginning of a
multirow is not "part" of a multirow:
See, for example, Tabular::isPartOfMultiRow.
I guess my confusion is because of:
CELL_BEGIN_OF_MULTICOLUMN,
///
CELL_PART_OF_MULTICOLUMN,
From a programming point of view, I understand why they are treated
differently, but from an interface point of view, I wonder if there
could be better names. I can't think of any though.
Form the technical point of view there was no other solution and prom the UI point of view as a user
you are never confronted with this and thus won't notice.
- My same comment as above but for isPartOfMultiColumn.
Dito.
I have two questions regarding the following code, which is in
function <<bool InsetTabular::getStatus(Cursor & cur, FuncRequest
const & cmd, FuncStatus & status) const>>,
switch (action) {
case Tabular::SET_PWIDTH:
case Tabular::SET_MPWIDTH:
case Tabular::SET_SPECIAL_COLUMN:
case Tabular::SET_SPECIAL_MULTICOLUMN:
case Tabular::APPEND_ROW:
case Tabular::APPEND_COLUMN:
case Tabular::DELETE_ROW:
case Tabular::DELETE_COLUMN:
case Tabular::COPY_ROW:
case Tabular::COPY_COLUMN:
case Tabular::SET_TOP_SPACE:
case Tabular::SET_BOTTOM_SPACE:
case Tabular::SET_INTERLINE_SPACE:
status.clear();
return true;
(1) What is the purpose of this status.clear()?
(2) Why is "return true" used here instead of "break" (there is a
"return true" right after the cases and all the other cases use
"break")? Is this just inconsistent style or do they not provide
equivalent functionality?
I also don't know.
regards Uwe
