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

Chat messages #84

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Chat messages #84

wants to merge 10 commits into from

Conversation

camdez
Copy link
Contributor

@camdez camdez commented May 4, 2015

Work in progress.

Adds inbound (#76) + outbound message handling.

Outbound messages can be sent using the floobits-send-message command. Inbound messages will be displayed in the minibuffer as well as in the *Floobits Chat* buffer, which contains a timestamped log of all messages sent / received, including clickable URLs and email addresses. The RET key can be used from the *Floobits Chat* buffer to invoke floobits-send-message.

Everything mentioned works well but I'm playing with different ideas about what the final interface should be (should all types of notifications be included in one buffer rather than just chat messages? Something akin to #36?). Sharing for feedback / collaboration.

TODO

  • Probably better if we use message timestamps instead of system time.
  • Display appropriate user name for outbound messages (not me).
  • Apply faces for highlighting.
  • Decide how to notify when workspace has changed.
  • Support typing into buffer to send messages (investigate comint-mode).
  • Show count of unread messages on modeline?
  • Detect when we don't have msg permissions (trying to send a message without permissions results in the session being destroyed)

@kans
Copy link
Member

kans commented May 5, 2015

@camdez
An example of how colors are calculated for users can be found here: https://github.com/Floobits/django-stuff/blob/master/web/static/js/fl/util.js#L77.

@camdez
Copy link
Contributor Author

camdez commented May 5, 2015

Hey @kans,

I suspect that might be a private repo because it 404s for me. Any chance you could adjust permissions or share in another way? Thanks.

@kans
Copy link
Member

kans commented May 5, 2015

@ggreer
Copy link
Member

ggreer commented May 5, 2015

I wouldn't worry too much about getting colors consistent. We should update that code to let people choose their own colors anyway.

@camdez
Copy link
Contributor Author

camdez commented May 5, 2015

@ggreer Sure, but it's a simple algorithm. I might toss it in anyway ahead of allowing users to pick their own colors.

@ggreer @kans Can you guys help me with something else? How can I determine the (Emacs) user's Floobits username?

@ggreer
Copy link
Member

ggreer commented May 5, 2015

The room_info response (first event sent back by the server) should contain the username. It looks like on_room_info in floo/agent_connection.py discards that when sending to emacs. If you like, I can pair with you to answer these sorts of questions and help with any python bits.

@camdez
Copy link
Contributor Author

camdez commented May 6, 2015

@ggreer Thanks. I'll definitely hit you up if I run into any roadblocks. Your comment above was enough to get me going on this one.

@camdez
Copy link
Contributor Author

camdez commented May 6, 2015

I decided to add the colors just for fun:

screen shot 2015-05-06 at 10 36 05 am

Clashing with the user's color theme is definitely a concern, but it's a start.

@ggreer
Copy link
Member

ggreer commented May 7, 2015

Very nice. BTW, your first TODO (use message timestamps instead of system time) might be more important than you think. When a user joins a workspace, we replay messages they haven't received. If you use the system time, users might be confused. I don't consider that a blocker, but it's worth taking into account.

Oh and just FYI: All times sent from the server are UTC. It's up to the client to figure out the time zone and localize them.

@camdez
Copy link
Contributor Author

camdez commented May 7, 2015

@ggreer Aha. Thanks for pointing that out. Are the messages guaranteed to be delivered in (chronological) order?

@ggreer
Copy link
Member

ggreer commented May 7, 2015

They are. The order is room_info (always the first event), then replayed msg events chronologically.

@camdez
Copy link
Contributor Author

camdez commented May 18, 2015

screen shot 2015-05-17 at 3 28 57 pm

I had a chance to use this in anger yesterday and it was a pretty cool experience, though I did get burned by the permissions issue (added to the checklist above). I think I'll solve that, work on nailing down the user experience (particularly around input), cover the system messaging thing (i.e. notify when switching workspaces, etc.), and then look towards merging. Adding a mode line indicator and the ability to customize how messages are formatted are niceties that I can return to later.

From the Elisp manual:

> If you need Common Lisp extensions, use the cl-lib library rather than
> the old cl library. The latter does not use a clean namespace (i.e.,
> its definitions do not start with a ‘cl-’ prefix). If your package
> loads cl at run time, that could cause name clashes for users who
> don't use that package.
@ggreer
Copy link
Member

ggreer commented May 20, 2015

Wow, very nice.

I don't know what Emacs users prefer, but typically we make one pane for all messages (chat and informational). In that way, it's like IRC: You see joins, kicks, +o, and other info interspersed with chat messages.

If you need any help that requires back-and-forth, feel free to ping me on IRC.

@camdez
Copy link
Contributor Author

camdez commented May 21, 2015

@ggreer Great. That's what I was thinking as well and I have most of it implemented locally.

sellout added a commit to sellout/floobits-emacs that referenced this pull request Jul 9, 2016
Partially stolen from Floobits#84, this uses username
highlight colors for the cursor as well.
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.

3 participants