expression evaluation broken on some GTK installations

[Thomas Pointhuber]

It seems expression evaluation does no longer work in nightlies. Entering something like 5+2 for via position does not work anymore.

Application: kicad
Version: (6.0.0-rc1-dev-562-g969e85daa), debug build
Libraries:
    wxWidgets 3.1.1
    libcurl/7.61.1 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.4) nghttp2/1.33.0
Platform: Linux 4.18.8-arch1-1-ARCH x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.1.1 (wchar_t,wx containers) GTK+ 2.24
    Boost: 1.67.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.61.1
    Compiler: Clang 6.0.1 with C++ ABI 1002

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Jeff Young (jeyjey)]

This works fine for me on OSX (tried Via Properties: Position X: and Text Properties: Width:).

Can anyone else reproduce it?

[eelik (eelik)]

Works for me. The new value is calculated and shown when I move to another field or when I click OK.

Application: kicad
Version: (6.0.0-rc1-dev-576-gae13e441a), debug build
Libraries:
    wxWidgets 3.0.3
Platform: Linux 4.13.0-46-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.3 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.62.0
    Compiler: GCC 7.2.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=OFF
    KICAD_USE_OCE=OFF
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

[Michael Geselbracht (mgeselbracht)]

Does not work for me. It stopped working when the UNIT_BINDER was added to the dialogs.
I thought it was work in progress and the evaluation was simply missing. But it does work with a nightly build on Windows 10.

Application: kicad
Version: (6.0.0-rc1-dev-576-gae13e441a), debug build
Libraries:
    wxWidgets 3.0.3
    libcurl/7.47.0 OpenSSL/1.0.2g zlib/1.2.8 libidn/1.32 librtmp/2.3
Platform: Linux 4.15.0-34-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.3 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.58.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.47.0
    Compiler: GCC 5.4.0 with C++ ABI 1009

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Jeff Young (jeyjey)]

Do we know anything more about this?

[Seth Hillbrand (sethh)]

I can confirm that the new value is not calculated when I move to another field on Debian 9. It is calculated when I click OK.

[Jeff Young (jeyjey)]

@Thomas, @Michael, can you confirm that the values are indeed evaluated on OK?

(And if anyone gets a chance to put a breakpoint in void UNIT_BINDER::onKillFocus(), it'd be interesting to know if the evaluation isn't getting done when you leave the field, or if the control just isn't getting refreshed with the new value.)

[Thomas Pointhuber (pointhi)]

I fixed the bug locally by changing the following line in UNIT_BINDER::onKillFocus( wxFocusEvent& aEvent ):

```
if( !aEvent.GetWindow() || aEvent.GetWindow()->GetId() == wxID_CANCEL )
```

to

```
if( aEvent.GetWindow() && aEvent.GetWindow()->GetId() == wxID_CANCEL )
```

This fixed the evaluation in general, but a regression was visible: entering a value and then pressing Enter skipped the evaluation. A user had to explicitly press OK with the Mouse or set focus to another object.

[Jeff Young (jeyjey)]

OK, so it's getting confused on the in-the-middle-of-destruction guard. Let me look into it....

[Jeff Young (jeyjey)]

I can't remember under what conditions that guard was necessary, and after removing it to try and find out I'm unable to get any bad behaviour. It's possible whatever it was guarding is no longer an issue due to control flow changes elsewhere, but if not I'm sure whatever it was will get flushed out.

@Thomas, if you get a chance to test out the new code that would be great.

[Thomas Pointhuber (pointhi)]

@Jeff, seems to work

Issues I noticed:

* Enter does not result in the evaluation of the expression (as noted before)
* when I remember correctly when refocusing on the field, the expression reappeared (instead of staying on the calculated value)

[Michael Geselbracht (mgeselbracht)]

@Jeff
A BP in UNIT_BINDER::onKillFocus() is hit when I press <TAB> in the "Move Exactly" and FP "Properties" dialogs.
Clicking "OK" or hitting <ENTER> evaluates an expression in the "Move Exactly" dialog but not in a FP properties dialog (tested with "Position X" field).

BTW: In the meantime I have upgraded my OS from "KDE Neon" 16.04 to 18.04. It does not seem to have any effect on this issue.

Application: pcbnew
Version: (6.0.0-rc1-dev-689-g9e828e0d9), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.58.0 OpenSSL/1.1.0g zlib/1.2.11 libidn2/2.0.4 libpsl/0.19.1 (+libidn2/2.0.4) nghttp2/1.30.0 librtmp/2.3
Platform: Linux 4.15.0-36-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.65.1
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.58.0
    Compiler: GCC 7.3.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

[Jeff Young (jeyjey)]

@Thomas, I'm not following you. By "enter" do you mean an enter that closes the dialog? (I don't actually have an <enter> key on my keyboard, only <return>, so perhaps there is some other action you're talking about.)

The lack of the expression coming back when re-focusing is a different bug (which reproduces on all platforms). I'll have a fix for that shortly....

@Michael, thanks for that. Thomas' previous message got me going down the right track, although the <enter> issue might be something different.

[KiCad Janitor (kicad-janitor)]

Fixed in revision 5ce14dad2a109815de02de3ad6043b26ec60877c
https://git.launchpad.net/kicad/patch/?id=5ce14dad2a109815de02de3ad6043b26ec60877c

[Michael Geselbracht (mgeselbracht)]

Expression evaluation works for me now when I press the <TAB> key.

Changing/entering an expression followed by <ENTER> (no <TAB>) is working in the "Move Exactly" dialog but not in FP Properties dialog.

[Thomas Pointhuber (pointhi)]

Ok, I was talking about the return key. I always called it enter ^^

[Jeff Young (jeyjey)]

@Thomas & Michael, I've merged a fix for replacing the original (un-evaled) text when clicking back in the field.

Can you guys tell me if enter is still failing in the FP Properties dialog? (If so, does a breakpoint in UNIT_BINDER::onKillFocus() get hit?)

[Michael Geselbracht (mgeselbracht)]

Text is replaced like it is in 5.0 branch. Evaluation and <ENTER> is a bit strange.
I tested the feature in the properties dialog by appending "+1" to the position ("148.766+1"). This is not evaluated when I press enter (but it is with <TAB>).
But the evaluation is working with a different input ("1+2+3"). It seems that the beginning of the entered string is compared with the old one in order to detect a changed input.
So if the position is "148" and I change it to "148.1+1" it is working while "148+1" is not. Perhaps there is a test using atoi() ?

The BP is hit when I press enter.

[Jeff Young (jeyjey)]

@Michael, when you press <ENTER> does it close the dialog on your platform? Or is it more like pressing <TAB>?

[Michael Geselbracht (mgeselbracht)]

<ENTER> closes the dialog. If an expression is recognized the FP moves otherwise it stays where it is. When I re-open the dialog in the latter case the old values are displayed.

[Jeff Young (jeyjey)]

Interesting. I *think* it's a platform difference regarding the order of the KILL_FOCUS event and the TransferDataFromWindow() call.

New code to come shortly....

[KiCad Janitor (kicad-janitor)]

Fixed in revision 3024ded91ea3b4aae1f214654d3df963cfcb597a
https://git.launchpad.net/kicad/patch/?id=3024ded91ea3b4aae1f214654d3df963cfcb597a