fix: don't match a trailing period after the path

Fixes #17. This also extends `trailingPeriod: false` (the default) to exclude matching a trailing question mark (?) or exclamation mark (!). That is if you want "Check out example.com/project." to NOT match the trailing period, you typically also want it to NOT match the trailing exclamation mark in "Check out example.com/project!". Likewise for "Have you seen example.com/project?". I guess technically `trailingPeriod` should be renamed `trailingPunctuation` but maybe it's fine as-is?
spamscanner · Aug 14, 2023 · f745e26 · f745e26
1 parent 3228299
commit f745e26
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 24 deletions.
diff --git a/src/index.js b/src/index.js
@@ -45,17 +45,22 @@ module.exports = (options) => {
   })${options.trailingPeriod ? '\\.?' : ''}`;
 
   const port = '(?::\\d{2,5})?';
-  // Not accept closing parenthesis
-  // <https://github.com/kevva/url-regex/pull/35>
-  // Don't allow apostrophes
-  // <https://github.com/kevva/url-regex/pull/55>
-  const path = options.parens
-    ? options.apostrophes
-      ? '(?:[/?#][^\\s"]*)?'
-      : '(?:[/?#][^\\s"\']*)?'
-    : options.apostrophes
-    ? '(?:[/?#][^\\s"\\)]*)?'
-    : '(?:[/?#][^\\s"\\)\']*)?';
+  let disallowedChars = '\\s"';
+  if (!options.parens) {
+    // Not accept closing parenthesis
+    // <https://github.com/kevva/url-regex/pull/35>
+    disallowedChars += '\\)';
+  }
+
+  if (!options.apostrophes) {
+    // Don't allow apostrophes
+    // <https://github.com/kevva/url-regex/pull/55>
+    disallowedChars += "'";
+  }
+
+  const path = options.trailingPeriod
+    ? `(?:[/?#][^${disallowedChars}]*)?`
+    : `(?:(?:[/?#][^${disallowedChars}]*[^${disallowedChars}.?!])|[/])?`;
 
   // Added IPv6 support
   // <https://github.com/kevva/url-regex/issues/60>

diff --git a/test/test.js b/test/test.js
@@ -500,17 +500,77 @@ test('localhost', (t) => {
   );
 });
 
-test('trailing period', (t) => {
-  t.deepEqual(
-    'background example.com. foobar.com'.match(
-      urlRegex({ trailingPeriod: true })
-    ),
-    ['example.com.', 'foobar.com']
-  );
-  t.deepEqual(
-    'background example.com. foobar.com'.match(
-      urlRegex({ trailingPeriod: false })
-    ),
+for (const [source, withTrailingPeriod, withoutTrailingPeriod] of [
+  [
+    'background example.com. foobar.com',
+    ['example.com.', 'foobar.com'],
     ['example.com', 'foobar.com']
-  );
-});
+  ],
+  [
+    'https://example.com/dir.',
+    ['https://example.com/dir.'],
+    ['https://example.com/dir']
+  ],
+  [
+    'https://example.com/dir. ',
+    ['https://example.com/dir.'],
+    ['https://example.com/dir']
+  ],
+  [
+    'https://example.com/dir.\n',
+    ['https://example.com/dir.'],
+    ['https://example.com/dir']
+  ],
+  [
+    'https://example.com/index.html',
+    ['https://example.com/index.html'],
+    ['https://example.com/index.html']
+  ],
+  [
+    'https://example.com/index.html.',
+    ['https://example.com/index.html.'],
+    ['https://example.com/index.html']
+  ],
+  [
+    'https://example.com/dir.with.dot/.',
+    ['https://example.com/dir.with.dot/.'],
+    ['https://example.com/dir.with.dot/']
+  ],
+  // Question marks
+  ['Have you ever visited example.com?', ['example.com?'], ['example.com']],
+  ['example.com/?', ['example.com/?'], ['example.com/']],
+  [
+    'https://example.com/dir?',
+    ['https://example.com/dir?'],
+    ['https://example.com/dir']
+  ],
+  // Exclamation marks
+  ['You should check out example.com!', ['example.com'], ['example.com']],
+  ['Here is example.com/!', ['example.com/!'], ['example.com/']],
+  [
+    'https://example.com/dir/!',
+    ['https://example.com/dir/!'],
+    ['https://example.com/dir/']
+  ],
+  [
+    'https://example.com/dir!',
+    ['https://example.com/dir!'],
+    ['https://example.com/dir']
+  ]
+]) {
+  const sourceTitle = source.replace('\n', '\\n');
+
+  test(`trailingPeriod: true (${sourceTitle})`, (t) => {
+    t.deepEqual(
+      source.match(urlRegex({ trailingPeriod: true })),
+      withTrailingPeriod
+    );
+  });
+
+  test(`trailingPeriod: false (${sourceTitle})`, (t) => {
+    t.deepEqual(
+      source.match(urlRegex({ trailingPeriod: false })),
+      withoutTrailingPeriod
+    );
+  });
+}