Merge pull request #4944 from overleaf/bg-use-file-line-errors

use file line errors in log output

GitOrigin-RevId: 6732b19552fe15431a70fbefbc572253c389c64e

services/clsi/test/unit/js/LatexRunnerTests.js

@@ -175,7 +175,7 @@ describe('LatexRunner', function () {
             compiler: this.compiler,
             image: this.image,
             timeout: (this.timeout = 42000),
-            flags: ['-file-line-error', '-halt-on-error'],
+            flags: ['-shell-restricted', '-halt-on-error'],
           },
           this.callback
         )
@@ -184,10 +184,10 @@ describe('LatexRunner', function () {
       return it('should include the flags in the command', function () {
         const command = this.CommandRunner.run.args[0][1]
         const flags = command.filter(
-          arg => arg === '-file-line-error' || arg === '-halt-on-error'
+          arg => arg === '-shell-restricted' || arg === '-halt-on-error'
         )
         flags.length.should.equal(2)
-        flags[0].should.equal('-file-line-error')
+        flags[0].should.equal('-shell-restricted')
         return flags[1].should.equal('-halt-on-error')
       })
     })

services/web/app/src/Features/Compile/ClsiManager.js

@@ -787,6 +787,7 @@ const ClsiManager = {
 
   _finaliseRequest(projectId, options, project, docs, files, callback) {
     const resources = []
+    let flags
     let rootResourcePath = null
     let rootResourcePathOverride = null
     let hasMainFile = false
@@ -847,6 +848,10 @@ const ClsiManager = {
       })
     }
 
+    if (options.fileLineErrors) {
+      flags = ['-file-line-error']
+    }
+
     callback(null, {
       compile: {
         options: {
@@ -860,6 +865,7 @@ const ClsiManager = {
           compileGroup: options.compileGroup,
           enablePdfCaching:
             (Settings.enablePdfCaching && options.enablePdfCaching) || false,
+          flags: flags,
         },
         rootResourcePath,
         resources,

services/web/app/src/Features/Compile/CompileController.js

@@ -44,10 +44,12 @@ module.exports = CompileController = {
     const project_id = req.params.Project_id
     const isAutoCompile = !!req.query.auto_compile
     const enablePdfCaching = !!req.query.enable_pdf_caching
+    const fileLineErrors = !!req.query.file_line_errors
     const user_id = SessionManager.getLoggedInUserId(req.session)
     const options = {
       isAutoCompile,
       enablePdfCaching,
+      fileLineErrors,
     }
 
     if (req.body.rootDoc_id) {

services/web/frontend/js/ide/pdf/controllers/PdfController.js

@@ -283,6 +283,9 @@ App.controller(
       if (getMeta('ol-enablePdfCaching')) {
         params.enable_pdf_caching = true
       }
+      if (window.location.search.includes('file_line_errors=true')) {
+        params.file_line_errors = 'true'
+      }
       // if the previous run was a check, clear the error logs
       if ($scope.check) {
         $scope.pdf.logEntries = {}

services/web/frontend/js/vendor/libs/latex-log-parser.js

@@ -1,298 +1,366 @@
-// Generated by CoffeeScript 1.10.0
-define(function() {
-  var HBOX_WARNING_REGEX, LATEX_WARNING_REGEX, LINES_REGEX, LOG_WRAP_LIMIT, LatexParser, LogText, PACKAGE_REGEX, PACKAGE_WARNING_REGEX, state;
-  LOG_WRAP_LIMIT = 79;
-  LATEX_WARNING_REGEX = /^LaTeX Warning: (.*)$/;
-  HBOX_WARNING_REGEX = /^(Over|Under)full \\(v|h)box/;
-  PACKAGE_WARNING_REGEX = /^(Package \b.+\b Warning:.*)$/;
-  LINES_REGEX = /lines? ([0-9]+)/;
-  PACKAGE_REGEX = /^Package (\b.+\b) Warning/;
-  LogText = function(text) {
-    var i, wrappedLines;
-    this.text = text.replace(/(\r\n)|\r/g, '\n');
-    wrappedLines = this.text.split('\n');
-    this.lines = [wrappedLines[0]];
-    i = 1;
+define(function () {
+  // Define some constants
+  const LOG_WRAP_LIMIT = 79
+  const LATEX_WARNING_REGEX = /^LaTeX Warning: (.*)$/
+  const HBOX_WARNING_REGEX = /^(Over|Under)full \\(v|h)box/
+  const PACKAGE_WARNING_REGEX = /^(Package \b.+\b Warning:.*)$/
+  // This is used to parse the line number from common latex warnings
+  const LINES_REGEX = /lines? ([0-9]+)/
+  // This is used to parse the package name from the package warnings
+  const PACKAGE_REGEX = /^Package (\b.+\b) Warning/
+
+  const LogText = function (text) {
+    this.text = text.replace(/(\r\n)|\r/g, '\n')
+    // Join any lines which look like they have wrapped.
+    const wrappedLines = this.text.split('\n')
+    this.lines = [wrappedLines[0]]
+    let i = 1
     while (i < wrappedLines.length) {
-      if (wrappedLines[i - 1].length === LOG_WRAP_LIMIT && wrappedLines[i - 1].slice(-3) !== '...') {
-        this.lines[this.lines.length - 1] += wrappedLines[i];
+      // If the previous line is as long as the wrap limit then
+      // append this line to it.
+      // Some lines end with ... when LaTeX knows it's hit the limit
+      // These shouldn't be wrapped.
+      if (
+        wrappedLines[i - 1].length === LOG_WRAP_LIMIT &&
+        wrappedLines[i - 1].slice(-3) !== '...'
+      ) {
+        this.lines[this.lines.length - 1] += wrappedLines[i]
       } else {
-        this.lines.push(wrappedLines[i]);
+        this.lines.push(wrappedLines[i])
       }
-      i++;
+      i++
     }
-    this.row = 0;
-  };
-  (function() {
-    this.nextLine = function() {
-      this.row++;
+    this.row = 0
+  }
+
+  ;(function () {
+    this.nextLine = function () {
+      this.row++
       if (this.row >= this.lines.length) {
-        return false;
+        return false
       } else {
-        return this.lines[this.row];
+        return this.lines[this.row]
       }
-    };
-    this.rewindLine = function() {
-      this.row--;
-    };
-    this.linesUpToNextWhitespaceLine = function() {
-      return this.linesUpToNextMatchingLine(/^ *$/);
-    };
-    this.linesUpToNextMatchingLine = function(match) {
-      var lines, nextLine;
-      lines = [];
-      nextLine = this.nextLine();
+    }
+
+    this.rewindLine = function () {
+      this.row--
+    }
+
+    this.linesUpToNextWhitespaceLine = function () {
+      return this.linesUpToNextMatchingLine(/^ *$/)
+    }
+
+    this.linesUpToNextMatchingLine = function (match) {
+      const lines = []
+      let nextLine = this.nextLine()
       if (nextLine !== false) {
-        lines.push(nextLine);
+        lines.push(nextLine)
       }
-      while (nextLine !== false && !nextLine.match(match) && nextLine !== false) {
-        nextLine = this.nextLine();
+      while (
+        nextLine !== false &&
+        !nextLine.match(match) &&
+        nextLine !== false
+      ) {
+        nextLine = this.nextLine()
         if (nextLine !== false) {
-          lines.push(nextLine);
+          lines.push(nextLine)
         }
       }
-      return lines;
-    };
-  }).call(LogText.prototype);
-  state = {
+      return lines
+    }
+  }.call(LogText.prototype))
+
+  const state = {
     NORMAL: 0,
-    ERROR: 1
-  };
-  LatexParser = function(text, options) {
-    this.log = new LogText(text);
-    this.state = state.NORMAL;
-    options = options || {};
-    this.fileBaseNames = options.fileBaseNames || [/compiles/, /\/usr\/local/];
-    this.ignoreDuplicates = options.ignoreDuplicates;
-    this.data = [];
-    this.fileStack = [];
-    this.currentFileList = this.rootFileList = [];
-    this.openParens = 0;
-  };
-  (function() {
-    this.parse = function() {
-      var lineNo;
+    ERROR: 1,
+  }
+
+  const LatexParser = function (text, options) {
+    this.log = new LogText(text)
+    this.state = state.NORMAL
+    options = options || {}
+    this.fileBaseNames = options.fileBaseNames || [/compiles/, /\/usr\/local/]
+    this.ignoreDuplicates = options.ignoreDuplicates
+    this.data = []
+    this.fileStack = []
+    this.currentFileList = this.rootFileList = []
+    this.openParens = 0
+  }
+
+  ;(function () {
+    this.parse = function () {
       while ((this.currentLine = this.log.nextLine()) !== false) {
         if (this.state === state.NORMAL) {
           if (this.currentLineIsError()) {
-            this.state = state.ERROR;
+            this.state = state.ERROR
             this.currentError = {
               line: null,
               file: this.currentFilePath,
               level: 'error',
               message: this.currentLine.slice(2),
               content: '',
-              raw: this.currentLine + '\n'
-            };
+              raw: this.currentLine + '\n',
+            }
+          } else if (this.currentLineIsFileLineError()) {
+            this.state = state.ERROR
+            this.parseFileLineError()
           } else if (this.currentLineIsRunawayArgument()) {
-            this.parseRunawayArgumentError();
+            this.parseRunawayArgumentError()
           } else if (this.currentLineIsWarning()) {
-            this.parseSingleWarningLine(LATEX_WARNING_REGEX);
+            this.parseSingleWarningLine(LATEX_WARNING_REGEX)
           } else if (this.currentLineIsHboxWarning()) {
-            this.parseHboxLine();
+            this.parseHboxLine()
           } else if (this.currentLineIsPackageWarning()) {
-            this.parseMultipleWarningLine();
+            this.parseMultipleWarningLine()
           } else {
-            this.parseParensForFilenames();
+            this.parseParensForFilenames()
           }
         }
         if (this.state === state.ERROR) {
-          this.currentError.content += this.log.linesUpToNextMatchingLine(/^l\.[0-9]+/).join('\n');
-          this.currentError.content += '\n';
-          this.currentError.content += this.log.linesUpToNextWhitespaceLine().join('\n');
-          this.currentError.content += '\n';
-          this.currentError.content += this.log.linesUpToNextWhitespaceLine().join('\n');
-          this.currentError.raw += this.currentError.content;
-          lineNo = this.currentError.raw.match(/l\.([0-9]+)/);
-          if (lineNo) {
-            this.currentError.line = parseInt(lineNo[1], 10);
+          this.currentError.content += this.log
+            .linesUpToNextMatchingLine(/^l\.[0-9]+/)
+            .join('\n')
+          this.currentError.content += '\n'
+          this.currentError.content += this.log
+            .linesUpToNextWhitespaceLine()
+            .join('\n')
+          this.currentError.content += '\n'
+          this.currentError.content += this.log
+            .linesUpToNextWhitespaceLine()
+            .join('\n')
+          this.currentError.raw += this.currentError.content
+          const lineNo = this.currentError.raw.match(/l\.([0-9]+)/)
+          if (lineNo && this.currentError.line === null) {
+            this.currentError.line = parseInt(lineNo[1], 10)
           }
-          this.data.push(this.currentError);
-          this.state = state.NORMAL;
+          this.data.push(this.currentError)
+          this.state = state.NORMAL
         }
       }
-      return this.postProcess(this.data);
-    };
-    this.currentLineIsError = function() {
-      return this.currentLine[0] === '!';
-    };
-    this.currentLineIsRunawayArgument = function() {
-      return this.currentLine.match(/^Runaway argument/);
-    };
-    this.currentLineIsWarning = function() {
-      return !!this.currentLine.match(LATEX_WARNING_REGEX);
-    };
-    this.currentLineIsPackageWarning = function() {
-      return !!this.currentLine.match(PACKAGE_WARNING_REGEX);
-    };
-    this.currentLineIsHboxWarning = function() {
-      return !!this.currentLine.match(HBOX_WARNING_REGEX);
-    };
-    this.parseRunawayArgumentError = function() {
-      var lineNo;
+      return this.postProcess(this.data)
+    }
+
+    this.currentLineIsError = function () {
+      return this.currentLine[0] === '!'
+    }
+
+    this.currentLineIsFileLineError = function () {
+      return /^\/.*:\d+: .*/.test(this.currentLine)
+    }
+
+    this.currentLineIsRunawayArgument = function () {
+      return this.currentLine.match(/^Runaway argument/)
+    }
+
+    this.currentLineIsWarning = function () {
+      return !!this.currentLine.match(LATEX_WARNING_REGEX)
+    }
+
+    this.currentLineIsPackageWarning = function () {
+      return !!this.currentLine.match(PACKAGE_WARNING_REGEX)
+    }
+
+    this.currentLineIsHboxWarning = function () {
+      return !!this.currentLine.match(HBOX_WARNING_REGEX)
+    }
+
+    this.parseFileLineError = function () {
+      const result = this.currentLine.match(/^(\/.*):(\d+): (.*)/)
+      this.currentError = {
+        line: result[2],
+        file: result[1],
+        level: 'error',
+        message: result[3],
+        content: '',
+        raw: this.currentLine + '\n',
+      }
+    }
+
+    this.parseRunawayArgumentError = function () {
       this.currentError = {
         line: null,
         file: this.currentFilePath,
         level: 'error',
         message: this.currentLine,
         content: '',
-        raw: this.currentLine + '\n'
-      };
-      this.currentError.content += this.log.linesUpToNextWhitespaceLine().join('\n');
-      this.currentError.content += '\n';
-      this.currentError.content += this.log.linesUpToNextWhitespaceLine().join('\n');
-      this.currentError.raw += this.currentError.content;
-      lineNo = this.currentError.raw.match(/l\.([0-9]+)/);
+        raw: this.currentLine + '\n',
+      }
+      this.currentError.content += this.log
+        .linesUpToNextWhitespaceLine()
+        .join('\n')
+      this.currentError.content += '\n'
+      this.currentError.content += this.log
+        .linesUpToNextWhitespaceLine()
+        .join('\n')
+      this.currentError.raw += this.currentError.content
+      const lineNo = this.currentError.raw.match(/l\.([0-9]+)/)
       if (lineNo) {
-        this.currentError.line = parseInt(lineNo[1], 10);
+        this.currentError.line = parseInt(lineNo[1], 10)
       }
-      return this.data.push(this.currentError);
-    };
-    this.parseSingleWarningLine = function(prefix_regex) {
-      var line, lineMatch, warning, warningMatch;
-      warningMatch = this.currentLine.match(prefix_regex);
+      return this.data.push(this.currentError)
+    }
+
+    this.parseSingleWarningLine = function (prefix_regex) {
+      const warningMatch = this.currentLine.match(prefix_regex)
       if (!warningMatch) {
-        return;
+        return
       }
-      warning = warningMatch[1];
-      lineMatch = warning.match(LINES_REGEX);
-      line = lineMatch ? parseInt(lineMatch[1], 10) : null;
+      const warning = warningMatch[1]
+      const lineMatch = warning.match(LINES_REGEX)
+      const line = lineMatch ? parseInt(lineMatch[1], 10) : null
       this.data.push({
-        line: line,
+        line,
         file: this.currentFilePath,
         level: 'warning',
         message: warning,
-        raw: warning
-      });
-    };
-    this.parseMultipleWarningLine = function() {
-      var line, lineMatch, packageMatch, packageName, prefixRegex, raw_message, warningMatch, warning_lines;
-      warningMatch = this.currentLine.match(PACKAGE_WARNING_REGEX);
+        raw: warning,
+      })
+    }
+
+    this.parseMultipleWarningLine = function () {
+      // Some package warnings are multiple lines, let's parse the first line
+      let warningMatch = this.currentLine.match(PACKAGE_WARNING_REGEX)
       if (!warningMatch) {
-        return;
+        return
       }
-      warning_lines = [warningMatch[1]];
-      lineMatch = this.currentLine.match(LINES_REGEX);
-      line = lineMatch ? parseInt(lineMatch[1], 10) : null;
-      packageMatch = this.currentLine.match(PACKAGE_REGEX);
-      packageName = packageMatch[1];
-      prefixRegex = new RegExp('(?:\\(' + packageName + '\\))*[\\s]*(.*)', 'i');
+      // Something strange happened, return early
+      const warning_lines = [warningMatch[1]]
+      let lineMatch = this.currentLine.match(LINES_REGEX)
+      let line = lineMatch ? parseInt(lineMatch[1], 10) : null
+      const packageMatch = this.currentLine.match(PACKAGE_REGEX)
+      const packageName = packageMatch[1]
+      // Regex to get rid of the unnecesary (packagename) prefix in most multi-line warnings
+      const prefixRegex = new RegExp(
+        '(?:\\(' + packageName + '\\))*[\\s]*(.*)',
+        'i'
+      )
+      // After every warning message there's a blank line, let's use it
       while (!!(this.currentLine = this.log.nextLine())) {
-        lineMatch = this.currentLine.match(LINES_REGEX);
-        line = lineMatch ? parseInt(lineMatch[1], 10) : line;
-        warningMatch = this.currentLine.match(prefixRegex);
-        warning_lines.push(warningMatch[1]);
+        lineMatch = this.currentLine.match(LINES_REGEX)
+        line = lineMatch ? parseInt(lineMatch[1], 10) : line
+        warningMatch = this.currentLine.match(prefixRegex)
+        warning_lines.push(warningMatch[1])
       }
-      raw_message = warning_lines.join(' ');
+      const raw_message = warning_lines.join(' ')
       this.data.push({
-        line: line,
+        line,
         file: this.currentFilePath,
         level: 'warning',
         message: raw_message,
-        raw: raw_message
-      });
-    };
-    this.parseHboxLine = function() {
-      var line, lineMatch;
-      lineMatch = this.currentLine.match(LINES_REGEX);
-      line = lineMatch ? parseInt(lineMatch[1], 10) : null;
+        raw: raw_message,
+      })
+    }
+
+    this.parseHboxLine = function () {
+      const lineMatch = this.currentLine.match(LINES_REGEX)
+      const line = lineMatch ? parseInt(lineMatch[1], 10) : null
       this.data.push({
-        line: line,
+        line,
         file: this.currentFilePath,
         level: 'typesetting',
         message: this.currentLine,
-        raw: this.currentLine
-      });
-    };
-    this.parseParensForFilenames = function() {
-      var filePath, newFile, pos, previousFile, token;
-      pos = this.currentLine.search(/\(|\)/);
+        raw: this.currentLine,
+      })
+    }
+
+    // Check if we're entering or leaving a new file in this line
+
+    this.parseParensForFilenames = function () {
+      const pos = this.currentLine.search(/\(|\)/)
       if (pos !== -1) {
-        token = this.currentLine[pos];
-        this.currentLine = this.currentLine.slice(pos + 1);
+        const token = this.currentLine[pos]
+        this.currentLine = this.currentLine.slice(pos + 1)
         if (token === '(') {
-          filePath = this.consumeFilePath();
+          const filePath = this.consumeFilePath()
           if (filePath) {
-            this.currentFilePath = filePath;
-            newFile = {
+            this.currentFilePath = filePath
+            const newFile = {
               path: filePath,
-              files: []
-            };
-            this.fileStack.push(newFile);
-            this.currentFileList.push(newFile);
-            this.currentFileList = newFile.files;
+              files: [],
+            }
+            this.fileStack.push(newFile)
+            this.currentFileList.push(newFile)
+            this.currentFileList = newFile.files
           } else {
-            this.openParens++;
+            this.openParens++
           }
         } else if (token === ')') {
           if (this.openParens > 0) {
-            this.openParens--;
+            this.openParens--
           } else {
             if (this.fileStack.length > 1) {
-              this.fileStack.pop();
-              previousFile = this.fileStack[this.fileStack.length - 1];
-              this.currentFilePath = previousFile.path;
-              this.currentFileList = previousFile.files;
+              this.fileStack.pop()
+              const previousFile = this.fileStack[this.fileStack.length - 1]
+              this.currentFilePath = previousFile.path
+              this.currentFileList = previousFile.files
             }
           }
         }
-        this.parseParensForFilenames();
+        // else {
+        //		 Something has gone wrong but all we can do now is ignore it :(
+        // }
+        // Process the rest of the line
+        this.parseParensForFilenames()
       }
-    };
-    this.consumeFilePath = function() {
-      var endOfFilePath, path;
+    }
+
+    this.consumeFilePath = function () {
+      // Our heuristic for detecting file names are rather crude
+      // A file may not contain a space, or ) in it
+      // To be a file path it must have at least one /
       if (!this.currentLine.match(/^\/?([^ \)]+\/)+/)) {
-        return false;
+        return false
       }
-      endOfFilePath = this.currentLine.search(RegExp(' |\\)'));
-      path = void 0;
+      const endOfFilePath = this.currentLine.search(RegExp(' |\\)'))
+      let path = undefined
       if (endOfFilePath === -1) {
-        path = this.currentLine;
-        this.currentLine = '';
+        path = this.currentLine
+        this.currentLine = ''
       } else {
-        path = this.currentLine.slice(0, endOfFilePath);
-        this.currentLine = this.currentLine.slice(endOfFilePath);
+        path = this.currentLine.slice(0, endOfFilePath)
+        this.currentLine = this.currentLine.slice(endOfFilePath)
       }
-      return path;
-    };
-    return this.postProcess = function(data) {
-      var all, errors, hashEntry, hashes, i, typesetting, warnings;
-      all = [];
-      errors = [];
-      warnings = [];
-      typesetting = [];
-      hashes = [];
-      hashEntry = function(entry) {
-        return entry.raw;
-      };
-      i = 0;
+      return path
+    }
+
+    this.postProcess = function (data) {
+      const all = []
+      const errors = []
+      const warnings = []
+      const typesetting = []
+      const hashes = []
+
+      const hashEntry = entry => entry.raw
+
+      let i = 0
       while (i < data.length) {
         if (this.ignoreDuplicates && hashes.indexOf(hashEntry(data[i])) > -1) {
-          i++;
-          continue;
+          i++
+          continue
         }
         if (data[i].level === 'error') {
-          errors.push(data[i]);
+          errors.push(data[i])
         } else if (data[i].level === 'typesetting') {
-          typesetting.push(data[i]);
+          typesetting.push(data[i])
         } else if (data[i].level === 'warning') {
-          warnings.push(data[i]);
+          warnings.push(data[i])
         }
-        all.push(data[i]);
-        hashes.push(hashEntry(data[i]));
-        i++;
+        all.push(data[i])
+        hashes.push(hashEntry(data[i]))
+        i++
       }
       return {
-        errors: errors,
-        warnings: warnings,
-        typesetting: typesetting,
-        all: all,
-        files: this.rootFileList
-      };
-    };
-  }).call(LatexParser.prototype);
-  LatexParser.parse = function(text, options) {
-    return new LatexParser(text, options).parse();
-  };
-  return LatexParser;
-});
+        errors,
+        warnings,
+        typesetting,
+        all,
+        files: this.rootFileList,
+      }
+    }
+  }.call(LatexParser.prototype))
+
+  LatexParser.parse = (text, options) => new LatexParser(text, options).parse()
+
+  return LatexParser
+})

services/web/frontend/stylesheets/app/editor/pdf.less

@@ -246,6 +246,7 @@
   }
   pre {
     font-size: 12px;
+    white-space: pre-wrap;
   }
   .dropdown {
     position: relative;

services/web/test/unit/src/Compile/ClsiManagerTests.js

@@ -620,6 +620,7 @@ describe('ClsiManager', function () {
               syncState: undefined,
               compileGroup: 'standard',
               enablePdfCaching: false,
+              flags: undefined,
             }, // "01234567890abcdef"
             rootResourcePath: 'main.tex',
             resources: [
@@ -716,6 +717,7 @@ describe('ClsiManager', function () {
               syncState: '01234567890abcdef',
               compileGroup: 'priority',
               enablePdfCaching: false,
+              flags: undefined,
             },
             rootResourcePath: 'main.tex',
             resources: [

services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -107,6 +107,7 @@ describe('CompileController', function () {
           .calledWith(this.project_id, this.user_id, {
             isAutoCompile: false,
             enablePdfCaching: false,
+            fileLineErrors: false,
           })
           .should.equal(true)
       })
@@ -137,6 +138,7 @@ describe('CompileController', function () {
           .calledWith(this.project_id, this.user_id, {
             isAutoCompile: true,
             enablePdfCaching: false,
+            fileLineErrors: false,
           })
           .should.equal(true)
       })
@@ -154,6 +156,7 @@ describe('CompileController', function () {
             isAutoCompile: false,
             enablePdfCaching: false,
             draft: true,
+            fileLineErrors: false,
           })
           .should.equal(true)
       })