From 0571145029a5ab5cf81c28cb3f837b90b9f91469 Mon Sep 17 00:00:00 2001 From: evykassirer Date: Sun, 21 Jan 2024 23:10:12 -0800 Subject: [PATCH] markdown: Remove wrapper around get_topic_links. Since it's only used in one place, and all callers of it user the same value for the linkifier. https://github.com/zulip/zulip/pull/28652#discussion_r1470516258 --- web/src/echo.js | 6 +++++- web/src/markdown.js | 19 +++++-------------- web/tests/compose.test.js | 2 +- web/tests/echo.test.js | 16 +++++----------- web/tests/markdown.test.js | 20 ++++++++++---------- web/tests/markdown_parse.test.js | 8 ++++---- 6 files changed, 30 insertions(+), 41 deletions(-) diff --git a/web/src/echo.js b/web/src/echo.js index f03b657d1b..7a54bbf4e4 100644 --- a/web/src/echo.js +++ b/web/src/echo.js @@ -192,7 +192,11 @@ export function insert_local_message(message_request, local_id_float, insert_new message.local_id = local_id_float.toString(); message.locally_echoed = true; message.id = local_id_float; - markdown.add_topic_links(message); + if (message.topic === undefined) { + message.topic_links = []; + } else { + message.topic_links = markdown.get_topic_links(message.topic); + } waiting_for_id.set(message.local_id, message); waiting_for_ack.set(message.local_id, message); diff --git a/web/src/markdown.js b/web/src/markdown.js index 490cb53e3b..fd97dbb8d9 100644 --- a/web/src/markdown.js +++ b/web/src/markdown.js @@ -30,6 +30,8 @@ function contains_preview_link(content) { return preview_regexes.some((re) => re.test(content)); } +let web_app_helpers; + export function translate_emoticons_to_names({src, get_emoticon_translations}) { // Translates emoticons in a string to their colon syntax. let translated = src; @@ -318,13 +320,15 @@ function is_overlapping(match_a, match_b) { ); } -export function get_topic_links({topic, get_linkifier_map}) { +export function get_topic_links(topic) { // We export this for testing purposes, and mobile may want to // use this as well in the future. const links = []; // The lower the precedence is, the more prioritized the pattern is. let precedence = 0; + const get_linkifier_map = web_app_helpers.get_linkifier_map; + for (const [pattern, {url_template, group_number_to_name}] of get_linkifier_map().entries()) { let match; while ((match = pattern.exec(topic)) !== null) { @@ -667,8 +671,6 @@ export function parse({raw_content, helper_config}) { // We may eventually move this code to a new file, but we want // to wait till the dust settles a bit on some other changes first. -let web_app_helpers; - export function initialize(helper_config) { // This is generally only intended to be called by the web app. Most // other platforms should call setup(). @@ -686,17 +688,6 @@ export function render(raw_content) { }; } -export function add_topic_links(message) { - if (message.type !== "stream") { - message.topic_links = []; - return; - } - message.topic_links = get_topic_links({ - topic: message.topic, - get_linkifier_map: web_app_helpers.get_linkifier_map, - }); -} - export function contains_backend_only_syntax(content) { return content_contains_backend_only_syntax({ content, diff --git a/web/tests/compose.test.js b/web/tests/compose.test.js index 58d0969d51..5bc7d97d91 100644 --- a/web/tests/compose.test.js +++ b/web/tests/compose.test.js @@ -236,7 +236,7 @@ test_ui("send_message", ({override, override_rewire, mock_template}) => { const server_message_id = 127; override(markdown, "render", noop); - override(markdown, "add_topic_links", noop); + override(markdown, "get_topic_links", noop); override_rewire(echo, "try_deliver_locally", (message_request) => { const local_id_float = 123.04; diff --git a/web/tests/echo.test.js b/web/tests/echo.test.js index 372b45f1d8..80b72d7a2a 100644 --- a/web/tests/echo.test.js +++ b/web/tests/echo.test.js @@ -222,15 +222,15 @@ run_test("insert_local_message streams", ({override}) => { const local_id_float = 101.01; let render_called = false; - let add_topic_links_called = false; + let get_topic_links_called = false; let insert_message_called = false; override(markdown, "render", () => { render_called = true; }); - override(markdown, "add_topic_links", () => { - add_topic_links_called = true; + override(markdown, "get_topic_links", () => { + get_topic_links_called = true; }); const insert_new_messages = ([message]) => { @@ -245,6 +245,7 @@ run_test("insert_local_message streams", ({override}) => { const message_request = { type: "stream", stream_id: general_sub.stream_id, + topic: "important note", sender_email: "iago@zulip.com", sender_full_name: "Iago", sender_id: 123, @@ -252,7 +253,7 @@ run_test("insert_local_message streams", ({override}) => { echo.insert_local_message(message_request, local_id_float, insert_new_messages); assert.ok(render_called); - assert.ok(add_topic_links_called); + assert.ok(get_topic_links_called); assert.ok(insert_message_called); }); @@ -273,7 +274,6 @@ run_test("insert_local_message direct message", ({override}) => { params.cross_realm_bots = []; people.initialize(page_params.user_id, params); - let add_topic_links_called = false; let render_called = false; let insert_message_called = false; @@ -286,10 +286,6 @@ run_test("insert_local_message direct message", ({override}) => { render_called = true; }); - override(markdown, "add_topic_links", () => { - add_topic_links_called = true; - }); - const message_request = { private_message_recipient: "cordelia@zulip.com,hamlet@zulip.com", type: "private", @@ -298,7 +294,6 @@ run_test("insert_local_message direct message", ({override}) => { sender_id: 123, }; echo.insert_local_message(message_request, local_id_float, insert_new_messages); - assert.ok(add_topic_links_called); assert.ok(render_called); assert.ok(insert_message_called); }); @@ -307,7 +302,6 @@ run_test("test reify_message_id", ({override}) => { const local_id_float = 103.01; override(markdown, "render", noop); - override(markdown, "add_topic_links", noop); const message_request = { type: "stream", diff --git a/web/tests/markdown.test.js b/web/tests/markdown.test.js index d867ea4250..c1a5c57de3 100644 --- a/web/tests/markdown.test.js +++ b/web/tests/markdown.test.js @@ -640,11 +640,11 @@ test("marked", () => { test("topic_links", () => { let message = {type: "stream", topic: "No links here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 0); message = {type: "stream", topic: "One #123 link here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 1); assert.deepEqual(message.topic_links[0], { url: "https://trac.example.com/ticket/123", @@ -652,7 +652,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "Two #123 #456 link here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 2); assert.deepEqual(message.topic_links[0], { url: "https://trac.example.com/ticket/123", @@ -664,7 +664,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "New ZBUG_123 link here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 1); assert.deepEqual(message.topic_links[0], { url: "https://trac2.zulip.net/ticket/123", @@ -672,7 +672,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "New ZBUG_123 with #456 link here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 2); assert.deepEqual(message.topic_links[0], { url: "https://trac2.zulip.net/ticket/123", @@ -684,7 +684,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "One ZGROUP_123:45 link here"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 1); assert.deepEqual(message.topic_links[0], { url: "https://zone_45.zulip.net/ticket/123", @@ -692,7 +692,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "Hello https://google.com"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 1); assert.deepEqual(message.topic_links[0], { url: "https://google.com", @@ -700,7 +700,7 @@ test("topic_links", () => { }); message = {type: "stream", topic: "#456 https://google.com https://github.com"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 3); assert.deepEqual(message.topic_links[0], { url: "https://trac.example.com/ticket/456", @@ -716,11 +716,11 @@ test("topic_links", () => { }); message = {type: "not-stream"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 0); message = {type: "stream", topic: "FOO_abcde;e;zulip;luxembourg;foo;23;testing"}; - markdown.add_topic_links(message); + message.topic_links = markdown.get_topic_links(message.topic); assert.equal(message.topic_links.length, 1); assert.deepEqual(message.topic_links[0], { url: "https://zone_e.zulip.net/ticket/luxembourg/abcde?name=foo&chapter=23#testi", diff --git a/web/tests/markdown_parse.test.js b/web/tests/markdown_parse.test.js index 1e53500078..274902fc69 100644 --- a/web/tests/markdown_parse.test.js +++ b/web/tests/markdown_parse.test.js @@ -223,15 +223,15 @@ run_test("linkifiers", () => { }); function assert_topic_links(topic, expected_links) { - const topic_links = markdown.get_topic_links({ - topic, - get_linkifier_map: linkifiers.get_linkifier_map, - }); + const topic_links = markdown.get_topic_links(topic); assert.deepEqual(topic_links, expected_links); } run_test("topic links", () => { linkifiers.initialize([{pattern: "#foo(?P\\d+)", url_template: "http://foo.com/{id}"}]); + markdown.initialize({ + get_linkifier_map: linkifiers.get_linkifier_map, + }); assert_topic_links("progress on #foo101 and #foo102", [ { text: "#foo101",