bobbylight / AutoComplete

A code completion library for Swing text components, with special support for RSyntaxTextArea.
BSD 3-Clause "New" or "Revised" License
166 stars 55 forks source link

isAutoActivateOkay() checks wrong character #77

Open DevCharly opened 4 years ago

DevCharly commented 4 years ago

isAutoActivateOkay() checks the character after the caret, but shouldn't it check the character before the caret?

https://github.com/bobbylight/AutoComplete/blob/da75dd9c0362ec49c64c6d28338ffa0e892873c8/AutoComplete/src/main/java/org/fife/ui/autocomplete/CompletionProviderBase.java#L166-L177

bobbylight commented 4 years ago

This does seem wrong, but IIRC this is called in a DocumentListener#insertUpdate() callback that occurs before the caret is notified and has its position updated. Thus tc.getCaretPosition() still returns the prior caret position (this code is also only run if the text inserted is 1 character long, so say pasting a text snippet will never trigger the auto-activation, only typing a single character or pasting a single character).

Yeah, this code is a little obtuse :)

Let me know if you have a use case where this doesn't seem to be working right. I admittedly haven't used the auto-activation feature in quite a long time.

And BTW - thanks for FlatLaf. It's really nice to have such a polished look and feel that follows the practices of the baked in lafs extending Basic. Makes it easy to understand and customize, too many Lafs roll their own infrastructure and are hard to integrate into custom components.

DevCharly commented 4 years ago

Glad you like FlatLaf 😄 Tried to make it as easy to use in existing apps as possible...

Thanks for the explanation. That makes sense.

I asked because in FlatLaf Theme Editor the caret position is already updated and CompletionProviderBase.isAutoActivateOkay() did not work.

So I've overridden isAutoActivateOkay() to check character before caret and now it works fine: FlatCompletionProvider.java

On the other hand, I've just enabled auto-activation in class AutoCompleteDemo (from the README) and there the caret is before the inserted character.

So I tried to figure out what's different and found that my app invokes TextEditorPane.load(FileLocation loc, String defaultEnc) and this changes the behavior.

Maybe loading a file, which creates a new document, changes order of document listeners?

bobbylight commented 4 years ago

Yeah, relying on the order of the listeners is pretty bad. I'll take a look at fixing things, just have to have a think about the best way to do it.

On Thu, Jul 9, 2020 at 8:39 AM Karl T notifications@github.com wrote:

Glad you like FlatLaf 😄 Tried to make it as easy to use in existing apps as possible...

Thanks for the explanation. That makes sense.

I asked because in FlatLaf Theme Editor https://github.com/JFormDesigner/FlatLaf/tree/master/flatlaf-theme-editor the caret position is already updated and CompletionProviderBase.isAutoActivateOkay() did not work.

So I've overridden isAutoActivateOkay() to check character before caret and now it works fine: FlatCompletionProvider.java https://github.com/JFormDesigner/FlatLaf/blob/c404a0d1a922cbaf289ee4b43032208f73b20cb7/flatlaf-theme-editor/src/main/java/com/formdev/flatlaf/themeeditor/FlatCompletionProvider.java#L215-L227

On the other hand, I've just enabled auto-activation in class AutoCompleteDemo (from the README) and there the caret is before the inserted character.

So I tried to figure out what's different and found that my app invokes TextEditorPane.load(FileLocation loc, String defaultEnc) https://github.com/bobbylight/RSyntaxTextArea/blob/f03e87801156d406a2e922a831213e36c4314396/RSyntaxTextArea/src/main/java/org/fife/ui/rsyntaxtextarea/TextEditorPane.java#L480 and this changes the behavior.

Maybe loading a file, which creates a new document, changes order of document listeners?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bobbylight/AutoComplete/issues/77#issuecomment-656102735, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMOMFWS7XIPMYEUCQU33KLR2W27DANCNFSM4OURXYHQ .