Editor changes text after pressing Enter

[Former user]

Originally reported on Google Code with ID 76

What steps will reproduce the problem?
1. 1/sin pi
2. 1/sin pi
3.

What is the expected output? What do you see instead?
After the first step, the input field shows a highlighted "1/sin 0". After
repeating this command I get "1/sin sin", which is odd

What version of the product are you using? On what operating system?
0.8-alpha

Please provide any additional information below.

Reported by [e-mail] on 2007-07-18 21:58:03

[Former user]

``` I just tried to reproduce it, but I couldn't. May be I got the text from the history. ```

Reported by `[e-mail]` on 2007-07-18 22:09:22

[Former user]

``` Me neither :) ```

Reported by `helder.pereira.correia` on 2007-07-18 22:11:39 - Status changed: `Invalid`

[Former user]

``` As reported by Wolf, it happens when doin exactly the following:

p i return 1 / 0 return arrow right backspace s i n space p i return ```

Reported by `helder.pereira.correia` on 2007-07-18 22:42:34 - Status changed: `Accepted`

[Former user]

``` Actually, just type:

(be fast so no popup completion shows) 1 + sin SPACE pi ENTER

All ok...

Now type the same but:

1 + sin (WAIT-FOR-POPUP) pi ENTER

From now on everything will be messed. So I conclude the timer is doing something wrong. It happens with any function and changes the last term after pressing Enter. ```

Reported by `helder.pereira.correia` on 2007-07-22 23:35:14 - Labels added: Milestone-0.8

[Former user]

``` P.S.: if last term is a constant, everything is just fine. ```

Reported by `helder.pereira.correia` on 2007-07-22 23:43:38

[Former user]

``` I think I've solved it. I commented the line below and it seems to work. I've played around a lot and saw no side effects but it needs more testing to be sure nothing was broken.

void Editor::stopAutoComplete() { d->completionTimer->stop(); - d->completion->doneCompletion(); + d->completion->doneCompletion(); setFocus(); } ```

Reported by `helder.pereira.correia` on 2007-07-23 01:10:09

[Former user]

``` I have not done any QT programming so far, and I'm not familiar with code of SpeedCrunch outside the low level routines. So take the following with care.

If I understand the commented out "doneCompletion" right, it does three things: 1. starts a timer that triggers the fader after 750 ms / hides the popup box 2. sets the focus of the editor field 3. 'emits' (?, never seen that keyword before in C++) the selected item from the popup box. grep shows, that the only callers of stopAutoComplete are in crunch.cpp in function "applySettings" and "returnPressed". doneCompletion is called from several places, so touching this function can be hazardous. So, how is "doneCompletion" of use, when the user hits "return"? It hides the popup box. Is it possible that the popup box is showing when return is hit? Usually it is hidden, because most function calls require an argument, but (a) the user might have entered something wrong like "1+sin", (b) in future there might be functions like "random" that take no arguments. The setting of the focus is done in stopAutoComplete anyway, so this effect of doneCompletion can safely be skipped. And the inserting is faulting, that's why the function is eliminated. Now the next context "applySettings". Is the now missing hiding of the box necessary? The inserting the item from the popup box? At first sight, I would say not. But this has to be checked out. So, to be on the save side, I would copy the hiding portion from doneCompletion to stopAutoComplete to preserve this effect.

Wolf Lammen ```

Reported by `[e-mail]` on 2007-07-23 07:11:06

[Former user]

``` Or, even better, write a function "hidePopup", that gets the hiding code, and call this function in "doneCompletion" and "autoStopCompletion". This way you avoid duplicating of code. Wolf Lammen ```

Reported by `[e-mail]` on 2007-07-23 07:28:19

[Former user]

``` Here is my patch for that. Basically it clears the selected item first before calling doneCompletion. The impact should be very minimum. What do you think?

Index: editor.cpp

--- editor.cpp (revision 599) +++ editor.cpp (working copy) @@ -729,7 +729,8 @@ void Editor::stopAutoComplete() { d->completionTimer->stop(); - d->completion->doneCompletion(); + d->completion->selectItem(QString()); + d->completion->doneCompletion(); setFocus(); }

@@ -869,6 +870,18 @@ d->popup->show(); }

+void EditorCompletion::selectItem( const QString& item ) +{ + if( item.isNull() ) + d->popup->setCurrentItem( 0 ); + else + { + QList<QTreeWidgetItem*> targets = d->popup->findItems( item, Qt::MatchExactly ); + if( targets.count() > 0 ) + d->popup->setCurrentItem( targets[0] ); + } +} + void EditorCompletion::fade( int v ) { d->popup->setWindowOpacity( (qreal)(100-v)/100 ); Index: editor.h

--- editor.h (revision 599) +++ editor.h (working copy) @@ -136,6 +136,7 @@ void selectedCompletion( const QString& item );

public slots: + void selectItem( const QString& item ); void doneCompletion();

private slots:

```

Reported by `ariya.hidayat` on 2007-07-23 07:56:13

[Former user]

``` OK as a workaround, but actually you use a trick, because you fool doneCompletion into doing something without any effect. What about making your intention explicit? Add a parameter to doneCompletion like: doneCompletion(bool keepText), and use an if clause to circumvent the faulting statements. The parameter is true in stopAutoComplete, and false elsewhere. Wolf Lammen ```

Reported by `[e-mail]` on 2007-07-23 21:59:53

[Former user]

``` Going for Ariya's patch for now.

Fixed in branches/0.8, will be available in trunk later. ```

Reported by `helder.pereira.correia` on 2007-07-23 22:31:51 - Status changed: `Fixed`

[Former user]

``` IMHO it's not necessary to make it more complicated e.g. with extra bool parameter (which is, by itself, best avoided, google for "Designing Qt-Style C++ APIs").

Actually, it looks less like a workaround if you think that doneCompletion's job is to "close the automatic completion; if an item in the pop-up is selected, then replace the current selection with that item".

```

Reported by `ariya.hidayat` on 2007-07-24 06:05:33

[Former user]

``` Yep, if that's the meaning of 'doneCompletion', I agree. Wolf Lammen ```

Reported by `[e-mail]` on 2007-07-24 06:15:34