On 03/23/2011 02:15 PM, Stephan Witt wrote:
Am 23.03.2011 um 13:56 schrieb Abdelrazak Younes:

On 03/23/2011 12:48 PM, Stephan Witt wrote:
Am 23.03.2011 um 10:18 schrieb Abdelrazak Younes:

On 03/22/2011 08:20 AM, Stephan Witt wrote:
Am 21.03.2011 um 18:00 schrieb Stephan Witt:

Am 21.03.2011 um 09:43 schrieb Abdelrazak Younes:

On 03/21/2011 07:30 AM, Stephan Witt wrote:
Am 20.03.2011 um 17:50 schrieb Abdelrazak Younes:

Hi Pavel,

Here is a patch that should fixes #7375. The patch is big but this is mostly 
mechanical changes needed for a proper Dock widget and code shuffling:

* Transform Spellchecker.ui into a simple QWidget
* Pimpl all we can
* Remove Close button (as for all other dock widgets)
* Make the dialog buffer independent (as for all other dock widgets)

Please give this patch a try and tell me if it's OK to commit. There are still 
quite some bugs for this dialog that I will try to address afterwards.
I tried your first patch and it is an improvement, thanks.
The dock widget obviously is the problem for the broken push button placement 
too!
I'll test your second patch too, later today.
Please test the latest (3rd?) patch instead.
I did test it a little. It looks sane - but I've some strange effect:
sometimes the "ignore" button stops working, I have to switch between windows
to get it working again. Debugging it is tricky because it works when
it stops at some breakpoint and it doesn't happen all the time...
... it happens if inside some inset (table cell, note or caption).

I have to check if it's gone if I revert your patch...
Ok, the method to reproduce the problem is:
* open Embedded manual
* place the cursor inside the paragraph above the table in
   sub-section 2.6.2 Longtable alignment
* start spell checker with F7
* hit ignore until the first cell with "asd" is reached
* here the speller is stuck

Without your patch it doesn't happen. The first reason why
this is different may be the "missing" progress bar.
Are you able to reproduce "my" problem? If not I can try
to split your patch into two parts to find the culprit.
I think I know what's going on, this is because the "find next" function is 
triggered by the updateView() mechanism which is very bad.
Yes, this problem I saw already too.
But I'm not sure it's the root cause.
In forward() we have (without your patch):
=================
        DocIterator from = bufferview()->cursor();

        dispatch(FuncRequest(LFUN_ESCAPE));
        dispatch(FuncRequest(LFUN_CHAR_FORWARD));
        d->stuck_ = from == bufferview()->cursor();
=================
Normally it should never stuck, perhaps when at end-of-document...
But it happens with the recipe I mentioned above in the middle of the document
inside an inset and I cannot tell why.
So this means that this stuck_ variable was useful for some dark corner case it 
seems... I removed it because I didn't see how and why it is useful. So if the 
cursor is stuck here this means that there's a problem in LFUN_CHAR_FORWARD 
that we should fix instead.
Yes.

For end-of-document I'd call it normal when LFUN_CHAR_FORWARD does not do 
anything.
That was the original goal of the stuck_ variable - to catch the case when the
last word is misspelled and you hit the ignore button.

Well, there is also an explicit check for end of document (which I kept) so it's not clear if that was the goal...

Anyway, I have 1.6 here and when 'asd' is found and selected, instead of hitting ignore I called 'char-forward' in the minibuffer. The effect was to free the selection only but not to move the cursor onto the next cell. In normal text, the selection is freed and the cursor _is_ moved. So two fixes are possible:

1) Make 'char-forward' move to next cell (preferred)
2) call forward() again if cursor is stuck (without the need for the stuck_ member):

        DocIterator from = bufferview()->cursor();

        dispatch(FuncRequest(LFUN_ESCAPE));
        dispatch(FuncRequest(LFUN_CHAR_FORWARD));
        if (from == bufferview()->cursor()) {
                //FIXME we must be at the end of a cell
                dispatch(FuncRequest(LFUN_CHAR_FORWARD));
        }


To clarify what I did:
* I applied your patch and
* added some code like the stuck check to catch the "impossible" situation.
   As I already said - the debugger isn't helpful here since it causes a focus
   switch and then the problem isn't there anymore. Perhaps one can catch it
   with two displays - one for LyX and another for gdb.

OK, so maybe you should commit that for now so that the next release candidate is in a better shape.

Abdel.

Reply via email to