Skip to content

Commit f7e27c7

Browse files
KicuKicu
committed
Improve short mentions regex and make parser tests pass
1 parent bb4cc97 commit f7e27c7

File tree

3 files changed

+53
-26
lines changed

3 files changed

+53
-26
lines changed

‎__tests__/ExpensiMark-HTML-test.js

+26-15
Original file line numberDiff line numberDiff line change
@@ -693,11 +693,10 @@ test('Test url replacements', () => {
693693
'<a href="http://example.com/foo/*/bar/*/test.txt" target="_blank" rel="noreferrer noopener">http://example.com/foo/*/bar/*/test.txt</a> ' +
694694
'test-.com ' +
695695
'-<a href="https://test.com" target="_blank" rel="noreferrer noopener">test.com</a> ' +
696-
'@test.com ' +
697-
'@test.com <a href="https://test.com" target="_blank" rel="noreferrer noopener">test.com</a> ' +
698-
'@test.com @test.com ';
696+
'<mention-short>@test.com</mention-short> ' +
697+
'<mention-short>@test.com</mention-short> <a href="https://test.com" target="_blank" rel="noreferrer noopener">test.com</a> ' +
698+
'<mention-short>@test.com</mention-short> <mention-short>@test.com</mention-short> ';
699699

700-
// Fixme [short-mention] this errors on "test.com</a> @test.com @test.com <a" - probably @test.com should be a legit short-mention candidate
701700
expect(parser.replace(urlTestStartString)).toBe(urlTestReplacedString);
702701
});
703702

@@ -877,7 +876,7 @@ test('Test urls autolinks correctly', () => {
877876
{
878877
testString: 'expensify.com -expensify.com @expensify.com',
879878
resultString:
880-
'<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> -<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> @expensify.com',
879+
'<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> -<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> <mention-short>@expensify.com</mention-short>',
881880
},
882881
{
883882
testString: 'https//www.expensify.com',
@@ -932,7 +931,6 @@ test('Test urls autolinks correctly', () => {
932931
},
933932
];
934933

935-
// Fixme [short-mention] @expensify.com should now be considered a short-mention "candidate"
936934
testCases.forEach((testCase) => {
937935
expect(parser.replace(testCase.testString)).toBe(testCase.resultString);
938936
});
@@ -1327,12 +1325,6 @@ test('Test for user mention with @username@domain.com', () => {
13271325
expect(parser.replace(testString)).toBe(resultString);
13281326
});
13291327

1330-
test('Test for short mention mention with @username', () => {
1331-
const testString = '@john.doe';
1332-
const resultString = '<mention-short>@john.doe</mention-short>';
1333-
expect(parser.replace(testString)).toBe(resultString);
1334-
});
1335-
13361328
test('Test for user mention with @@username@domain.com', () => {
13371329
const testString = '@@username@expensify.com';
13381330
const resultString = '@<mention-user>@username@expensify.com</mention-user>';
@@ -1460,6 +1452,26 @@ test('Test for @here mention with inlineCodeBlock style', () => {
14601452
expect(parser.replace(testString)).toBe(resultString);
14611453
});
14621454

1455+
describe('Tests for short mentions', () => {
1456+
test('short mentions should work for @username', () => {
1457+
const testString = '@johnny';
1458+
const resultString = '<mention-short>@johnny</mention-short>';
1459+
expect(parser.replace(testString)).toBe(resultString);
1460+
});
1461+
1462+
test('short mentions should work for @firstname.lastname', () => {
1463+
const testString = '@john.doe';
1464+
const resultString = '<mention-short>@john.doe</mention-short>';
1465+
expect(parser.replace(testString)).toBe(resultString);
1466+
});
1467+
1468+
test('short mentions should work and not break @here after mention', () => {
1469+
const testString = '@john.doe@here';
1470+
const resultString = '<mention-short>@john.doe</mention-short><mention-here>@here</mention-here>';
1471+
expect(parser.replace(testString)).toBe(resultString);
1472+
});
1473+
});
1474+
14631475
// Examples that should match for here mentions:
14641476
test('Test for here mention with @here', () => {
14651477
const testString = '@here';
@@ -1516,7 +1528,6 @@ test('Test for @here mention with italic, bold and strikethrough styles', () =>
15161528
' @here!' +
15171529
' @here?';
15181530

1519-
// Fixme [short-mention] these should now be short-mention candidates
15201531
const resultString =
15211532
'<mention-here>@here</mention-here>' +
15221533
' <em><mention-here>@here</mention-here></em>' +
@@ -1659,13 +1670,13 @@ test('Skip rendering invalid markdown', () => {
16591670

16601671
test('Test for email with test+1@gmail.com@gmail.com', () => {
16611672
const testString = 'test+1@gmail.com@gmail.com';
1662-
const resultString = '<a href="mailto:test+1@gmail.com">test+1@gmail.com</a>@gmail.com';
1673+
const resultString = '<a href="mailto:test+1@gmail.com">test+1@gmail.com</a><mention-short>@gmail.com</mention-short>';
16631674
expect(parser.replace(testString)).toBe(resultString);
16641675
});
16651676

16661677
test('Test for email with test@gmail.com@gmail.com', () => {
16671678
const testString = 'test@gmail.com@gmail.com';
1668-
const resultString = '<a href="mailto:test@gmail.com">test@gmail.com</a>@gmail.com';
1679+
const resultString = '<a href="mailto:test@gmail.com">test@gmail.com</a><mention-short>@gmail.com</mention-short>';
16691680
expect(parser.replace(testString)).toBe(resultString);
16701681
});
16711682

‎lib/CONST.ts

+5
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,11 @@ const CONST = {
419419
*/
420420
EMOJI_RULE:
421421
/[\p{Extended_Pictographic}](\u200D[\p{Extended_Pictographic}]|[\u{1F3FB}-\u{1F3FF}]|[\u{E0020}-\u{E007F}]|\uFE0F|\u20E3)*|[\u{1F1E6}-\u{1F1FF}]{2}|[#*0-9]\uFE0F?\u20E3/gu,
422+
423+
/**
424+
* Regex to match a piece of text or @here, needed for both shortMention and userMention
425+
*/
426+
PRE_MENTION_TEXT_PART: '(@here|[a-zA-Z0-9.!$%&+=?^\\`{|}-]?)',
422427
},
423428

424429
REPORT: {

‎lib/ExpensiMark.ts

+22-11
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ export default class ExpensiMark {
365365
{
366366
name: 'userMentions',
367367
regex: new RegExp(
368-
`(@here|[a-zA-Z0-9.!$%&+=?^\`{|}-]?)(@${Constants.CONST.REG_EXP.EMAIL_PART}|@${Constants.CONST.REG_EXP.PHONE_PART})(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`,
368+
`${Constants.CONST.REG_EXP.PRE_MENTION_TEXT_PART}(@${Constants.CONST.REG_EXP.EMAIL_PART}|@${Constants.CONST.REG_EXP.PHONE_PART})(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`,
369369
'gim',
370370
),
371371
replacement: (_extras, match, g1, g2) => {
@@ -484,11 +484,25 @@ export default class ExpensiMark {
484484
rawInputReplacement: '$1<a href="mailto:$2" data-raw-href="$2" data-link-variant="auto">$2</a>',
485485
},
486486

487+
/**
488+
* This regex matches a short user mention in a string.
489+
* A short-mention is a string that starts with the '@' symbol and is followed by a valid user's primary login without the email domain part
490+
* Ex: @john.doe, @user12345, but NOT @user@email.com
491+
*
492+
* Notes:
493+
* Phone is not a valid short mention.
494+
* In reality these "short-mentions" are just possible candidates, because the parser has no way of verifying if there exists a user named ex: @john.examplename.
495+
* The actual verification whether these mentions are pointing to real users is done in specific projects using ExpensiMark.
496+
* Nevertheless, "@john.examplename" is a correct possible short-mention, and so would be parsed.
497+
* This behaviour is similar to treating every user@something as valid user login.
498+
*
499+
* This regex will correctly preserve any @here mentions, the same way as "userMention" rule.
500+
*/
487501
{
488502
name: 'shortMentions',
489503

490504
regex: new RegExp(
491-
"(@here|[a-zA-Z0-9.!$%&+=?^\\`{|}-]?)(@(?=((?=[\\w]+[\\w'#%+-]+(?:\\.[\\w'#%+-]+)*)[\\w\\.'#%+-]{1,64}(?= |_|\\b))(?!([:\\/\\\\]))(?<end>.*))\\S{3,254}(?=\\k<end>$))(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>|<\\/mention-user>|<\\/mention-here>))",
505+
`${Constants.CONST.REG_EXP.PRE_MENTION_TEXT_PART}(@(?=((?=[\\w]+[\\w'#%+-]+(?:\\.[\\w'#%+-]+)*)[\\w\\.'#%+-]{1,64}(?= |_|\\b))(?!([:\\/\\\\]))(?<end>.*))(?!here)\\S{3,254}(?=\\k<end>$))(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>|<\\/mention-user>|<\\/mention-here>))`,
492506
'gim',
493507
),
494508
replacement: (_extras, match, g1, g2) => {
@@ -497,15 +511,12 @@ export default class ExpensiMark {
497511
}
498512
return `${g1}<mention-short>${g2}</mention-short>`;
499513
},
500-
// rawInputReplacement: (_extras, match, g1, g2) => {
501-
// const phoneNumberRegex = new RegExp(`^${Constants.CONST.REG_EXP.PHONE_PART}$`);
502-
// const mention = g2.slice(1);
503-
// const mentionWithoutSMSDomain = str_1.default.removeSMSDomain(mention);
504-
// if (!str_1.default.isValidMention(match) || (phoneNumberRegex.test(mentionWithoutSMSDomain) && !str_1.default.isValidPhoneNumber(mentionWithoutSMSDomain))) {
505-
// return match;
506-
// }
507-
// return `${g1}<mention-user>${g2}</mention-user>`;
508-
// },
514+
},
515+
516+
{
517+
name: 'hereMentionAfterShortMentions',
518+
regex: /(<\/mention-short>)(@here)(?=\b)/gm,
519+
replacement: '$1<mention-here>$2</mention-here>',
509520
},
510521

511522
{

0 commit comments

Comments
 (0)