Re: [patch] Up/down cursor in math-macro jumps out of the macro, #3830

[Stefan Schimanski]

I mean: if (cur.result().update() == Update::Force) this means that the flag has been explicitely set previously as opposed to have the default value (Update::FitCursor | Update::Force). Maybe this information could be used?

Here is a solution in the direction I described: the old cursor is stored before dispatch and then used after moving up/down to tell the insets that the cursor left them.

But I think I like your solution better. With your patch, as we now sort of ignore the initial setting of the update flag maybe we should change this as well?

        case LFUN_UP_SELECT:
        case LFUN_DOWN_SELECT:
                needsUpdate |= cur.selHandle(select);
        case LFUN_UP:
        case LFUN_DOWN: {
                // move cursor up/down
                bool up = cmd.action == LFUN_UP_SELECT || cmd.action == LFUN_UP;
                bool const successful = cur.upDownInText(up, needsUpdate);
                if (successful) {
                        // notify insets which were left and get their update 
flags
                        notifyCursorLeaves(cur.beforeDispatchCursor(), cur);
                        cur.fixIfBroken();
                        
                        // redraw if you leave mathed (for the decorations)
                        needsUpdate |= cur.beforeDispatchCursor().inMathed();
                } else
                        cur.undispatched();
                
                // save new selection
                if (cur.selection())
                        saveSelection(cur);
                break;
        }

I think we can simplify Text::dispatch for up/down because of your mechanism with the flags and needsUpdate below the big cases:

This looks good. Instead of storing the old cursor you could have reset the cursor at the beginning of InsetMathNest::dispatch() as I have done in Text::dispatch():

 // FIXME: We use the update flag to indicates wether a singlePar or a

 // it at the end?
 cur.noUpdate();

// full screen update is needed. We reset it here but shall we restore

Text is somehow special because of the handling of the flags. It's not really the way it should be IMO to have this extra needsUpdate logic. We should try to use the flags for exactly that. Probably something to clean up for 1.6. If I understand it right, in mathed only the flags are used, so there is no need for this cur.noUpdate() trick.

Index: src/mathed/InsetMathNest.cpp
===================================================================

+        // FIXME: what is this doing?
+        bool select = cmd.action == LFUN_DOWN_SELECT ||
+            cmd.action == LFUN_UP_SELECT;
+        cur.selHandle(select);

It sets the selection. Otherwise how would you notify that what's in between the current cursor position and the soon to be moved cursor is going to be selected?

Then the name selHandle is maybe not a good one.

I guess you could as well do something like this:

    case LFUN_DOWN_SELECT:
    case LFUN_UP_SELECT:
        cur.updateFlags(Update::Force | Update::FitCursor);
        cur.selHandle(select);
    case LFUN_DOWN:
    case LFUN_UP:
         cur.updateFlags(cur.result().update()
                | Update::Decoration | Update::FitCursor);

Stefan

I think Update::Force is set already before. To avoid redrawing in the non-selection case an explicit updateFlags(Update::Decoration | Update::FitCursor) is needed. So in any case we need some "if". So I leave it as it is I think.

PGP.sig
Description: Signierter Teil der Nachricht