Merge pull request #14875 from overleaf/mj-linter-href

[web] Allow url characters in href argument in linter

GitOrigin-RevId: c62ee94003328286d1b1c2b3f9e8ee59f97f8139

services/web/frontend/js/features/source-editor/languages/latex/linter/latex-linter.worker.js

@@ -512,10 +512,12 @@ const readVerb = function (TokeniseResult, k) {
   return null
 }
 
-const readUrl = function (TokeniseResult, k) {
+const readUrl = function (TokeniseResult, k, options) {
   // read a url argument
-  // \url|foo|
+  // \url|https://example.com|
-  // \url{foo}
+  // \url{https://example.com}
+  // \href|https://example.com|
+  // \href{https://example.com}
 
   // Note: this is only an approximation, because we have already
   // tokenised the input stream, so anything after a comment
@@ -525,13 +527,22 @@ const readUrl = function (TokeniseResult, k) {
   const Tokens = TokeniseResult.tokens
   const text = TokeniseResult.text
 
-  const urlToken = Tokens[k]
+  // Tokens[k] is the href or url control sequence
-  // const urlStr = text.substring(urlToken[2], urlToken[3])
 
-  // start looking at text immediately after \url command
+  if (!Tokens[k + 1]) {
-  let pos = urlToken[3]
+    return null
-  const openDelimiter = text[pos]
+  }
-  const closeDelimiter = openDelimiter === '{' ? '}' : openDelimiter
+
+  let pos = Tokens[k + 1][2]
+  let openDelimiter = '{'
+  let closeDelimiter = '}'
+
+  if (options && options.allowCustomDelimiter) {
+    openDelimiter = text[pos]
+    closeDelimiter = openDelimiter === '{' ? '}' : openDelimiter
+  } else if (text[pos] !== openDelimiter) {
+    return null
+  }
 
   // Was the delimiter a token? if so, advance token index
   let nextToken = Tokens[k + 1]
@@ -869,11 +880,17 @@ const InterpretTokens = function (TokeniseResult, ErrorReporter) {
         } else {
           i = newPos
         }
-      } else if (seq === 'url') {
+      } else if (seq === 'url' || seq === 'href') {
         // \url{...} or \url|....|  where | = any char
-        const newPos = readUrl(TokeniseResult, i)
+        // \href{...} or \href|....|  where | = any char
+        // href has a second argument, which is the text to show for the url.
+        // We leave that alone here, since it can contain math mode, which we
+        // want to parse with the rest of the machinery.
+        const newPos = readUrl(TokeniseResult, i, {
+          allowCustomDelimiter: seq === 'url',
+        })
         if (newPos === null) {
-          TokenError(token, 'invalid url command')
+          TokenError(token, `invalid ${seq} command`)
         } else {
           i = newPos
         }

services/web/test/frontend/features/source-editor/languages/latex/latex-linter.test.ts

@@ -239,6 +239,40 @@ describe('LatexLinter', function () {
     assert.equal(errors.length, 0)
   })
 
+  it('should accept \\href{...}{...}', function () {
+    const { errors } = Parse(
+      'this is text \\href{http://www.sharelatex.com/}{test} and more\n'
+    )
+    assert.equal(errors.length, 0)
+  })
+
+  it('should accept \\href{...}{...} with dollarsign in url', function () {
+    const { errors } = Parse(
+      'this is text \\href{http://www.sharelatex.com/foo=$bar}{test} and more\n'
+    )
+    assert.equal(errors.length, 0)
+  })
+
+  it('should not accept \\href|...|{...}', function () {
+    const { errors } = Parse(
+      'this is text \\href|http://www.sharelatex.com|{test} and more\n'
+    )
+    assert.equal(errors.length, 1)
+    assert.equal(errors[0].text, 'invalid href command')
+    assert.equal(errors[0].type, 'error')
+  })
+
+  it('should catch error in text argument of \\href{...}{...}', function () {
+    const { errors } = Parse(
+      'this is text \\href{http://www.sharelatex.com/foo=$bar}{i have made an $error} and more\n'
+    )
+    assert.equal(errors.length, 2)
+    assert.equal(errors[0].text, 'unclosed $ found at close group }')
+    assert.equal(errors[0].type, 'error')
+    assert.equal(errors[1].text, 'unexpected close group } after $')
+    assert.equal(errors[1].type, 'error')
+  })
+
   it('should accept \\left( and \\right)', function () {
     const { errors } = Parse('math $\\left( x + y \\right) = y + x$ and more\n')
     assert.equal(errors.length, 0)