From 769cd06ab65a700df1623aad79d206105c088432 Mon Sep 17 00:00:00 2001 From: akshatdalton Date: Wed, 19 May 2021 04:18:23 +0000 Subject: [PATCH] refactor: Extract linkifier non-settings logic from `markdown.js`. The extracted logic is in linkifier.js. We have decided to name it linkifier.js instead of realm_linkifier.js because in future when we will add stream-level linkifiers, we'll likely want them to be managed by this same file. --- frontend_tests/node_tests/dispatch.js | 4 +- frontend_tests/node_tests/linkifiers.js | 58 +++++++++++ frontend_tests/node_tests/markdown.js | 54 +--------- frontend_tests/node_tests/markdown_katex.js | 2 +- static/js/linkifiers.js | 109 ++++++++++++++++++++ static/js/markdown.js | 108 +------------------ static/js/server_events_dispatch.js | 4 +- static/js/ui_init.js | 4 +- 8 files changed, 183 insertions(+), 160 deletions(-) create mode 100644 frontend_tests/node_tests/linkifiers.js create mode 100644 static/js/linkifiers.js diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index 2322d810dc..1d024cab46 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -28,7 +28,7 @@ const bot_data = mock_esm("../../static/js/bot_data"); const composebox_typeahead = mock_esm("../../static/js/composebox_typeahead"); const emoji_picker = mock_esm("../../static/js/emoji_picker"); const hotspots = mock_esm("../../static/js/hotspots"); -const markdown = mock_esm("../../static/js/markdown"); +const linkifiers = mock_esm("../../static/js/linkifiers"); const message_edit = mock_esm("../../static/js/message_edit"); const message_events = mock_esm("../../static/js/message_events"); const message_list = mock_esm("../../static/js/message_list"); @@ -523,7 +523,7 @@ run_test("realm_linkifiers", (override) => { const event = event_fixtures.realm_linkifiers; page_params.realm_linkifiers = []; override(settings_linkifiers, "populate_linkifiers", noop); - override(markdown, "update_linkifier_rules", noop); + override(linkifiers, "update_linkifier_rules", noop); dispatch(event); assert_same(page_params.realm_linkifiers, event.realm_linkifiers); }); diff --git a/frontend_tests/node_tests/linkifiers.js b/frontend_tests/node_tests/linkifiers.js new file mode 100644 index 0000000000..f4e90172aa --- /dev/null +++ b/frontend_tests/node_tests/linkifiers.js @@ -0,0 +1,58 @@ +"use strict"; + +const {strict: assert} = require("assert"); + +const {zrequire} = require("../zjsunit/namespace"); +const {run_test} = require("../zjsunit/test"); +const blueslip = require("../zjsunit/zblueslip"); + +const linkifiers = zrequire("linkifiers"); +const marked = zrequire("../third/marked/lib/marked"); + +linkifiers.initialize([]); + +run_test("python_to_js_linkifier", () => { + // The only way to reach python_to_js_linkifier is indirectly, hence the call + // to update_linkifier_rules. + linkifiers.update_linkifier_rules([ + { + pattern: "/a(?im)a/g", + url_format: "http://example1.example.com", + id: 10, + }, + { + pattern: "/a(?L)a/g", + url_format: "http://example2.example.com", + id: 20, + }, + ]); + let actual_value = marked.InlineLexer.rules.zulip.linkifiers; + let expected_value = [/\/aa\/g(?!\w)/gim, /\/aa\/g(?!\w)/g]; + assert.deepEqual(actual_value, expected_value); + // Test case with multiple replacements. + linkifiers.update_linkifier_rules([ + { + pattern: "#cf(?P\\d+)(?P[A-Z][\\dA-Z]*)", + url_format: "http://example3.example.com", + id: 30, + }, + ]); + actual_value = marked.InlineLexer.rules.zulip.linkifiers; + expected_value = [/#cf(\d+)([A-Z][\dA-Z]*)(?!\w)/g]; + assert.deepEqual(actual_value, expected_value); + // Test incorrect syntax. + blueslip.expect( + "error", + "python_to_js_linkifier: Invalid regular expression: /!@#@(!#&((!&(@#((?!\\w)/: Unterminated group", + ); + linkifiers.update_linkifier_rules([ + { + pattern: "!@#@(!#&((!&(@#(", + url_format: "http://example4.example.com", + id: 40, + }, + ]); + actual_value = marked.InlineLexer.rules.zulip.linkifiers; + expected_value = []; + assert.deepEqual(actual_value, expected_value); +}); diff --git a/frontend_tests/node_tests/markdown.js b/frontend_tests/node_tests/markdown.js index 0c076e92b4..675e985570 100644 --- a/frontend_tests/node_tests/markdown.js +++ b/frontend_tests/node_tests/markdown.js @@ -6,7 +6,6 @@ const markdown_test_cases = require("../../zerver/tests/fixtures/markdown_test_c const markdown_assert = require("../zjsunit/markdown_assert"); const {set_global, zrequire} = require("../zjsunit/namespace"); const {run_test} = require("../zjsunit/test"); -const blueslip = require("../zjsunit/zblueslip"); const {page_params} = require("../zjsunit/zpage_params"); set_global("location", { @@ -42,10 +41,10 @@ set_global("document", doc); const emoji = zrequire("../shared/js/emoji"); const emoji_codes = zrequire("../generated/emoji/emoji_codes.json"); +const linkifiers = zrequire("linkifiers"); const pygments_data = zrequire("../generated/pygments_data.json"); const fenced_code = zrequire("../shared/js/fenced_code"); const markdown_config = zrequire("markdown_config"); -const marked = zrequire("../third/marked/lib/marked"); const markdown = zrequire("markdown"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); @@ -188,12 +187,13 @@ stream_data.add_sub(edgecase_stream_2); // streamTopicHandler and it would be parsed as edgecase_stream_2. stream_data.add_sub(amp_stream); -markdown.initialize(example_realm_linkifiers, markdown_config.get_helpers()); +markdown.initialize(markdown_config.get_helpers()); +linkifiers.initialize(example_realm_linkifiers); function test(label, f) { run_test(label, (override) => { page_params.realm_users = []; - markdown.update_linkifier_rules(example_realm_linkifiers); + linkifiers.update_linkifier_rules(example_realm_linkifiers); f(override); }); } @@ -717,52 +717,6 @@ test("backend_only_linkifiers", () => { } }); -test("python_to_js_linkifier", () => { - // The only way to reach python_to_js_linkifier is indirectly, hence the call - // to update_linkifier_rules. - markdown.update_linkifier_rules([ - { - pattern: "/a(?im)a/g", - url_format: "http://example1.example.com", - id: 10, - }, - { - pattern: "/a(?L)a/g", - url_format: "http://example2.example.com", - id: 20, - }, - ]); - let actual_value = marked.InlineLexer.rules.zulip.linkifiers; - let expected_value = [/\/aa\/g(?!\w)/gim, /\/aa\/g(?!\w)/g]; - assert.deepEqual(actual_value, expected_value); - // Test case with multiple replacements. - markdown.update_linkifier_rules([ - { - pattern: "#cf(?P\\d+)(?P[A-Z][\\dA-Z]*)", - url_format: "http://example3.example.com", - id: 30, - }, - ]); - actual_value = marked.InlineLexer.rules.zulip.linkifiers; - expected_value = [/#cf(\d+)([A-Z][\dA-Z]*)(?!\w)/g]; - assert.deepEqual(actual_value, expected_value); - // Test incorrect syntax. - blueslip.expect( - "error", - "python_to_js_linkifier: Invalid regular expression: /!@#@(!#&((!&(@#((?!\\w)/: Unterminated group", - ); - markdown.update_linkifier_rules([ - { - pattern: "!@#@(!#&((!&(@#(", - url_format: "http://example4.example.com", - id: 40, - }, - ]); - actual_value = marked.InlineLexer.rules.zulip.linkifiers; - expected_value = []; - assert.deepEqual(actual_value, expected_value); -}); - test("translate_emoticons_to_names", () => { // Simple test const test_text = "Testing :)"; diff --git a/frontend_tests/node_tests/markdown_katex.js b/frontend_tests/node_tests/markdown_katex.js index afdc70192f..bf8794131f 100644 --- a/frontend_tests/node_tests/markdown_katex.js +++ b/frontend_tests/node_tests/markdown_katex.js @@ -15,7 +15,7 @@ const markdown_config = zrequire("markdown_config"); const markdown = zrequire("markdown"); -markdown.initialize([], markdown_config.get_helpers()); +markdown.initialize(markdown_config.get_helpers()); run_test("katex_throws_unexpected_exceptions", () => { blueslip.expect("error", "Error: some-exception"); diff --git a/static/js/linkifiers.js b/static/js/linkifiers.js new file mode 100644 index 0000000000..ccec5601ea --- /dev/null +++ b/static/js/linkifiers.js @@ -0,0 +1,109 @@ +import marked from "../third/marked/lib/marked"; + +import * as blueslip from "./blueslip"; + +const linkifier_map = new Map(); +export let linkifier_list = []; + +function handleLinkifier(pattern, matches) { + let url = linkifier_map.get(pattern); + + let current_group = 1; + + for (const match of matches) { + const back_ref = "\\" + current_group; + url = url.replace(back_ref, match); + current_group += 1; + } + + return url; +} + +function python_to_js_linkifier(pattern, url) { + // Converts a python named-group regex to a javascript-compatible numbered + // group regex... with a regex! + const named_group_re = /\(?P<([^>]+?)>/g; + let match = named_group_re.exec(pattern); + let current_group = 1; + while (match) { + const name = match[1]; + // Replace named group with regular matching group + pattern = pattern.replace("(?P<" + name + ">", "("); + // Replace named reference in URL to numbered reference + url = url.replace("%(" + name + ")s", "\\" + current_group); + + // Reset the RegExp state + named_group_re.lastIndex = 0; + match = named_group_re.exec(pattern); + + current_group += 1; + } + // Convert any python in-regex flags to RegExp flags + let js_flags = "g"; + const inline_flag_re = /\(\?([Limsux]+)\)/; + match = inline_flag_re.exec(pattern); + + // JS regexes only support i (case insensitivity) and m (multiline) + // flags, so keep those and ignore the rest + if (match) { + const py_flags = match[1].split(""); + + for (const flag of py_flags) { + if ("im".includes(flag)) { + js_flags += flag; + } + } + + pattern = pattern.replace(inline_flag_re, ""); + } + // Ideally we should have been checking that linkifiers + // begin with certain characters but since there is no + // support for negative lookbehind in javascript, we check + // for this condition in `contains_backend_only_syntax()` + // function. If the condition is satisfied then the message + // is rendered locally, otherwise, we return false there and + // message is rendered on the backend which has proper support + // for negative lookbehind. + pattern = pattern + /(?!\w)/.source; + let final_regex = null; + try { + final_regex = new RegExp(pattern, js_flags); + } catch (error) { + // We have an error computing the generated regex syntax. + // We'll ignore this linkifier for now, but log this + // failure for debugging later. + blueslip.error("python_to_js_linkifier: " + error.message); + } + return [final_regex, url]; +} + +export function update_linkifier_rules(linkifiers) { + // Update the marked parser with our particular set of linkifiers + linkifier_map.clear(); + linkifier_list = []; + + const marked_rules = []; + + for (const linkifier of linkifiers) { + const [regex, final_url] = python_to_js_linkifier(linkifier.pattern, linkifier.url_format); + if (!regex) { + // Skip any linkifiers that could not be converted + continue; + } + + linkifier_map.set(regex, final_url); + linkifier_list.push({ + pattern: regex, + url_format: final_url, + }); + marked_rules.push(regex); + } + + marked.InlineLexer.rules.zulip.linkifiers = marked_rules; +} + +export function initialize(linkifiers) { + update_linkifier_rules(linkifiers); + + marked.setOptions({linkifierHandler: handleLinkifier}); +} diff --git a/static/js/markdown.js b/static/js/markdown.js index 02dc8a949c..2882fe3de8 100644 --- a/static/js/markdown.js +++ b/static/js/markdown.js @@ -7,6 +7,7 @@ import * as fenced_code from "../shared/js/fenced_code"; import marked from "../third/marked/lib/marked"; import * as blueslip from "./blueslip"; +import * as linkifiers from "./linkifiers"; import * as message_store from "./message_store"; // This contains zulip's frontend Markdown implementation; see @@ -23,9 +24,6 @@ import * as message_store from "./message_store"; // for example usage. let helpers; -const linkifier_map = new Map(); -let linkifier_list = []; - // Regexes that match some of our common backend-only Markdown syntax const backend_only_markdown_re = [ // Inline image previews, check for contiguous chars ending in image suffix @@ -88,6 +86,7 @@ export function contains_backend_only_syntax(content) { // If a linkifier doesn't start with some specified characters // then don't render it locally. It is workaround for the fact that // javascript regex doesn't support lookbehind. + const linkifier_list = linkifiers.linkifier_list; const false_linkifier_match = linkifier_list.find((re) => { const pattern = /[^\s"'(,:<]/.source + re.pattern.source + /(?!\w)/.source; const regex = new RegExp(pattern); @@ -223,6 +222,7 @@ export function add_topic_links(message) { } const topic = message.topic; const links = []; + const linkifier_list = linkifiers.linkifier_list; for (const linkifier of linkifier_list) { const pattern = linkifier.pattern; @@ -359,20 +359,6 @@ function handleStreamTopic(stream_name, topic) { )}" href="/${_.escape(href)}">${_.escape(text)}`; } -function handleLinkifier(pattern, matches) { - let url = linkifier_map.get(pattern); - - let current_group = 1; - - for (const match of matches) { - const back_ref = "\\" + current_group; - url = url.replace(back_ref, match); - current_group += 1; - } - - return url; -} - function handleTex(tex, fullmatch) { try { return katex.renderToString(tex); @@ -386,90 +372,7 @@ function handleTex(tex, fullmatch) { } } -function python_to_js_linkifier(pattern, url) { - // Converts a python named-group regex to a javascript-compatible numbered - // group regex... with a regex! - const named_group_re = /\(?P<([^>]+?)>/g; - let match = named_group_re.exec(pattern); - let current_group = 1; - while (match) { - const name = match[1]; - // Replace named group with regular matching group - pattern = pattern.replace("(?P<" + name + ">", "("); - // Replace named reference in URL to numbered reference - url = url.replace("%(" + name + ")s", "\\" + current_group); - - // Reset the RegExp state - named_group_re.lastIndex = 0; - match = named_group_re.exec(pattern); - - current_group += 1; - } - // Convert any python in-regex flags to RegExp flags - let js_flags = "g"; - const inline_flag_re = /\(\?([Limsux]+)\)/; - match = inline_flag_re.exec(pattern); - - // JS regexes only support i (case insensitivity) and m (multiline) - // flags, so keep those and ignore the rest - if (match) { - const py_flags = match[1].split(""); - - for (const flag of py_flags) { - if ("im".includes(flag)) { - js_flags += flag; - } - } - - pattern = pattern.replace(inline_flag_re, ""); - } - // Ideally we should have been checking that linkifiers - // begin with certain characters but since there is no - // support for negative lookbehind in javascript, we check - // for this condition in `contains_backend_only_syntax()` - // function. If the condition is satisfied then the message - // is rendered locally, otherwise, we return false there and - // message is rendered on the backend which has proper support - // for negative lookbehind. - pattern = pattern + /(?!\w)/.source; - let final_regex = null; - try { - final_regex = new RegExp(pattern, js_flags); - } catch (error) { - // We have an error computing the generated regex syntax. - // We'll ignore this linkifier for now, but log this - // failure for debugging later. - blueslip.error("python_to_js_linkifier: " + error.message); - } - return [final_regex, url]; -} - -export function update_linkifier_rules(linkifiers) { - // Update the marked parser with our particular set of linkifiers - linkifier_map.clear(); - linkifier_list = []; - - const marked_rules = []; - - for (const linkifier of linkifiers) { - const [regex, final_url] = python_to_js_linkifier(linkifier.pattern, linkifier.url_format); - if (!regex) { - // Skip any linkifiers that could not be converted - continue; - } - - linkifier_map.set(regex, final_url); - linkifier_list.push({ - pattern: regex, - url_format: final_url, - }); - marked_rules.push(regex); - } - - marked.InlineLexer.rules.zulip.linkifiers = marked_rules; -} - -export function initialize(linkifiers, helper_config) { +export function initialize(helper_config) { helpers = helper_config; function disable_markdown_regex(rules, name) { @@ -528,8 +431,6 @@ export function initialize(linkifiers, helper_config) { // Disable autolink as (a) it is not used in our backend and (b) it interferes with @mentions disable_markdown_regex(marked.InlineLexer.rules.zulip, "autolink"); - update_linkifier_rules(linkifiers); - // Tell our fenced code preprocessor how to insert arbitrary // HTML into the output. This generated HTML is safe to not escape fenced_code.set_stash_func((html) => marked.stashHtml(html, true)); @@ -547,7 +448,6 @@ export function initialize(linkifiers, helper_config) { unicodeEmojiHandler: handleUnicodeEmoji, streamHandler: handleStream, streamTopicHandler: handleStreamTopic, - linkifierHandler: handleLinkifier, texHandler: handleTex, timestampHandler: handleTimestamp, renderer: r, diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index be73a1963e..cc623565e1 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -14,7 +14,7 @@ import * as composebox_typeahead from "./composebox_typeahead"; import * as emoji_picker from "./emoji_picker"; import * as giphy from "./giphy"; import * as hotspots from "./hotspots"; -import * as markdown from "./markdown"; +import * as linkifiers from "./linkifiers"; import * as message_edit from "./message_edit"; import * as message_events from "./message_events"; import * as message_flags from "./message_flags"; @@ -333,7 +333,7 @@ export function dispatch_normal_event(event) { case "realm_linkifiers": page_params.realm_linkifiers = event.realm_linkifiers; - markdown.update_linkifier_rules(page_params.realm_linkifiers); + linkifiers.update_linkifier_rules(page_params.realm_linkifiers); settings_linkifiers.populate_linkifiers(page_params.realm_linkifiers); break; diff --git a/static/js/ui_init.js b/static/js/ui_init.js index 2d44a837bc..973bb9ce20 100644 --- a/static/js/ui_init.js +++ b/static/js/ui_init.js @@ -28,6 +28,7 @@ import * as hashchange from "./hashchange"; import * as hotspots from "./hotspots"; import * as invite from "./invite"; import * as lightbox from "./lightbox"; +import * as linkifiers from "./linkifiers"; import * as markdown from "./markdown"; import * as markdown_config from "./markdown_config"; import * as message_edit from "./message_edit"; @@ -503,7 +504,8 @@ export function initialize_everything() { realm_emoji: emoji_params.realm_emoji, emoji_codes: generated_emoji_codes, }); - markdown.initialize(page_params.realm_linkifiers, markdown_config.get_helpers()); + markdown.initialize(markdown_config.get_helpers()); + linkifiers.initialize(page_params.realm_linkifiers); realm_playground.initialize(page_params.realm_playgrounds, generated_pygments_data); composebox_typeahead.initialize(); // Must happen after compose.initialize() search.initialize();