Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...))

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Jul 20, 11:19am, Oliver Kiddle wrote:
} Subject: Re: PATCH: narrow-to-region (was Re: This widget implementation f
}
} My main use of narrow-to-region is to retrieve history lines into the
} middle of the current line. And it is just this situation that is
} breaking it. When doing:
}   zle undo $_ntr_changeno
} it is falling foul of the first line of unapplychange which is:
}   if(ch->hist != histline) {
} and not undoing enough changes.
} 
} This is a consequence of the fix in 10328. If I comment out the two
} lines of undo() which do else break; it appears to work fine and the
} problem that 10328 targetted doesn't seem to come back. But it is hard
} to know how that was failing at the time.

So you mean that part of undo becomes

        if (unapplychange(prev))
            curchange = prev;
    } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV));

??

What must be happening is that unapplychange() alters the history line,
returns zero, and then is called again with exactly the same struct,
which now succeeds on the adjusted history line.  Pre-10328, the call
to unapplychange() would fail but curchange was moved to ->prev either
way, skipping over the change that failed.

So the "else break;" looks like paranoia to prevent an infinite loop in
the event that unapplychange() fails every time.  Which could happen if
quietgethist() is a no-op?  Perhaps when a line has fallen out of the
history because of HISTSIZE?

Anyway modulo infinite-loop-prevention, it looks to me as if removing
that else-clause is the right thing to do, and similarly in redo() a
bit further along.  I'll let Oliver do the patch if we all agree.