Kivy: setting suggestion_text crashes if textinput.text is empty string

[janssen]

Example:

import sys

from kivy.app import App
from kivy.uix.textinput import TextInput

class MyApp(App):

    def on_start(self):
        self.root.text = ""
        self.root.bind(text=lambda w, v: sys.stdout.write(v + "\n"))
        self.root.bind(suggestion_text=lambda w, v: sys.stdout.write("suggestion_text changed to: " + v + '\n'))
        self.root.suggestion_text = "foo"

    def build(self):
        return TextInput(readonly=False, multiline=False, focus=True, on_parent=lambda w, x: w.setter("focus")(True))

if __name__ == "__main__":
    MyApp().run()

and the result:

 Traceback (most recent call last):
   File "/tmp/test5.py", line 18, in <module>
     MyApp().run()
   File "/usr/lib/python2.7/site-packages/kivy/app.py", line 827, in run
     self.dispatch('on_start')
   File "kivy/_event.pyx", line 718, in kivy._event.EventDispatcher.dispatch (kivy/_event.c:7699)
   File "/tmp/test5.py", line 12, in on_start
     self.root.suggestion_text = "foo"
   File "kivy/properties.pyx", line 408, in kivy.properties.Property.__set__ (kivy/properties.c:5114)
   File "kivy/properties.pyx", line 446, in kivy.properties.Property.set (kivy/properties.c:5876)
   File "kivy/properties.pyx", line 501, in kivy.properties.Property.dispatch (kivy/properties.c:6557)
   File "kivy/_event.pyx", line 1224, in kivy._event.EventObservers.dispatch (kivy/_event.c:13497)
   File "kivy/_event.pyx", line 1130, in kivy._event.EventObservers._dispatch (kivy/_event.c:12696)
   File "/usr/lib/python2.7/site-packages/kivy/uix/textinput.py", line 2772, in on_suggestion_text
     txt = self._lines[self.cursor_row]
 IndexError: list index out of range

[janssen]

There's clearly an initialization bug and race condition here. This crashes:

import sys

from kivy.app import App
from kivy.uix.textinput import TextInput
from kivy.clock import Clock
from kivy.uix.boxlayout import BoxLayout

class MyApp(App):

    def setit(self, dt):
        if self.ti._lines:
            self.ti.suggestion_text = "foo"

    def on_start(self):
        Clock.schedule_once(self.setit)

    def build(self):
        b = BoxLayout()
        self.ti = TextInput(multiline=False, readonly=False, focus=True, on_parent=lambda w, x: w.setter("focus")(True))
        self.ti.bind(text=lambda w, v: sys.stdout.write(v + "\n"))
        self.ti.bind(suggestion_text=lambda w, v: sys.stdout.write("suggestion_text changed to: " + v + '\n'))
        b.add_widget(self.ti)
        return b

if __name__ == "__main__":
    MyApp().run()

but this doesn't (only difference is delaying the Clock callback by one second):

import sys

from kivy.app import App
from kivy.uix.textinput import TextInput
from kivy.clock import Clock
from kivy.uix.boxlayout import BoxLayout

class MyApp(App):

    def setit(self, dt):
        if self.ti._lines:
            self.ti.suggestion_text = "foo"

    def on_start(self):
        Clock.schedule_once(self.setit, 1)

    def build(self):
        b = BoxLayout()
        self.ti = TextInput(multiline=False, readonly=False, focus=True, on_parent=lambda w, x: w.setter("focus")(True))
        self.ti.bind(text=lambda w, v: sys.stdout.write(v + "\n"))
        self.ti.bind(suggestion_text=lambda w, v: sys.stdout.write("suggestion_text changed to: " + v + '\n'))
        b.add_widget(self.ti)
        return b

if __name__ == "__main__":
    MyApp().run()

[Zen-CODE]

Looking into that code, it seems easy enough to fix, but I don't see any suggestion text using 1.9.2dev? It all executes without error, but all I see is the white textbox and what I type?

[janssen]

And this one crashes, too:

import sys

from kivy.app import App
from kivy.uix.textinput import TextInput
from kivy.clock import Clock
from kivy.uix.boxlayout import BoxLayout

class MyApp(App):

    def build(self):
        b = BoxLayout()
        self.ti = TextInput(text="foo", multiline=False, readonly=False, on_parent=lambda w, x: w.setter("focus")(True), suggestion_text="bar")
        self.ti.bind(text=lambda w, v: sys.stdout.write(v + "\n"))
        self.ti.bind(suggestion_text=lambda w, v: sys.stdout.write("suggestion_text changed to: " + v + '\n'))
        b.add_widget(self.ti)
        return b

if __name__ == "__main__":
    MyApp().run()

[Zen-CODE]

So, I would like to kill this issue, but I'm not even 100% what this feature does? Do you have screenshots of how it should look? It does nothing for me even if I remove that error....

[akshayaurora]

@Zen-CODE following is a example of how suggestion_text is supposed to work.

https://gist.github.com/akshayaurora/fa5a68980af585e355668e5adce5f98b

You really are not supposed to use suggestion before the UI is setup, it does not make sense to have a suggestion even before the user has started to type.

I could add a Clock.schedule in the core code to work around this, but this in my opinion is really a programmer error.

[Zen-CODE]

Well, it's not documented, so users won't know this. As long as it's explained, I think that will be fine? Will look into that shortly.

Thanks

[janssen]

Well, if you could add a flag to a Widget so that it would know whether the "UI is set up", it would certainly help. Right now, though, there's no systematic way to know what the "UI set up" state of a Widget is, from inside the widget. One has to kind of guess. A programmer error? Or just a bad guess?

For instance, should I be able to bind to changes in "suggestion_text", as shown in my crash above? How do I remove a suggestion? Just setting suggestion_text to None, or to an empty string, also crashes it.

[Zen-CODE]

The error can easily be avoid by adding placing a check at line 2844 of TextInput.py, in 'on_suggestion_text':

    if self.canvas is not None:
        self._update_graphics()

This prevents the error, but it's still pretty pointless as no suggestion text appears. I think it's important here to establish exactly how the 'suggestion_text' should work, then the implementation should follow.

So, 'suggestion_text' displays, in an active TextInput (it does not make sense to display suggestions if the user is not trying to enter text). When it gets focus, akshayaurora questioned whether it's meaningful to display the suggestion_text? Perhaps not as an 'autocomplete', but perhaps the choice of text is 90% determined with pre-calculated data, so it does make sense? Pressing tab simply completes all the fields without typing. That's a reasonable scenario.

To give users real freedom, I think we need to allow any use that is vaguely sane. If you set the suggestion text on focus, or initially, it should just do it. Why not? The programmer should be given the choice of when it appears simply by setting it.

Do we agree on that? I think the feature is useful and nice, but don't think it should be restrictive in how it's used. Ideally, if it's set, you should see the suggestions. Simple.

Even if that is tricky to implement, I think it's more harmful to introduce a feature with unexpected requirements and behavior. Agreed?

[akshayaurora]

Even if that is tricky to implement, I think it's more harmful to introduce a feature with unexpected requirements and behavior. Agreed?

Agreed.

[janssen]

Agreed.

[Zen-CODE]

Okay. So this branch fixes the errors.

https://github.com/kivy/kivy/compare/suggestion_text_tweak

But it still does not make it work as expected. The texture is only created and added when the suggestion text changes, but then it seems removed. The original intention to only show it in response to changes reflects in the way it's built. I'll look into tweaking that so it handles static values more consistently.

@akshayaurora. If you advise against moving the late import of MarkupLabel to the top, please comment. 'Factory.MarkupLabel(...)' would have been ideal, but that class does not seem to be registered in the factory.

[Zen-CODE]

Latest commits on the above branch implement the discussed behavior. The link below shows revised code from akshays example.

https://gist.github.com/Zen-CODE/650099bd55247223986928f12df42176

I think this more predictable behavior. The only gotcha is that it does now show when there is no text. An issue? I think 'text_hint' does that, so I doubt it. Either way, it avoids errors and behaves more predictably.

Will submit a PR and add docs if we're all happy? :-)

[akshayaurora]

@Zen-CODE do not add a function call like this. this will slow down the update graphics function.

Besides the change has to be on on_suggestion_text just like with every other property.

[Zen-CODE]

Okay, this just adds enough to prevent the error: https://github.com/kivy/kivy/pull/4762

But it's still does not behave well, to my mind. It does not appear if the text is empty, or when set initially, and the words disappear erratically using the gist I posted above. I'll look into that but it's not obvious how to achieve the desired behavior given it's current implementation.

Is a function call really that much overhead? There it lot of other object lookups and code in update graphics. Why would a function call be heavy? Is it to with the cythonization/graphics instructions?

[saqib1707]

@Zen-CODE By desired behaviour , do u mean kind of feature present in sublime text(which shows a list of probable options for us to chose from). ? If yes then we can achieve using bubble option to show those probable options.

[Zen-CODE]

@saqib1707. The suggestion_text currently holds only one piece of text, not a list of probably options. Sorry, I don't use sublime text: that was a reference made in the example posted here:

https://gist.github.com/akshayaurora/fa5a68980af585e355668e5adce5f98b

[akshayaurora]

@Zen-CODE I will check @4762.

The main reason for adding this feature was to allow for IME specially on mobiles to display possible text that is currently being edited, while still is not actual part of the text.

[udiboy1209]

@akshayaurora @Zen-CODE just guessing here but shouldn't self._lines be initialized to one empty label initially? The root of this problem/race condition is that on_suggestion_text is trying to fetch the current text being typed from self._lines[cursor_row] which is an empty list initially, so even though cursor_row==0 it fails. I feel that logically it should be initialized to a list having an empty label as a single item. But I dont know if this will break something else.

[akshayaurora]

@udiboy1209 look at the rest of the code in TextInput there are many checks for self._lines being empty, setting that to a label would break TextInput in multiple places.

[janssen]

Just a usage note: I use this in a desktop app to display possible filename completions with a filename textinput.

[udiboy1209]

@akshayaurora then if the desired behavior of suggestion_text is to show even for empty text, there would be a Label (with only the suggestion text) in self._lines . That would break those checks too right? Or we can disable suggestion_text when text is empty(with a warning message to the programmer) which I think @Zen-CODE has implemented in #4762

[janssen]

The two are orthogonal; you might very well have suggestion_text even if text is an empty string.

[akshayaurora]

That is the point showing suggestion text for when text input is empty does not make any sense to me.

You should be using hint_text for that. Suggestion text should not work when text input is empty or not on screen.

[akshayaurora]

closed by #4762

[janssen]

I'm not sure why it doesn't make sense to you, but perhaps I can explain. "hint_text" only shows when a TextInput is not focussed. It is typically used to describe what the TextInput is for, like "(type filename here)". "suggestion_text", on the other hand, shows whether or not the TextInput is focussed (perhaps it should only show when it is focussed), and is a suggestion about what to type, like "/". They are two completely different things, and you should never use one for the other.

[Zen-CODE]

@janssen. Totally agree. Also, I think it's important features are not limited to pre-conceived use-cases. A feature should do what it says. Ideally, it should not depend on context/timing/configuration (yes, that's an often-lofty ideal). Programmers should determine how and when it's used. That's fundamental to allowing innovative interfaces.

[dessant]

@Zen-CODE, @janssen suggestion_text was merged with an unrelated pr, apparently without feedback.
It could be beneficial to open a new issue and discuss/agree upon a better design, if you guys have some ideas about how the api should behave.

https://github.com/kivy/kivy/commit/be97eeae96bbb17ebd85ddf4e4d738033eccfbdd#diff-f6433d5683536d9ce43c2441a159250eR2482

[Zen-CODE]

Well, the PR that prevents the crashes was merged. https://github.com/kivy/kivy/pull/4762

So this ticket can properly be closed. The desired behaviour should probably discussed in a new ticket?

[dessant]

@Zen-CODE yeah