Skip to content
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

fix: Don't use QCompleter prefix filter #4855

Merged

Conversation

dnsge
Copy link
Contributor

@dnsge dnsge commented Oct 1, 2023

Description

Toggling the "Only search for emote autocompletion at the start of emote names" would require a user to close and reopen a tab for the change to take effect.

The overhaul put the onus of determining completions completely on the Strategy selected. We don't want to limit ourselves to only completing prefixes.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/widgets/helper/ResizingTextEdit.cpp Show resolved Hide resolved
@Felanbird
Copy link
Collaborator

Felanbird commented Oct 1, 2023

Bugfix: - Fixed an issue where the setting `Only search for emote autocompletion at the start of emote names` wouldn't disable if it was enabled when the client started. (#4855)

issue could have been* resolved on a per-channel basis without a client restart, but seems like this better explains the bug

@Felanbird
Copy link
Collaborator

Felanbird commented Oct 1, 2023

@pajlada current problem is as follows:

When the client is started with the setting Only search for emote autocompletion at the start of emote names enabled(default), disabling it does not update the completion model, the tab itself must be closed & re-opened or the entire client restarted.
When the client is started with this setting disabled keeping it disabled and enabling it both work exactly as expected

The changes in this PR resolve the first problem.

but, this setting also effects commands see #3440

on 06eb30a (before any input completion overhaul):
Enabled on startup = pains -> pains | popo -> popo
Enabled on startup & disabled | pains -> pains | popo -> popo
Disabled on startup = pains -> Pajapains | popo -> /popout
Disabled on startup & re-enabled = both emotes & commands are properly limited pains -> pains | popo -> popo

on 4d8b623: | tested #4853 as well, no change
Enabled on startup = pains -> pains | popo -> popo
Enabled on startup & disabled | pains -> pains | popo -> popo
Disabled on startup = pains -> Pajapains | popo -> /popout
Disabled on startup & re-enabled = emote completion respects the setting, commands do not pains -> pains | popo -> /popout

on this PR :
Enabled on startup = pains -> pains | popo -> /popout
Enabled on startup & disabled = pains -> Pajapains | popo -> /popout
Disabled on startup = pains -> Pajapains | popo -> /popout
Disabled on startup & enabled = pains -> pains | popo -> /popout

So this PR fixes the issue with commands, but does not fix the problem with commands not respecting the setting, though only partially caused by this PR

Problem likely stems from the way commands are handled in 4d8b623
image

@dnsge dnsge changed the title fix: Invalidate tab completion model when settings change fix: Don't use QCompleter prefix filter Oct 13, 2023
brian6932

This comment was marked as outdated.

@pajlada pajlada merged commit 653a14c into Chatterino:master Oct 13, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants