Skip to content

Commit

Permalink
Changed checking short links to loops instead of regex
Browse files Browse the repository at this point in the history
  • Loading branch information
@DimitryOrlov
DimitryOrlov committed Dec 12, 2024
1 parent 36c7eaa commit 6cbac4c
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 18 deletions.
60 changes: 50 additions & 10 deletions common/editorscommon.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2911,9 +2911,6 @@
rx_ref3D_quoted = new XRegExp("^'(?<name_from>(?:''|[^\\[\\]'\\/*?:])*)(?::(?<name_to>(?:''|[^\\[\\]'\\/*?:])*))?'!"),
rx_ref3D_non_quoted_2 = new XRegExp("^(?<name_from>[" + str_namedRanges + "\\d][" + str_namedRanges + "\\d.]*)(:(?<name_to>[" + str_namedRanges + "\\d][" + str_namedRanges + "\\d.]*))?!", "i"),
rx_ref3D = new XRegExp("^(?<name_from>[^:]+)(:(?<name_to>[^:]+))?!"),
// TODO rework by cycle
rx_ref3D_short = /^\!{1}([^\s\\]+)/i, // short links that ms writes as [externalLink] + "!" + "Defname"
rx_ref3D_short_local = /^([^'\[\]]+)!\s*(.+)$/i, // short links that user writes as "'externalLinkWithoutBrackets'" + "!" + "Defname" - "'DefTest.xlsx'!_s1"
rx_ref_external = /^(\[{1}(\d*)\]{1})/,
rx_ref_external2 = /^(\[{1}(\d*)\]{1})/,
rx_number = /^ *[+-]?\d*(\d|\.)\d*([eE][+-]?\d+)?/,
Expand Down Expand Up @@ -3015,6 +3012,48 @@
return null;
}

function isExternalShortLink (string) {
// short links that ms writes as [externalLink] + "!" + "Defname"
// strings come in "!"+"Defname" format
if (string[0] !== "!") {
return null;
}
// we go through the characters after the first
for (let i = 1; i < string.length; i++) {
let char = string[i];

// if the character is a space or backslash, return null
if (char === ' ' || char === '\\' || char === "!") {
return null;
}
}
return ["!" + string, string];
}

function isExternalShortLinkLocal (string) {
// short links that user writes as "'externalLinkWithoutBrackets'" + "!" + "Defname" - "'DefTest.xlsx'!_s1"
// we split the string into two parts, where the separator is an exclamation point
if (!string) {
return null;
}

let stringArray = string.split("!");
if (!stringArray || !stringArray[0] || !stringArray[1] ||
stringArray[0].includes("!") || stringArray[0].includes("[") || stringArray[0].includes("]") ||
stringArray[1].includes("!") || stringArray[1].includes("[") || stringArray[1].includes("]")) {
return null;
}

// check if the second part can be defname - parserHelp.isName()
let ph = {operand_str: stringArray[1], pCurrPos: 0};
let canBeDefName = parserHelp.isName.call(ph, stringArray[1], ph.pCurrPos);
if (!canBeDefName) {
return null;
}

return ["!" + stringArray[1], stringArray[1]];
}

function isValidFileUrl(url) {
if(!url.startsWith("file:")) {
return false;
Expand Down Expand Up @@ -3450,8 +3489,9 @@
}
}

let canBeShortLink = XRegExp.exec(subSTR, rx_ref3D_short) || XRegExp.exec(subSTR, rx_ref3D_short_local);
let match = XRegExp.exec(subSTR, rx_ref3D_quoted) || XRegExp.exec(subSTR, rx_ref3D_non_quoted) || canBeShortLink;
let shortLink = isExternalShortLink(subSTR) || isExternalShortLinkLocal(subSTR);
let match = XRegExp.exec(subSTR, rx_ref3D_quoted) || XRegExp.exec(subSTR, rx_ref3D_non_quoted);

if(!match && support_digital_start) {
match = XRegExp.exec(subSTR, rx_ref3D_non_quoted_2);
}
Expand All @@ -3461,11 +3501,11 @@
this.pCurrPos += match[0].length + externalLength;
this.operand_str = match[1];

if (!match["name_from"] && !match["name_to"] && canBeShortLink) {
return [true, null, null, external, subSTR];
}

return [true, match["name_from"] ? match["name_from"].replace(/''/g, "'") : null, match["name_to"] ? match["name_to"].replace(/''/g, "'") : null, external, canBeShortLink ? subSTR : null];
return [true, match["name_from"] ? match["name_from"].replace(/''/g, "'") : null, match["name_to"] ? match["name_to"].replace(/''/g, "'") : null, external, shortLink ? subSTR : null];
} else if (shortLink) {
this.pCurrPos += shortLink[0].length + externalLength;
this.operand_str = shortLink[1];
return [true, null, null, external, subSTR];
}
return [false, null, null, external, externalLength];
};
Expand Down
19 changes: 11 additions & 8 deletions tests/cell/spreadsheet-calculation/ExternalReference.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ $(function () {
fullLinkDefname = "'[1]Sheet1'!_s1",
shortLinkLocal = "'[book.xlsx]'!A1",
shortLinkDefnameLocal = "[book.xlsx]!_s1",
shortLinkDefnameLocalWithoutBrackets = "book.xlsx!_s1",
shortLink = "[1]!A1",
shortLinkDefname = "[1]!_s1",
externalWs;
Expand Down Expand Up @@ -1030,30 +1031,32 @@ $(function () {

// try to parse string to external ref similiar as read the file
oParser = new parserFormula(fullLink, cellWithFormula, ws);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Full link. isLocal = false." + fullLink);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Full link. isLocal = false. " + fullLink);

oParser = new parserFormula(fullLinkDefname, cellWithFormula, ws);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Full link to defname. isLocal = false." + fullLinkDefname);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Full link to defname. isLocal = false. " + fullLinkDefname);

oParser = new parserFormula(shortLink, cellWithFormula, ws);
assert.ok(!oParser.parse(false/*isLocal*/, null, parseResult), "Short link. isLocal = false." + shortLink);
assert.ok(!oParser.parse(false/*isLocal*/, null, parseResult), "Short link. isLocal = false. " + shortLink);

oParser = new parserFormula(shortLinkDefname, cellWithFormula, ws);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Short link to defname. isLocal = false." + shortLinkDefname);
assert.ok(oParser.parse(false/*isLocal*/, null, parseResult), "Short link to defname. isLocal = false. " + shortLinkDefname);

// try parse string to external ref similiar as writing a string manually
oParser = new parserFormula(fullLinkLocal, cellWithFormula, ws);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult), "Full link. isLocal = true." + fullLinkLocal);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult), "Full link. isLocal = true. " + fullLinkLocal);

oParser = new parserFormula(fullLinkDefnameLocal, cellWithFormula, ws);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult), "Full link to defname. isLocal = true." + fullLinkDefnameLocal);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult), "Full link to defname. isLocal = true. " + fullLinkDefnameLocal);

oParser = new parserFormula(shortLinkLocal, cellWithFormula, ws);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult) === false, "Short link. isLocal = true." + shortLinkLocal);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult) === false, "Short link. isLocal = true. " + shortLinkLocal);

oParser = new parserFormula(shortLinkDefnameLocal, cellWithFormula, ws);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult) === false, "Short link to defname. isLocal = true." + shortLinkDefnameLocal);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult) === false, "Short link to defname. isLocal = true. " + shortLinkDefnameLocal);

oParser = new parserFormula(shortLinkDefnameLocalWithoutBrackets, cellWithFormula, ws);
assert.ok(oParser.parse(true/*isLocal*/, null, parseResult), "Short link to defname without brackets. isLocal = true. " + shortLinkDefnameLocalWithoutBrackets);

//remove external reference
wb.removeExternalReferences([wb.externalReferences[0].getAscLink()]);
Expand Down

0 comments on commit 6cbac4c

Please sign in to comment.