Use matrix displayname if available

[amalon]

Use the matrix displayname retrieved using getProfileInfo(), fixing issue #3.

As far as I can tell (at least with synapse), this reports a displayname even if one hasn't been explicitly set, however it will raise an exception if the user doesn't exist, but given we just recieved a message from the user that shouldn't happen.

[amalon]

Thanks for the contribution! #3 is me wanting to replace the Server: <matrix username>: message with <matrix username>: message when sending to mumble.

Ah yes, I've removed reference to #3 as this is subtly different, and improved the justification in the commit message.

* Instead of adding bridge to the class, you should be able to use `this.getIntent()` below and either overwrite `event.sender` or add an optional paramater to `sendMessage`.

this.getIntent() doesn't seem to work there, but i can reference bridge in the outer function (since it should be intiialised by the time the callback is called). Hope thats sensible.

You will need to convert the containing function to an anonymous function to do this. https://github.com/mymindstorm/matrix-appservice-mumble/blob/d227033f826995a88cdd27ed04c18ddc9466a755/src/main.ts#L45

Sorry, I'm not sure I follow. Isn't it already anonymous? (which function do you mean).

* If it raises an exception, use a try catch statement and fall back to the event data if necessary.

Okay, i've done that too just in case. Let me know if there are other issues (I'm fairly inexperienced with typescript tbh)

[mymindstorm]

Oops, I meant arrow function. In JavaScript, the value of this changes depending on how the function is declared. Using this inside of onEvent: function(request: any, context: any) { will refer to the controller property, and onEvent: (request: any, context: any) => { would refer to the bridge (or at least I thought, it seems to just refer to the data in this case).

I have a patch below that caches the username lookups to save some time since this will be called a lot. Please apply and I will merge this.

0001-use-cache-when-getting-user-display-name.patch

[amalon]

Oops, I meant arrow function. In JavaScript, the value of this changes depending on how the function is declared. Using this inside of onEvent: function(request: any, context: any) { will refer to the controller property, and onEvent: (request: any, context: any) => { would refer to the bridge (or at least I thought, it seems to just refer to the data in this case).

Ah okay. I guess I'll leave it how it is now then.

I have a patch below that caches the username lookups to save some time since this will be called a lot. Please apply and I will merge this.

0001-use-cache-when-getting-user-display-name.patch

I've applied this, but note that the default value of getProfileInfo()'s useCache parameter does seem to be true (in fact I did confirm it only send one request to the homeserver from the homeserver logs).

Would you like me to remove the true argument?

[mymindstorm]

I've applied this, but note that the default value of getProfileInfo()'s useCache parameter does seem to be true (in fact I did confirm it only send one request to the homeserver from the homeserver logs).

Would you like me to remove the true argument?

Sure, as long as it's true by default.

[amalon]

Okay, i've dropped the useCache parameter and checked it behaves the same (it caches the displaynames, though the TTL is (I presume) set to the default of 90 seconds, so it does still make requests periodically)

[mymindstorm]

Last thing, please fix the merge conflicts. There should be a instructions on how to do it under the review approvals.

[amalon]

Last thing, please fix the merge conflicts. There should be a instructions on how to do it under the review approvals.

When testing master, i get the following when rebuilding:

src/main.ts:55:68 - error TS2345: Argument of type 'null' is not assignable to parameter of type 'string | undefined'.

55       murmur.setMatrixClient(bridge.getClientFactory().getClientAs(null));
                                                                      ~~~~

Found 1 error.

I guess that should be fixed first... I'll look into it later if I get time.

[amalon]

I've rebased the other two commits on master, along with a fix for the build failure on master

[mymindstorm]

Can you run npm i again? I added a dependency, that's probably why you are having build issues. I don't see this on my or the CI's builds.

[amalon]

Can you run npm i again? I added a dependency, that's probably why you are having build issues. I don't see this on my or the CI's builds.

I did that. But indeed if I use ./node_modules/.bin/tsc (3.8.2) it works. I was using tsc from the archlinux typescript package version 3.9.3. Is the newer version of tsc considered unsupported? If so I'm happy to drop the build fix and start using the older one.

[mymindstorm]

Ah, I will update typescript. Right now the only supported build method is through build.sh

[amalon]

Ah, I will update typescript. Right now the only supported build method is through build.sh

Ok, thanks for your patience :-)

[mymindstorm]

Sorry for giving you so much trouble! I 0.2 is on NPM :D