Skip to content

Commit

Permalink
[FIX] Cell: Fix detection of markdown links
Browse files Browse the repository at this point in the history
The current regex of markdown link would purposely discard any
`[label](link)` combination for which the label contained an opening
bracket `[`, the reason behind it was to reject strings for which the label
itself contained a markdown link as the markdown spec [1] specifies that we
capture square brackets immediately followed by parenthesis. However, we
should still support a label that contains closed square brackets
(e.g. `[Label has [brackets]](link)`. Applying the proper rule to detect
an exact link would be more costy than the current regex and considering
that it will be called on all evaluated cell when reevaluated.

Since the current regex is too restrictive, this revision relaxes it with
the downside that it will now also capture strings that are technically not
exact markdown links but better have false positive than false negatives.
The regex also becomes simpler.

[1] https://spec-md.com/#sec-Links

closes #3435

Task: 3628780
X-original-commit: a93f0cd
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
  • Loading branch information
rrahir committed Jan 15, 2024
1 parent 93874ce commit f3407c4
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/helpers/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export function isDateTime(str: string): boolean {
return parseDateTime(str) !== null;
}

const MARKDOWN_LINK_REGEX = /^\[([^\[]+)\]\((.+)\)$/;
const MARKDOWN_LINK_REGEX = /^\[(.+)\]\((.+)\)$/;
//link must start with http or https
//https://stackoverflow.com/a/3809435/4760614
const WEB_LINK_REGEX =
Expand Down
16 changes: 12 additions & 4 deletions tests/plugins/cell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,22 @@ describe("link cell", () => {
const cell = getEvaluatedCell(model, "A1");
expect(cell.value).toBe(markdown);
expect(cell.type).toBe(CellValueType.text);
expect(cell.link).toBeFalsy();
}
);

test("a markdown link in a markdown link", () => {
test.each([
["[label](url)", "label", "https://url"],
["[[label](link)](http://odoo.com)", "[label](link)", "http://odoo.com"],
["[lab[el](url)", "lab[el", "https://url"],
["[lab]el](url)", "lab]el", "https://url"],
["[[label]](url)", "[label]", "https://url"],
])("valid markdown %s is recognized as link", (markdown, label, link) => {
const model = new Model();
setCellContent(model, "A1", `[[label](link)](http://odoo.com)`);
expect(getEvaluatedCell(model, "A1").type).toBe(CellValueType.text);
expect(getCell(model, "A1")?.content).toBe("[[label](link)](http://odoo.com)");
setCellContent(model, "A1", markdown);
const cell = getEvaluatedCell(model, "A1");
expect(cell.link?.label).toBe(label);
expect(cell.link?.url).toBe(link);
});

test("can create a sheet link", () => {
Expand Down

0 comments on commit f3407c4

Please sign in to comment.