search

agl / xmpp-client

An XMPP client with OTR support
BSD 3-Clause "New" or "Revised" License
365 stars 71 forks source link

Display OTR encryption status for a conversation within the prompt #58

Closed isislovecruft closed 10 years ago

isislovecruft commented 10 years ago

While speaking with someone from the Freedom of the Press foundation who expressed the desire for an xmpp-client GUI (to better facilitate journalists in moving away from using Pidgin), I was looking into the structural feasibility of adding UI hooks (for your gogtk package used in Pond) to xmpp-client.

Bored with looking at code and not playing with it, and inspired by @leif's patch to add the JID of the current contact being spoken to in the prompt, I added support for changing the colour of that JID based on the OTR encrypted/unencrypted state of that conversation.

Personally, I'm terrified, after some messages leaked, unencrypted, to a contact I wasn't currently speaking to (near identical JIDs plus janky TAB key, #FTL) to turn off "Always Encrypt": true. @leif's patch helps significantly to be sure that I'm speaking to the correct person, and I'd be more comfortable disabling "Always Encrypt" if I were always kept informed of the current OTR state of the current conversation.

I apologise for dumping completely unsolicited patches on you. If this feature isn't desired, you should feel free to close this issue, and I am happy to just keep these patches as my own thing.

agl commented 10 years ago

Thanks! There were some layering and race issues with the original code that I tidied up a little.

isislovecruft commented 10 years ago

Interesting. (Sorry! I'm new at Go.) Although, it felt like I was doing something wrong by needing to call the mutex lock so many times, and passing the Session into Input.ProcessCommands felt wrong too.

What sorts of issues would this have caused? Would the race condition close the channel? Can race conditions in Go lead to reference counting issues?

agl commented 10 years ago

No worries! Go doesn't use reference counting, but racing a map can lead to crashes. If no doubt, Go comes with TSan (https://code.google.com/p/thread-sanitizer/) built in and it can be enabled with go build -race. Of course, it doesn't find all races, but it can certainly help.

isislovecruft commented 10 years ago

Oh, neat, it comes with TSan! Awesome.

Thanks, @agl!