New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TextCompleterSimple still requires the "Start" and "GetNext" method to be overriden. #836

Closed
RobinD42 opened this Issue Apr 30, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@RobinD42
Member

RobinD42 commented Apr 30, 2018

Operating system:any
wxPython version:4.02
Stock or custom build:any
Python version:any
Stock or custom build:any

Description of the problem:

https://groups.google.com/d/msg/wxpython-users/PSwmwPI4itA/cTZ5BuDJAwAJ

@swt2c

This comment has been minimized.

Contributor

swt2c commented Apr 30, 2018

I would think that the test case I added would expose this but it seems that it does not.

@RobinD42

This comment has been minimized.

Member

RobinD42 commented Apr 30, 2018

Yeah, I was also wondering about why it didn't show up there.

@swt2c

This comment has been minimized.

Contributor

swt2c commented May 1, 2018

Okay, I think I figured out why I didn't see any problems when running the unittest - wxTextEntry::AutoComplete(wxTextCompleter*) isn't implemented for GTK in wx 3.0. I still would think that SIP would flag the error just when instantiating the class, but it seems not.

I think that all that needs to be done here is Wig in the Start() and GetNext() method declarations since they're not declared in the interface header.

@RobinD42

This comment has been minimized.

Member

RobinD42 commented May 1, 2018

Oh, right. Classes that are marked as abstract get an exception when constructed, but those that aren't only raise the exception when the base class virtual is called without having been overridden in Python.

I think that all that needs to be done here is Wig in the Start() and GetNext() method declarations since they're not declared in the interface header.

Or just copy the MethodDefs from the base ClassDef. 😃 (I wish I had thought of doing it this way a long time ago...)

@XPsoud

This comment has been minimized.

Contributor

XPsoud commented May 1, 2018

Well, this doesn't seems to be enough...
Start and GetNext methods are not required anymore, but the GetCompletions one does nothing (tested with dev3733-9f056497)

Attached is a small app showing the use of both TextCompleter (witch works fine) and TextCompleterSimple (witch doesn't works).

And as a complement of informations : if the Start and GetNext methods of TextCompleterSimple are overriden, it works like the TextCompleter class (you can try copy/paste these methods from one class to the other to see)

AutoComplete.zip

@RobinD42

This comment has been minimized.

Member

RobinD42 commented May 1, 2018

Ok, thanks for the sample. I'll work on a fix soonish.

@RobinD42

This comment has been minimized.

Member

RobinD42 commented May 2, 2018

Fixed by #840

@RobinD42 RobinD42 closed this May 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment