urbit / talk

Urbit notifications and chat module
10 stars 7 forks source link

Tab autocompletion for ~ship names. #24

Closed Fang- closed 8 years ago

Fang- commented 8 years ago

Implements on-tab autocompletion of ship names (~ followed by zero or more characters in a valid short ship-name pattern). Press tab again to cycle forward through possible matches. Shift-tab cycles backwards. Matches are ordered "most recently spoke" first.

This is accomplished by, on first tab-press, going over all recent messages to find matching ship names, followed by all station members, compressing moon and comet names into their short versions as needed. This seems performant enough to deliver instantaneous results even for the busiest stations, though I haven't been able to test this on a lower-end machine. The list of matches is discarded when any key other than tab is pressed.

Do note that this only autocompletes text from the last ~ in your message up to the end of it, if it can be a valid ship name. So: hello ~tor autocompletes, hello ~tor! doesn't. hello ~ autocompletes, matching all ships. This should correspond to the expected behavior.

Ideally you may want this implemented in :talk (so the CLI version can also enjoy tab completion), and have this web client ask it for autocomplete matches.

I didn't encounter any code comments in existing code, so I refrained from adding those. Hopefully my additions are straight-forward enough, but I'll gladly add some comments if needed.

Let me know if this is up to standard, any and all feedback is appreciated!

cgyarvin commented 8 years ago

Wow.

On Fri, Jul 1, 2016 at 3:20 AM, Fang notifications@github.com wrote:

Implements on-tab autocompletion of ship names (~ followed by zero or more characters in a valid short ship-name pattern). Press tab again to cycle forward through possible matches. Shift-tab cycles backwards. Matches are ordered "most recently spoke" first.

This is accomplished by, on first tab-press, going over all recent messages to find matching ship names, followed by all station members, compressing moon and comet names into their short versions as needed. This seems performant enough to deliver instantaneous results even for the busiest stations, though I haven't been able to test this on a lower-end machine. The list of matches is discarded when any key other than tab is pressed.

Do note that this only autocompletes text from the last ~ in your message up to the end of it, if it can be a valid ship name. So: hello ~tor autocompletes, hello ~tor! doesn't. hello ~ autocompletes, matching all ships. This should correspond to the expected behavior.

Ideally you may want this implemented in :talk (so the CLI version can also enjoy tab completion), and have this web client ask it for autocomplete matches.

I didn't encounter any code comments in existing code, so I refrained from adding those. Hopefully my additions are straight-forward enough, but I'll gladly add some comments if needed.

Let me know if this is up to standard, any and all feedback is appreciated!

You can view, comment on, or merge this pull request online at:

https://github.com/urbit/talk/pull/24 Commit Summary

  • Implemented naive ship name autocompletion when pressing tab.
  • Tab completion now picks the first match based on most recent chat message, rather than station join order.
  • Autocompletion now includes both names from the most recent messages and the station members list (in that order). Repeatedly hitting tab advances through potential matches, shift-tab goes backwards. Matches get discarded on non-tab keypress.
  • To support autocompleting moon and comet names as they appear in chat (shortened to four syllables), full names now get shortened before checking if they need to be included in the autocomplete list.

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/urbit/talk/pull/24, or mute the thread https://github.com/notifications/unsubscribe/AALyAaI3E5xdkptJ-APkgdWue7fsFejOks5qROnugaJpZM4JDA89 .

galenwp commented 8 years ago

This looks good to me. @ohAitch is the most opinionated about JS architecture stuff. @ohAitch would you restructure this? Maybe it should be a utility method. It could also be used in setting your audience.

ohAitch commented 8 years ago

I'd maybe move the MessageStore.getAll() to stateFromStore. "Lazily-constructed traversable ship autocomplete that gets cleared on UI actions" as a whole might make sense as a Store entity, but it seems reasonable to stick it in Writing state as long as that's the only place it's used. And yeah, "copy-paste to Audience, generalize" belongs solidly on a wish-list somewhere, perhaps next to "recognize audience glyphs".

stamps approval, apologizing for delay

galenwp commented 8 years ago

It would be cool to implement this for the audience too. Thing is — the audience doesn't use the ~. But maybe it should. I took that out mostly for style reasons. Fang, any interest in working on that?

galenwp commented 8 years ago

If we did it in the audience the tab completion could turn ~rap into raplys-lornex/porch for example (or the closest room for that ship).

Fang- commented 8 years ago

Sure, I can take a look at that sometime soon.