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.

Sign up for GitHub

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

Use localpart in plaintext so we get highlighted for pills #658

Merged
merged 23 commits into from
Aug 23, 2018
Merged

Use localpart in plaintext so we get highlighted for pills #658

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0d52d37
Merge branch 'hs/joined-rooms-no-join' into develop
Half-Shot Jul 10, 2018
e15ff60
Merge branch 'hs/reject-invites' into develop
Half-Shot Jul 10, 2018
bb8649f
Bump matrix-appservice-bridge to 1.5.0a
Half-Shot Jul 25, 2018
7bde9d7
Merge branch 'develop' of github.com:Half-Shot/matrix-appservice-irc …
Half-Shot Aug 10, 2018
e6facc5
Counters are incremental
Half-Shot Aug 21, 2018
7790377
Use localpart in plaintext so we get highlighted
Half-Shot Aug 22, 2018
f830415
Use a better regex so we don't catch URLs
Half-Shot Aug 22, 2018
7dbee49
spacing
Half-Shot Aug 22, 2018
0c08853
Promiseify formatMentions
Half-Shot Aug 23, 2018
1e12084
Use getProfileInfo to fetch the displayname
Half-Shot Aug 23, 2018
6d4901f
Fixup tests
Half-Shot Aug 23, 2018
6fce830
Merge remote-tracking branch 'origin/develop' into hs/mentions-use-di…
Half-Shot Aug 23, 2018
adeb5fe
Merge remote-tracking branch 'upstream/develop' into hs/mentions-use-…
Half-Shot Aug 23, 2018
0e7930e
Merge remote-tracking branch 'upstream/develop' into hs/mentions-use-…
Half-Shot Aug 23, 2018
57ad38e
*deeep sigh*
Half-Shot Aug 23, 2018
f7a3419
Linting
Half-Shot Aug 23, 2018
f698081
Less whitespace
Half-Shot Aug 23, 2018
d2abf26
Trailing spaces have been neutralised
Half-Shot Aug 23, 2018
7faeb2b
Merge remote-tracking branch 'upstream/develop' into hs/mentions-use-…
Half-Shot Aug 23, 2018
2509712
Merge remote-tracking branch 'upstream/develop' into hs/mentions-use-…
Half-Shot Aug 23, 2018
402e674
Test fetching displaynames
Half-Shot Aug 23, 2018
0580ff2
Fetch displayname properly & Fix intent
Half-Shot Aug 23, 2018
8686f87
Fix line lengths
Half-Shot Aug 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/models/MatrixAction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,18 @@ MatrixAction.prototype.formatMentions = function(nickUserIdMap) {
this.htmlText = this.text;
}
userId = ircFormatting.escapeHtmlChars(userId);
/* Due to how Riot and friends do push notifications, we need the plain text to match something.
Modern client's will pill-ify and will show a displayname, so we can use the localpart so it
matches and doesn't degrade too badly on limited clients.*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: double space after matches

const localpart = userId.substr(1, userId.indexOf(":")-1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be using the display name, as per matrix-org/matrix-spec-proposals#1547

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really want to do a displayname lookup for every match? The localparts already match.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer it if the cached display name was used, given we already know about the member event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we do know about the mem...OH you mean the member cache thingatron?

this.htmlText = this.htmlText.replace(
new RegExp(escapeStringRegexp(matchName), "igm"),
`<a href="https://matrix.to/#/${userId}">${ircFormatting.escapeHtmlChars(matchName)}</a>`
);
this.text = this.text.replace(
new RegExp(escapeStringRegexp(matchName), "igm"),
localpart
);
// Don't match this name twice, we've already replaced all entries.
matched.add(matchName.toLowerCase());
}
Expand Down
12 changes: 6 additions & 6 deletions spec/unit/MatrixAction.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe("MatrixAction", function() {
action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.text).toEqual("jc.denton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>, it's a bomb!"
);
Expand All @@ -35,7 +35,7 @@ describe("MatrixAction", function() {
action.formatMentions({
"jcdenton": "@jc.denton:unatco.gov"
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.text).toEqual("jc.denton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">jcdenton</a>, it's a bomb!"
);
Expand All @@ -45,7 +45,7 @@ describe("MatrixAction", function() {
action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.text).toEqual("jc.denton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>, it's a bomb!"
);
Expand All @@ -55,7 +55,7 @@ describe("MatrixAction", function() {
action.formatMentions({
"`||JCDenton[m]": "@jc.denton:unatco.gov"
});
expect(action.text).toEqual("`||JCDenton[m], it's a bomb!");
expect(action.text).toEqual("jc.denton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">`||JCDenton[m]</a>, it's a bomb!"
);
Expand All @@ -71,7 +71,7 @@ describe("MatrixAction", function() {
"JCDenton": "@jc.denton:unatco.gov",
"PaulDenton": "@paul.denton:unatco.gov"
});
expect(action.text).toEqual("JCDenton is sent to assassinate PaulDenton");
expect(action.text).toEqual("jc.denton is sent to assassinate paul.denton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a> is sent" +
" to assassinate <a href=\"https://matrix.to/#/@paul.denton:unatco.gov\">PaulDenton</a>"
Expand All @@ -87,7 +87,7 @@ describe("MatrixAction", function() {
action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
});
expect(action.text).toEqual("JCDenton, meet JCDenton");
expect(action.text).toEqual("jc.denton, meet jc.denton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>," +
" meet <a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>"
Expand Down