From 74cdad29d086db412940d5044a008e995961f13c Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Thu, 12 Apr 2012 23:52:50 -0700
Subject: [PATCH] Don't save highlithing errors to cache

Summary:
If I have Pygments enabled in config but `pygmentize` doesn't work then unhighlighted source is stored to cache.
If I later make `pygmentize` work then the unhighlighted source is still loaded from the cache.

Test Plan:
Break `pygmentize`.
View a diff with JS files.
Fix `pygmenize`.
View the diff again.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, gatos99, Koolvin

Differential Revision: https://secure.phabricator.com/D2227
---
 .../changeset/DifferentialChangesetParser.php | 26 ++++++++++++++++---
 .../parser/changeset/__init__.php             |  1 +
 .../syntax/PhabricatorSyntaxHighlighter.php   |  6 ++---
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
index aa9b4decad..1a04771f1a 100644
--- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
@@ -55,8 +55,9 @@ final class DifferentialChangesetParser {
   private $isTopLevel;
   private $coverage;
   private $markupEngine;
+  private $highlightErrors;
 
-  const CACHE_VERSION = 4;
+  const CACHE_VERSION = 5;
 
   const ATTR_GENERATED  = 'attr:generated';
   const ATTR_DELETED    = 'attr:deleted';
@@ -503,7 +504,7 @@ final class DifferentialChangesetParser {
         $new_corpus[] = $n['text'];
       }
     }
-    $new_corpus_block = implode("\n", $new_corpus);
+    $new_corpus_block = implode("\n", $old_corpus);
 
     $old_future = $this->getHighlightFuture($old_corpus_block);
     $new_future = $this->getHighlightFuture($new_corpus_block);
@@ -511,18 +512,31 @@ final class DifferentialChangesetParser {
       'old' => $old_future,
       'new' => $new_future,
     );
+    $corpus_blocks = array(
+      'old' => $old_corpus_block,
+      'new' => $new_corpus_block,
+    );
+    $this->highlightErrors = false;
     foreach (Futures($futures) as $key => $future) {
       try {
+        try {
+          $highlighted = $future->resolve();
+        } catch (PhutilSyntaxHighlighterException $ex) {
+          $this->highlightErrors = true;
+          $highlighted = id(new PhutilDefaultSyntaxHighlighter())
+            ->getHighlightFuture($corpus_blocks[$key])
+            ->resolve();
+        }
         switch ($key) {
           case 'old':
             $this->oldRender = $this->processHighlightedSource(
               $this->old,
-              $future->resolve());
+              $highlighted);
             break;
           case 'new':
             $this->newRender = $this->processHighlightedSource(
               $this->new,
-              $future->resolve());
+              $highlighted);
             break;
         }
       } catch (Exception $ex) {
@@ -630,6 +644,10 @@ final class DifferentialChangesetParser {
   }
 
   public function saveCache() {
+    if ($this->highlightErrors) {
+      return false;
+    }
+
     $render_cache_key = $this->getRenderCacheKey();
     if (!$render_cache_key) {
       return false;
diff --git a/src/applications/differential/parser/changeset/__init__.php b/src/applications/differential/parser/changeset/__init__.php
index 75f3d0475a..247eb85c85 100644
--- a/src/applications/differential/parser/changeset/__init__.php
+++ b/src/applications/differential/parser/changeset/__init__.php
@@ -25,6 +25,7 @@ phutil_require_module('phutil', 'error');
 phutil_require_module('phutil', 'events/engine');
 phutil_require_module('phutil', 'future');
 phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'markup/syntax/highlighter/default');
 phutil_require_module('phutil', 'utils');
 
 
diff --git a/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php b/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php
index 52597ec333..26ce6bca9e 100644
--- a/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php
+++ b/src/applications/markup/syntax/PhabricatorSyntaxHighlighter.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,12 +39,12 @@ final class PhabricatorSyntaxHighlighter {
   public static function highlightWithFilename($filename, $source) {
     $engine = self::newEngine();
     $language = $engine->getLanguageFromFilename($filename);
-    return $engine->getHighlightFuture($language, $source)->resolve();
+    return $engine->highlightSource($language, $source);
   }
 
   public static function highlightWithLanguage($language, $source) {
     $engine = self::newEngine();
-    return $engine->getHighlightFuture($language, $source)->resolve();
+    return $engine->highlightSource($language, $source);
   }