On Mon, Jan 08, 2001 at 11:04:16AM +0100, Juergen Vigna wrote:
> 1. ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> @@ -2195,7 +2195,16 @@ int LyXTabular::Latex(Buffer const * buf
> +           bool rtl = inset->par->isRightToLeftPar(buf->params) &&
> +                   inset->par->Last() > 0 && GetPWidth(cell).empty();
> 
> Last() returns a pointer so it really should be .. && inset->par->Last() &&
> shouldn't it?

No, Last() returns LyXParagraph::size_type (= unsigned int).

> Then IMO this should be fixed in the insettext NOT in the insettabular!
> I will remove this part of the patch!

Why? this is a fix which is specific for tabular, so it should be in the
tabular code.

> 2. ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> -       if (inset->IsTextInset()) {
> -               if (cursor.par()->isRightToLeftPar(bview->buffer()->params))
> -                       CursorRight(bview);
> -               else
> -                       CursorLeft(bview);
> +       if (inset->Editable() == Inset::HIGHLY_EDITABLE) {
> +               CursorLeft(bview, true);
> 
> Why did you remove the check for the RTL paragraph?

Before the if cluase we have cursor.pos() = inset_pos+1 (because InsertChar
moved the cursor), and we want to have cursor.pos() = inset_pos, so we need to
call to CursorLeft() for both RTL and LTR text!

> 3. ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> +
> +    // If the inset is empty set the language of the current font to the
> +    // language to the surronding text.
> +    if (par->Last() == 0 && !par->next) {
> +       LyXFont font(LyXFont::ALL_IGNORE);
> +       font.setLanguage(bv->getParentLanguage(this));
> +       SetFont(bv, font, false);
> +    }
> 
> I really don't know if I like this part (well I don't like it :). Think
> of me erasing all the contents of a cell, then going somewhere to copy
> some text and to paste it inside the cell. After reentering the cell
> (as now it is empty) the language is set to the surronding language and
> not to the language I have set earlier! IMO we should find a better solution
> but as this is just a move of the settings in a better location is is sound.

No, the language of the pasted text will be the language of the original text
(i.e. the text which was copied).
So I don't see a problem here.

> 4. ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> -           case -1:
> +           case LFUN_UNKNOWN_ACTION:
> +           case LFUN_BREAKPARAGRAPH:
> +           case LFUN_BREAKLINE:
> 
> Well the LFUN_UNKNOWN_ACTION is the right thing, but why the tow?

I don't know. I just copied this from lyxfunc.C

> 5. ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> -           case LFUN_DOWN:
> -               moveDown(bv);

Because it is not needed (the action will be performed in the switch below).
Note that we don't have LFUN_UP case, so we don't need LFUN_DOWN.

> Why did you remove this handlings?
> 
> -           return DISPATCHED;
> +           if (dispatched)
> +               return DISPATCHED;
> 
> I'm not sure this is correct! Could you explain?

Insert a math inset into a cell of a tabular, and then press space at the
end of the math inset. The correct behavior is to exit from the math inset
into the insettext, AND insert space in the insettext after the math inset.
However, because of the return in the original code, the latter is not done.
The change I've made fixes this.

> > 2. Why the insettabular needs two "dummy" cursor positions per cell ?
> 
> Because otherwise you can enter an inset only from one position and how

Why do you need this? (you can use Home/End).

> would you make selection of columns possible form both directions?

I don't understand what you meant here. Currently, the selection behave the
same whether the cursor is currently at a left dummy position or at the
right dummy position.

Reply via email to