Added username completion in channels #93

Tangdongle · 2023-07-04T07:51:31Z

This adds nickname auto-completion for channels.

When we instantiate the text_input from the Channel's view with a list of users in that channel.

From there, we need to differentiate between a /command and a user completion, so we add a CompletionType to each Completion Entry. /commands only auto-complete from the start of the input text, whereas name completion can occur from any word.

Finally, we replace the current word being completed without sending, so the user can continue to type.

src/widget/input/completion.rs

tarkah · 2023-07-04T15:28:27Z

src/widget/input/completion.rs

+ pub fn process(&mut self, input: &str, users: Vec<User>) {
+ self.with_users(users);
+ let maybe_command = input.starts_with('/');
+ let Some((head, rest)) = Self::valid_completion_input(input, maybe_command) else {
 self.reset();
 return;


Let's add some better separation between processing for a command vs user completion. If input starts with a slash, we can branch into a process_command function, otherwise process_user.

Then we only need to add the users info inside process_user compared to us always adding it here

src/widget/input.rs

src/widget/input/completion.rs

Tangdongle · 2023-07-05T00:59:30Z

Awesome, thanks for the feedback, I'll have a look at these today

tarkah · 2023-07-05T23:41:57Z

@Tangdongle It's looking good! I've pushed a few commits to get it over the finish line, let me know what you think.

I've cleaned up some of the types to eliminate some impossible states and streamline some things
User completions are now only triggered if @ is typed. This is because normal typing becomes extremely annoying when the completions list is constantly popping up for any letter. I'm open to suggestions on better alternatives to trigger user completions, but this feels fairly standard.
I've limited the completion popup to a sliding window of 5 entries. Since many user entries can show up, the popup sometimes would get way too long. Tabbing shifts this window as expected.

Tangdongle · 2023-07-06T01:25:39Z

@tarkah Legendary! Thanks for cleaning that up, hope it wasn't too much of a hassle!

Yeah, I see what you've done here, looks much cleaner. I've taken it for a spin and it looks good to me!

tarkah · 2023-07-06T01:30:29Z

Awesome! Not at all, you put it in a really nice spot so it all came together nicely 👌

I'm going to hold off on merging until @casperstorm can test. I'm not sure I love @ as the trigger in IRC so I want his feedback on the best UX for this.

hecrj · 2023-07-06T04:18:06Z

src/widget/input.rs

@@ -51,6 +52,7 @@ pub enum Event {
 pub struct Input<'a, Message> {
 id: Id,
 buffer: Buffer,
+ users: Vec<User>,


Could this borrow a slice to avoid the clone in view?

Suggested change

users: Vec<User>,

users: &'a [User],

Unfortunately not. We're currently depending on the IRC crate for managing the user list per channel and it doesn't expose a way to get a reference to this data.

Ideally we manage channels and users ourselves so we could make this optimization. It'd definitely benefit us in the long run as we have better control over our data.

A middle ground solution would be to extract channel and user lists after receiving each batch of IRC messages which can be stored in Connection. This will ensure they're in sync but give us this level of control.

Yup, just took a look at the irc crate. A bit unfortunate it relies heavily on interior mutation :/

I've created a tracking issue for this here:

#102

Fixed in 3a54355 after rebasing to #104

casperstorm · 2023-07-06T06:51:32Z

I'm going to hold off on merging until @casperstorm can test. I'm not sure I love @ as the trigger in IRC so I want his feedback on the best UX for this.

I think we need to rethink the UX slightly, it not that irc idiomatic. The core functionality however is spot on.
Reason its nice to have completions list for / commands is that it also serves as a simple documentation for users. IRC commands can be a little awkward to remember. However, for highlighting a nick I don't think we need a completions list since you already know what (who) you are looking for, as soon as you start writing.

Because of this, the most common practise is to be able to trigger at all times when pressing Tab (without any prefix). This also means that we save a few clicks. Currently in this PR if i want to highlight tarkah I would do:

@
ta
Tab
Enter

My suggestion would be a system where when pressing Tab we try to match a nick, and pressing Tab again cycles, without displaying our completion list. Take the following scenario as an example: We have a irc channel with three users: [tarkah, tarsnap, casper].
Starting with a empty prompt:

hiSpace
a. prompt: [hi ]
t
a. prompt: [hi t]
tab
a. prompt: [hi tarkah]
tab
a. prompt: [hi tarsnap]

Tangdongle · 2023-07-06T08:31:37Z

Ok, I got a little something going on, lemme know what you think. Seems to work well enough from my tests (i.e. "it works on my machine").

did notice that if you have text in the input buffer and then open a new channel, the text carries across to the newly opened channels buffer. Doesn't seem related to the change, so I'll raise a bug for it.

tarkah · 2023-07-06T15:33:13Z

@Tangdongle I ended up pushing a larger refactor since the previous implementation didn't really jive with the new requirements and we were introducing a lot of impossible states.

The new implementation separates user / command completion logic and should be much simpler to follow. We also can get user completions while showing the command popup for the selected command! Great for /msg and the like :D

Thanks so much for tackling this, it was a huge help!

Removed some heap allocations. Split up how we process command vs user completions

casperstorm

This is SUCH a great feature.
Thanks for the work @Tangdongle and @tarkah.

casperstorm · 2023-07-06T17:47:52Z

src/widget/input/completion.rs

+ let nickname = user.nickname();
+ nickname


A tiny thing could be that we lower-cased both rest and nickname so we can tab-complete from t to Tarkah.

Not blocking but just an idea.

Good idea! 09c9e78

casperstorm · 2023-07-06T17:49:58Z

src/widget/input/completion.rs

+ }
+}
+
+static COMMAND_LIST: Lazy<Vec<Command>> = Lazy::new(|| {


I like this 👍🏻
Perhaps some macro magic in the future.

CHANGELOG.md

Co-authored-by: Casper Rogild Storm <casper+github@rogildstorm.com>

Tangdongle mentioned this pull request Jul 4, 2023

User name autocomplete #69

Closed

tarkah reviewed Jul 4, 2023

View reviewed changes

src/widget/input/completion.rs Outdated Show resolved Hide resolved

tarkah reviewed Jul 4, 2023

View reviewed changes

src/widget/input.rs Outdated Show resolved Hide resolved

tarkah reviewed Jul 4, 2023

View reviewed changes

src/widget/input/completion.rs Outdated Show resolved Hide resolved

Tangdongle force-pushed the add_username_completion branch 3 times, most recently from d8a421e to c868590 Compare July 5, 2023 08:33

tarkah force-pushed the add_username_completion branch from b13f97e to c99c01d Compare July 5, 2023 23:23

tarkah approved these changes Jul 5, 2023

View reviewed changes

tarkah force-pushed the add_username_completion branch from 0c99d01 to 1820448 Compare July 5, 2023 23:51

hecrj reviewed Jul 6, 2023

View reviewed changes

tarkah force-pushed the add_username_completion branch 2 times, most recently from 5b58711 to d2ca1c5 Compare July 6, 2023 15:35

Ryan C and others added 9 commits July 6, 2023 10:18

Added username completion in channels

d0a6ae5

Stopped memory leak.

6781317

Removed some heap allocations. Split up how we process command vs user completions

Cleanup completion types

31eea1e

Only trigger w/ @

842d968

Limit to showing 5 completion entries

0c9840c

Update comments

b2b501f

Remove allocation on non-matched users

81c6284

User completion no longer shows highlight

aaa6236

Refactor completion to better support command & user

3732008

Use users slice - avoid allocation

3a54355

tarkah force-pushed the add_username_completion branch from d2ca1c5 to 3a54355 Compare July 6, 2023 17:20

tarkah changed the base branch from main to feat/optimize-channel-lists July 6, 2023 17:20

Update changelog

33d1949

casperstorm approved these changes Jul 6, 2023

View reviewed changes

tarkah and others added 2 commits July 6, 2023 10:53

Update CHANGELOG.md

5c37a84

Co-authored-by: Casper Rogild Storm <casper+github@rogildstorm.com>

Make case insensitive

09c9e78

tarkah merged commit b705237 into squidowl:feat/optimize-channel-lists Jul 6, 2023
1 check passed

ineeee mentioned this pull request Jul 9, 2023

feature request: channel completion #138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added username completion in channels #93

Added username completion in channels #93

Tangdongle commented Jul 4, 2023

tarkah Jul 4, 2023

Tangdongle commented Jul 5, 2023

tarkah commented Jul 5, 2023

Tangdongle commented Jul 6, 2023

tarkah commented Jul 6, 2023

hecrj Jul 6, 2023

tarkah Jul 6, 2023

tarkah Jul 6, 2023 •

edited

Loading

hecrj Jul 6, 2023

tarkah Jul 6, 2023 •

edited

Loading

tarkah Jul 6, 2023

casperstorm commented Jul 6, 2023

Tangdongle commented Jul 6, 2023

tarkah commented Jul 6, 2023

casperstorm left a comment

casperstorm Jul 6, 2023

tarkah Jul 6, 2023

casperstorm Jul 6, 2023

Added username completion in channels #93

Added username completion in channels #93

Conversation

Tangdongle commented Jul 4, 2023

Choose a reason for hiding this comment

Tangdongle commented Jul 5, 2023

tarkah commented Jul 5, 2023

Tangdongle commented Jul 6, 2023

tarkah commented Jul 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarkah Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarkah Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

casperstorm commented Jul 6, 2023

Tangdongle commented Jul 6, 2023

tarkah commented Jul 6, 2023

casperstorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarkah Jul 6, 2023 •

edited

Loading

tarkah Jul 6, 2023 •

edited

Loading