Jean-Marc Lasgouttes wrote:
> Juergen> Without having actually tested your patch,
>
> I see you use the same method as I do :)
;-)
(actually, I didn't know where to test it. Both my trees have been full of
changes lately, and I'd like to keep track).
> Juergen> We had fixed at least a big part of it (s&r), so the patch is
> Juergen> a ui-regression wrt 1.3 (while Alfredo's fixes the problem
> Juergen> completely, i.e. also after spellchecking, next-note etc.).
>
> Only the insets where a match is found get opened. I do not see why my
> patch could get us back to the old behaviour where all insets got
> opened...
Sorry, I haven't expressed myself clearly. All insets where a match is found
get and remain opened. In 1.3, we close them again after leaving, and I think
this feature is lost with you patch (I haven't tested, so please correct me
if I'm wrong).
> As far as I can see, the only difference is whether insets will close
> again when one leaves the insets. So the difference is an enhancement.
Partly. As said, in case of s&r, we already close, so we have a regression
with your patch, an enhancement with Alfredo's.
> One thing that makes me nervous with your approach is that it does not
> touch BufferView::setCursor:
>
> void BufferView::setCursor(DocIterator const & dit)
> {
> size_t const n = dit.depth();
> for (size_t i = 0; i < n; ++i)
> dit[i].inset().edit(cursor(), true);
>
> cursor().setCursor(dit);
> cursor().selection() = false;
> }
>
>
> The loop in this code calls edit() with the last value of cursor(),
> not the new value that we set later. It seems to me that this can
> create big problems (also I saw none yet). Should this code be moved
> after the cursor().setCursorcall?
This looks suspicious indeed. I don't really know why it is like it is (there
might be reasons).
Anyway, if you'd like me to change the order in that function, I can do that.
That does not particularly disqualify my approach, no?
Jürgen
