From 767528c0ed556299660ab1574eb6fc810f4468c6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Jan 2020 11:40:00 -0800
Subject: [PATCH 01/17] Move search query parser/compiler classes to
 Phabricator

Summary: Ref T13472. Ref T13395. These classes are only used by Phabricator and not likely to find much use in Arcanist.

Test Plan: Grepped libphutil and Arcanist for removed symbols.

Maniphest Tasks: T13472, T13395

Differential Revision: https://secure.phabricator.com/D20939
---
 externals/porter-stemmer/LICENSE              |  20 +
 externals/porter-stemmer/README.md            |  42 ++
 externals/porter-stemmer/src/Porter.php       | 426 ++++++++++++++++++
 src/__phutil_library_map__.php                |  12 +
 .../compiler/PhutilSearchQueryCompiler.php    | 374 +++++++++++++++
 ...utilSearchQueryCompilerSyntaxException.php |   4 +
 .../compiler/PhutilSearchQueryToken.php       |  37 ++
 .../search/compiler/PhutilSearchStemmer.php   |  74 +++
 .../PhutilSearchQueryCompilerTestCase.php     | 220 +++++++++
 .../__tests__/PhutilSearchStemmerTestCase.php |  85 ++++
 10 files changed, 1294 insertions(+)
 create mode 100644 externals/porter-stemmer/LICENSE
 create mode 100644 externals/porter-stemmer/README.md
 create mode 100644 externals/porter-stemmer/src/Porter.php
 create mode 100644 src/applications/search/compiler/PhutilSearchQueryCompiler.php
 create mode 100644 src/applications/search/compiler/PhutilSearchQueryCompilerSyntaxException.php
 create mode 100644 src/applications/search/compiler/PhutilSearchQueryToken.php
 create mode 100644 src/applications/search/compiler/PhutilSearchStemmer.php
 create mode 100644 src/applications/search/compiler/__tests__/PhutilSearchQueryCompilerTestCase.php
 create mode 100644 src/applications/search/compiler/__tests__/PhutilSearchStemmerTestCase.php

diff --git a/externals/porter-stemmer/LICENSE b/externals/porter-stemmer/LICENSE
new file mode 100644
index 0000000000..d4afc6cfa7
--- /dev/null
+++ b/externals/porter-stemmer/LICENSE
@@ -0,0 +1,20 @@
+The MIT License (MIT)
+
+Copyright (c) 2005-2016 Richard Heyes (http://www.phpguru.org/)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of
+this software and associated documentation files (the "Software"), to deal in
+the Software without restriction, including without limitation the rights to
+use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+the Software, and to permit persons to whom the Software is furnished to do so,
+subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
diff --git a/externals/porter-stemmer/README.md b/externals/porter-stemmer/README.md
new file mode 100644
index 0000000000..06bc417f9f
--- /dev/null
+++ b/externals/porter-stemmer/README.md
@@ -0,0 +1,42 @@
+# Porter Stemmer by Richard Heyes
+
+# Installation (with composer)
+
+```json
+{
+	"require": {
+		"camspiers/porter-stemmer": "1.0.0"
+	}
+}
+```
+
+	$ composer install
+
+# Usage
+
+```php
+$stem = Porter::Stem($word);
+```
+
+# License
+
+The MIT License (MIT)
+
+Copyright (c) 2005-2016 Richard Heyes (http://www.phpguru.org/)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of
+this software and associated documentation files (the "Software"), to deal in
+the Software without restriction, including without limitation the rights to
+use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+the Software, and to permit persons to whom the Software is furnished to do so,
+subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
diff --git a/externals/porter-stemmer/src/Porter.php b/externals/porter-stemmer/src/Porter.php
new file mode 100644
index 0000000000..0720c53ba0
--- /dev/null
+++ b/externals/porter-stemmer/src/Porter.php
@@ -0,0 +1,426 @@
+<?php
+
+# vim: set expandtab tabstop=4 shiftwidth=4 softtabstop=4:
+
+/**
+ * Copyright (c) 2005-2016 Richard Heyes (http://www.phpguru.org/)
+ *
+ * Portions Copyright 2003-2007 Jon Abernathy <jon@chuggnutt.com>
+ *
+ * Originally available under the GPL 2 or greater. Relicensed with permission
+ * of original authors under the MIT License in 2016.
+ *
+ * All rights reserved.
+ *
+ * @package   PorterStemmer
+ * @author    Richard Heyes
+ * @author    Jon Abernathy <jon@chuggnutt.com>
+ * @copyright 2005-2016 Richard Heyes (http://www.phpguru.org/)
+ * @license   http://www.opensource.org/licenses/mit-license.html MIT License
+ */
+
+/**
+ * PHP 5 Implementation of the Porter Stemmer algorithm. Certain elements
+ * were borrowed from the (broken) implementation by Jon Abernathy.
+ *
+ * See http://tartarus.org/~martin/PorterStemmer/ for a description of the
+ * algorithm.
+ *
+ * Usage:
+ *
+ *  $stem = PorterStemmer::Stem($word);
+ *
+ * How easy is that?
+ *
+ * @package   PorterStemmer
+ * @author    Richard Heyes
+ * @author    Jon Abernathy <jon@chuggnutt.com>
+ * @copyright 2005-2016 Richard Heyes (http://www.phpguru.org/)
+ * @license   http://www.opensource.org/licenses/mit-license.html MIT License
+ */
+class Porter
+{
+    /**
+     * Regex for matching a consonant
+     *
+     * @var string
+     */
+    private static $regex_consonant = '(?:[bcdfghjklmnpqrstvwxz]|(?<=[aeiou])y|^y)';
+
+    /**
+     * Regex for matching a vowel
+     *
+     * @var string
+     */
+    private static $regex_vowel = '(?:[aeiou]|(?<![aeiou])y)';
+
+    /**
+     * Stems a word. Simple huh?
+     *
+     * @param string $word Word to stem
+     *
+     * @return string Stemmed word
+     */
+    public static function Stem($word)
+    {
+        if (strlen($word) <= 2) {
+            return $word;
+        }
+
+        $word = self::step1ab($word);
+        $word = self::step1c($word);
+        $word = self::step2($word);
+        $word = self::step3($word);
+        $word = self::step4($word);
+        $word = self::step5($word);
+
+        return $word;
+    }
+
+    /**
+     * Step 1
+     */
+    private static function step1ab($word)
+    {
+        // Part a
+        if (substr($word, -1) == 's') {
+
+               self::replace($word, 'sses', 'ss')
+            OR self::replace($word, 'ies', 'i')
+            OR self::replace($word, 'ss', 'ss')
+            OR self::replace($word, 's', '');
+        }
+
+        // Part b
+        if (substr($word, -2, 1) != 'e' OR !self::replace($word, 'eed', 'ee', 0)) { // First rule
+            $v = self::$regex_vowel;
+
+            // ing and ed
+            if (   preg_match("#$v+#", substr($word, 0, -3)) && self::replace($word, 'ing', '')
+                OR preg_match("#$v+#", substr($word, 0, -2)) && self::replace($word, 'ed', '')) { // Note use of && and OR, for precedence reasons
+
+                // If one of above two test successful
+                if (    !self::replace($word, 'at', 'ate')
+                    AND !self::replace($word, 'bl', 'ble')
+                    AND !self::replace($word, 'iz', 'ize')) {
+
+                    // Double consonant ending
+                    if (    self::doubleConsonant($word)
+                        AND substr($word, -2) != 'll'
+                        AND substr($word, -2) != 'ss'
+                        AND substr($word, -2) != 'zz') {
+
+                        $word = substr($word, 0, -1);
+
+                    } elseif (self::m($word) == 1 AND self::cvc($word)) {
+                        $word .= 'e';
+                    }
+                }
+            }
+        }
+
+        return $word;
+    }
+
+    /**
+     * Step 1c
+     *
+     * @param string $word Word to stem
+     */
+    private static function step1c($word)
+    {
+        $v = self::$regex_vowel;
+
+        if (substr($word, -1) == 'y' && preg_match("#$v+#", substr($word, 0, -1))) {
+            self::replace($word, 'y', 'i');
+        }
+
+        return $word;
+    }
+
+    /**
+     * Step 2
+     *
+     * @param string $word Word to stem
+     */
+    private static function step2($word)
+    {
+        switch (substr($word, -2, 1)) {
+            case 'a':
+                   self::replace($word, 'ational', 'ate', 0)
+                OR self::replace($word, 'tional', 'tion', 0);
+                break;
+
+            case 'c':
+                   self::replace($word, 'enci', 'ence', 0)
+                OR self::replace($word, 'anci', 'ance', 0);
+                break;
+
+            case 'e':
+                self::replace($word, 'izer', 'ize', 0);
+                break;
+
+            case 'g':
+                self::replace($word, 'logi', 'log', 0);
+                break;
+
+            case 'l':
+                   self::replace($word, 'entli', 'ent', 0)
+                OR self::replace($word, 'ousli', 'ous', 0)
+                OR self::replace($word, 'alli', 'al', 0)
+                OR self::replace($word, 'bli', 'ble', 0)
+                OR self::replace($word, 'eli', 'e', 0);
+                break;
+
+            case 'o':
+                   self::replace($word, 'ization', 'ize', 0)
+                OR self::replace($word, 'ation', 'ate', 0)
+                OR self::replace($word, 'ator', 'ate', 0);
+                break;
+
+            case 's':
+                   self::replace($word, 'iveness', 'ive', 0)
+                OR self::replace($word, 'fulness', 'ful', 0)
+                OR self::replace($word, 'ousness', 'ous', 0)
+                OR self::replace($word, 'alism', 'al', 0);
+                break;
+
+            case 't':
+                   self::replace($word, 'biliti', 'ble', 0)
+                OR self::replace($word, 'aliti', 'al', 0)
+                OR self::replace($word, 'iviti', 'ive', 0);
+                break;
+        }
+
+        return $word;
+    }
+
+    /**
+     * Step 3
+     *
+     * @param string $word String to stem
+     */
+    private static function step3($word)
+    {
+        switch (substr($word, -2, 1)) {
+            case 'a':
+                self::replace($word, 'ical', 'ic', 0);
+                break;
+
+            case 's':
+                self::replace($word, 'ness', '', 0);
+                break;
+
+            case 't':
+                   self::replace($word, 'icate', 'ic', 0)
+                OR self::replace($word, 'iciti', 'ic', 0);
+                break;
+
+            case 'u':
+                self::replace($word, 'ful', '', 0);
+                break;
+
+            case 'v':
+                self::replace($word, 'ative', '', 0);
+                break;
+
+            case 'z':
+                self::replace($word, 'alize', 'al', 0);
+                break;
+        }
+
+        return $word;
+    }
+
+    /**
+     * Step 4
+     *
+     * @param string $word Word to stem
+     */
+    private static function step4($word)
+    {
+        switch (substr($word, -2, 1)) {
+            case 'a':
+                self::replace($word, 'al', '', 1);
+                break;
+
+            case 'c':
+                   self::replace($word, 'ance', '', 1)
+                OR self::replace($word, 'ence', '', 1);
+                break;
+
+            case 'e':
+                self::replace($word, 'er', '', 1);
+                break;
+
+            case 'i':
+                self::replace($word, 'ic', '', 1);
+                break;
+
+            case 'l':
+                   self::replace($word, 'able', '', 1)
+                OR self::replace($word, 'ible', '', 1);
+                break;
+
+            case 'n':
+                   self::replace($word, 'ant', '', 1)
+                OR self::replace($word, 'ement', '', 1)
+                OR self::replace($word, 'ment', '', 1)
+                OR self::replace($word, 'ent', '', 1);
+                break;
+
+            case 'o':
+                if (substr($word, -4) == 'tion' OR substr($word, -4) == 'sion') {
+                   self::replace($word, 'ion', '', 1);
+                } else {
+                    self::replace($word, 'ou', '', 1);
+                }
+                break;
+
+            case 's':
+                self::replace($word, 'ism', '', 1);
+                break;
+
+            case 't':
+                   self::replace($word, 'ate', '', 1)
+                OR self::replace($word, 'iti', '', 1);
+                break;
+
+            case 'u':
+                self::replace($word, 'ous', '', 1);
+                break;
+
+            case 'v':
+                self::replace($word, 'ive', '', 1);
+                break;
+
+            case 'z':
+                self::replace($word, 'ize', '', 1);
+                break;
+        }
+
+        return $word;
+    }
+
+    /**
+     * Step 5
+     *
+     * @param string $word Word to stem
+     */
+    private static function step5($word)
+    {
+        // Part a
+        if (substr($word, -1) == 'e') {
+            if (self::m(substr($word, 0, -1)) > 1) {
+                self::replace($word, 'e', '');
+
+            } elseif (self::m(substr($word, 0, -1)) == 1) {
+
+                if (!self::cvc(substr($word, 0, -1))) {
+                    self::replace($word, 'e', '');
+                }
+            }
+        }
+
+        // Part b
+        if (self::m($word) > 1 AND self::doubleConsonant($word) AND substr($word, -1) == 'l') {
+            $word = substr($word, 0, -1);
+        }
+
+        return $word;
+    }
+
+    /**
+     * Replaces the first string with the second, at the end of the string
+     *
+     * If third arg is given, then the preceding string must match that m
+     * count at least.
+     *
+     * @param  string $str   String to check
+     * @param  string $check Ending to check for
+     * @param  string $repl  Replacement string
+     * @param  int    $m     Optional minimum number of m() to meet
+     *
+     * @return bool Whether the $check string was at the end of the $str
+     *              string. True does not necessarily mean that it was
+     *              replaced.
+     */
+    private static function replace(&$str, $check, $repl, $m = null)
+    {
+        $len = 0 - strlen($check);
+
+        if (substr($str, $len) == $check) {
+            $substr = substr($str, 0, $len);
+            if (is_null($m) OR self::m($substr) > $m) {
+                $str = $substr . $repl;
+            }
+
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * What, you mean it's not obvious from the name?
+     *
+     * m() measures the number of consonant sequences in $str. if c is
+     * a consonant sequence and v a vowel sequence, and <..> indicates arbitrary
+     * presence,
+     *
+     * <c><v>       gives 0
+     * <c>vc<v>     gives 1
+     * <c>vcvc<v>   gives 2
+     * <c>vcvcvc<v> gives 3
+     *
+     * @param  string $str The string to return the m count for
+     *
+     * @return int The m count
+     */
+    private static function m($str)
+    {
+        $c = self::$regex_consonant;
+        $v = self::$regex_vowel;
+
+        $str = preg_replace("#^$c+#", '', $str);
+        $str = preg_replace("#$v+$#", '', $str);
+
+        preg_match_all("#($v+$c+)#", $str, $matches);
+
+        return count($matches[1]);
+    }
+
+    /**
+     * Returns true/false as to whether the given string contains two
+     * of the same consonant next to each other at the end of the string.
+     *
+     * @param  string $str String to check
+     *
+     * @return bool Result
+     */
+    private static function doubleConsonant($str)
+    {
+        $c = self::$regex_consonant;
+
+        return preg_match("#$c{2}$#", $str, $matches) AND $matches[0]{0} == $matches[0]{1};
+    }
+
+    /**
+     * Checks for ending CVC sequence where second C is not W, X or Y
+     *
+     * @param  string $str String to check
+     *
+     * @return bool Result
+     */
+    private static function cvc($str)
+    {
+        $c = self::$regex_consonant;
+        $v = self::$regex_vowel;
+
+        return     preg_match("#($c$v$c)$#", $str, $matches)
+               AND strlen($matches[1]) == 3
+               AND $matches[1]{2} != 'w'
+               AND $matches[1]{2} != 'x'
+               AND $matches[1]{2} != 'y';
+    }
+}
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 174c0e4876..9b15a81c49 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -5658,6 +5658,12 @@ phutil_register_library_map(array(
     'PhutilRemarkupTableBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php',
     'PhutilRemarkupTestInterpreterRule' => 'infrastructure/markup/blockrule/PhutilRemarkupTestInterpreterRule.php',
     'PhutilRemarkupUnderlineRule' => 'infrastructure/markup/markuprule/PhutilRemarkupUnderlineRule.php',
+    'PhutilSearchQueryCompiler' => 'applications/search/compiler/PhutilSearchQueryCompiler.php',
+    'PhutilSearchQueryCompilerSyntaxException' => 'applications/search/compiler/PhutilSearchQueryCompilerSyntaxException.php',
+    'PhutilSearchQueryCompilerTestCase' => 'applications/search/compiler/__tests__/PhutilSearchQueryCompilerTestCase.php',
+    'PhutilSearchQueryToken' => 'applications/search/compiler/PhutilSearchQueryToken.php',
+    'PhutilSearchStemmer' => 'applications/search/compiler/PhutilSearchStemmer.php',
+    'PhutilSearchStemmerTestCase' => 'applications/search/compiler/__tests__/PhutilSearchStemmerTestCase.php',
     'PhutilSlackAuthAdapter' => 'applications/auth/adapter/PhutilSlackAuthAdapter.php',
     'PhutilTwitchAuthAdapter' => 'applications/auth/adapter/PhutilTwitchAuthAdapter.php',
     'PhutilTwitterAuthAdapter' => 'applications/auth/adapter/PhutilTwitterAuthAdapter.php',
@@ -12483,6 +12489,12 @@ phutil_register_library_map(array(
     'PhutilRemarkupTableBlockRule' => 'PhutilRemarkupBlockRule',
     'PhutilRemarkupTestInterpreterRule' => 'PhutilRemarkupBlockInterpreter',
     'PhutilRemarkupUnderlineRule' => 'PhutilRemarkupRule',
+    'PhutilSearchQueryCompiler' => 'Phobject',
+    'PhutilSearchQueryCompilerSyntaxException' => 'Exception',
+    'PhutilSearchQueryCompilerTestCase' => 'PhutilTestCase',
+    'PhutilSearchQueryToken' => 'Phobject',
+    'PhutilSearchStemmer' => 'Phobject',
+    'PhutilSearchStemmerTestCase' => 'PhutilTestCase',
     'PhutilSlackAuthAdapter' => 'PhutilOAuthAuthAdapter',
     'PhutilTwitchAuthAdapter' => 'PhutilOAuthAuthAdapter',
     'PhutilTwitterAuthAdapter' => 'PhutilOAuth1AuthAdapter',
diff --git a/src/applications/search/compiler/PhutilSearchQueryCompiler.php b/src/applications/search/compiler/PhutilSearchQueryCompiler.php
new file mode 100644
index 0000000000..eb5ee860c9
--- /dev/null
+++ b/src/applications/search/compiler/PhutilSearchQueryCompiler.php
@@ -0,0 +1,374 @@
+<?php
+
+final class PhutilSearchQueryCompiler
+  extends Phobject {
+
+  private $operators = '+ -><()~*:""&|';
+  private $query;
+  private $stemmer;
+  private $enableFunctions = false;
+
+  const OPERATOR_NOT = 'not';
+  const OPERATOR_AND = 'and';
+  const OPERATOR_SUBSTRING = 'sub';
+  const OPERATOR_EXACT = 'exact';
+
+  public function setOperators($operators) {
+    $this->operators = $operators;
+    return $this;
+  }
+
+  public function getOperators() {
+    return $this->operators;
+  }
+
+  public function setStemmer(PhutilSearchStemmer $stemmer) {
+    $this->stemmer = $stemmer;
+    return $this;
+  }
+
+  public function getStemmer() {
+    return $this->stemmer;
+  }
+
+  public function setEnableFunctions($enable_functions) {
+    $this->enableFunctions = $enable_functions;
+    return $this;
+  }
+
+  public function getEnableFunctions() {
+    return $this->enableFunctions;
+  }
+
+  public function compileQuery(array $tokens) {
+    assert_instances_of($tokens, 'PhutilSearchQueryToken');
+
+    $result = array();
+    foreach ($tokens as $token) {
+      $result[] = $this->renderToken($token);
+    }
+
+    return $this->compileRenderedTokens($result);
+  }
+
+  public function compileLiteralQuery(array $tokens) {
+    assert_instances_of($tokens, 'PhutilSearchQueryToken');
+
+    $result = array();
+    foreach ($tokens as $token) {
+      if (!$token->isQuoted()) {
+        continue;
+      }
+      $result[] = $this->renderToken($token);
+    }
+
+    return $this->compileRenderedTokens($result);
+  }
+
+  public function compileStemmedQuery(array $tokens) {
+    assert_instances_of($tokens, 'PhutilSearchQueryToken');
+
+    $result = array();
+    foreach ($tokens as $token) {
+      if ($token->isQuoted()) {
+        continue;
+      }
+      $result[] = $this->renderToken($token, $this->getStemmer());
+    }
+
+    return $this->compileRenderedTokens($result);
+  }
+
+  private function compileRenderedTokens(array $list) {
+    if (!$list) {
+      return null;
+    }
+
+    $list = array_unique($list);
+    return implode(' ', $list);
+  }
+
+  public function newTokens($query) {
+    $results = $this->tokenizeQuery($query);
+
+    $tokens = array();
+    foreach ($results as $result) {
+      $tokens[] = PhutilSearchQueryToken::newFromDictionary($result);
+    }
+
+    return $tokens;
+  }
+
+  private function tokenizeQuery($query) {
+    $maximum_bytes = 1024;
+
+    $query_bytes = strlen($query);
+    if ($query_bytes > $maximum_bytes) {
+      throw new PhutilSearchQueryCompilerSyntaxException(
+        pht(
+          'Query is too long (%s bytes, maximum is %s bytes).',
+          new PhutilNumber($query_bytes),
+          new PhutilNumber($maximum_bytes)));
+    }
+
+    $query = phutil_utf8v($query);
+    $length = count($query);
+
+    $enable_functions = $this->getEnableFunctions();
+
+    $mode = 'scan';
+    $current_operator = array();
+    $current_token = array();
+    $current_function = null;
+    $is_quoted = false;
+    $tokens = array();
+
+    if ($enable_functions) {
+      $operator_characters = '[~=+-]';
+    } else {
+      $operator_characters = '[+-]';
+    }
+
+    for ($ii = 0; $ii < $length; $ii++) {
+      $character = $query[$ii];
+
+      if ($mode == 'scan') {
+        if (preg_match('/^\s\z/u', $character)) {
+          continue;
+        }
+
+        $mode = 'function';
+      }
+
+      if ($mode == 'function') {
+        $mode = 'operator';
+
+        if ($enable_functions) {
+          $found = false;
+          for ($jj = $ii; $jj < $length; $jj++) {
+            if (preg_match('/^[a-zA-Z]\z/u', $query[$jj])) {
+              continue;
+            }
+            if ($query[$jj] == ':') {
+              $found = $jj;
+            }
+            break;
+          }
+
+          if ($found !== false) {
+            $function = array_slice($query, $ii, ($jj - $ii));
+            $current_function = implode('', $function);
+
+            if (!strlen($current_function)) {
+              $current_function = null;
+            }
+
+            $ii = $jj;
+            continue;
+          }
+        }
+      }
+
+      if ($mode == 'operator') {
+        if (preg_match('/^\s\z/u', $character)) {
+          continue;
+        }
+
+        if (preg_match('/^'.$operator_characters.'\z/', $character)) {
+          $current_operator[] = $character;
+          continue;
+        }
+
+        $mode = 'quote';
+      }
+
+      if ($mode == 'quote') {
+        if (preg_match('/^"\z/', $character)) {
+          $is_quoted = true;
+          $mode = 'token';
+          continue;
+        }
+
+        $mode = 'token';
+      }
+
+      if ($mode == 'token') {
+        $capture = false;
+        $was_quoted = $is_quoted;
+        if ($is_quoted) {
+          if (preg_match('/^"\z/', $character)) {
+            $capture = true;
+            $mode = 'scan';
+            $is_quoted = false;
+          }
+        } else {
+          if (preg_match('/^\s\z/u', $character)) {
+            $capture = true;
+            $mode = 'scan';
+          }
+
+          if (preg_match('/^"\z/', $character)) {
+            $capture = true;
+            $mode = 'token';
+            $is_quoted = true;
+          }
+        }
+
+        if ($capture) {
+          $token = array(
+            'operator' => $current_operator,
+            'quoted' => $was_quoted,
+            'value' => $current_token,
+          );
+
+          if ($enable_functions) {
+            $token['function'] = $current_function;
+          }
+
+          $tokens[] = $token;
+
+          $current_operator = array();
+          $current_token = array();
+          $current_function = null;
+          continue;
+        } else {
+          $current_token[] = $character;
+        }
+      }
+    }
+
+    if ($is_quoted) {
+      throw new PhutilSearchQueryCompilerSyntaxException(
+        pht(
+          'Query contains unmatched double quotes.'));
+    }
+
+    if ($mode == 'operator') {
+      throw new PhutilSearchQueryCompilerSyntaxException(
+        pht(
+          'Query contains operator ("%s") with no search term.',
+          implode('', $current_operator)));
+    }
+
+    $token = array(
+      'operator' => $current_operator,
+      'quoted' => false,
+      'value' => $current_token,
+    );
+
+    if ($enable_functions) {
+      $token['function'] = $current_function;
+    }
+
+    $tokens[] = $token;
+
+    $results = array();
+    foreach ($tokens as $token) {
+      $value = implode('', $token['value']);
+      $operator_string = implode('', $token['operator']);
+
+      if (!strlen($value)) {
+        continue;
+      }
+
+      $is_quoted = $token['quoted'];
+
+      switch ($operator_string) {
+        case '-':
+          $operator = self::OPERATOR_NOT;
+          break;
+        case '~':
+          $operator = self::OPERATOR_SUBSTRING;
+          break;
+        case '=':
+          $operator = self::OPERATOR_EXACT;
+          break;
+        case '+':
+          $operator = self::OPERATOR_AND;
+          break;
+        case '':
+          // See T12995. If this query term contains Chinese, Japanese or
+          // Korean characters, treat the term as a substring term by default.
+          // These languages do not separate words with spaces, so the term
+          // search mode is normally useless.
+          if ($enable_functions && !$is_quoted && phutil_utf8_is_cjk($value)) {
+            $operator = self::OPERATOR_SUBSTRING;
+          } else {
+            $operator = self::OPERATOR_AND;
+          }
+          break;
+        default:
+          throw new PhutilSearchQueryCompilerSyntaxException(
+            pht(
+              'Query has an invalid sequence of operators ("%s").',
+              $operator_string));
+      }
+
+      $result = array(
+        'operator' => $operator,
+        'quoted' => $is_quoted,
+        'value' => $value,
+      );
+
+      if ($enable_functions) {
+        $result['function'] = $token['function'];
+      }
+
+      $results[] = $result;
+    }
+
+    return $results;
+  }
+
+  private function renderToken(
+    PhutilSearchQueryToken $token,
+    PhutilSearchStemmer $stemmer = null) {
+    $value = $token->getValue();
+
+    if ($stemmer) {
+      $value = $stemmer->stemToken($value);
+    }
+
+    $value = $this->quoteToken($value);
+    $operator = $token->getOperator();
+    $prefix = $this->getOperatorPrefix($operator);
+
+    $value = $prefix.$value;
+
+    return $value;
+  }
+
+  private function getOperatorPrefix($operator) {
+    $operators = $this->operators;
+
+    switch ($operator) {
+      case self::OPERATOR_AND:
+        $prefix = $operators[0];
+        break;
+      case self::OPERATOR_NOT:
+        $prefix = $operators[2];
+        break;
+      default:
+        throw new PhutilSearchQueryCompilerSyntaxException(
+          pht(
+            'Unsupported operator prefix "%s".',
+            $operator));
+    }
+
+    if ($prefix == ' ') {
+      $prefix = null;
+    }
+
+    return $prefix;
+  }
+
+  private function quoteToken($value) {
+    $operators = $this->operators;
+
+    $open_quote = $this->operators[10];
+    $close_quote = $this->operators[11];
+
+    return $open_quote.$value.$close_quote;
+  }
+
+}
diff --git a/src/applications/search/compiler/PhutilSearchQueryCompilerSyntaxException.php b/src/applications/search/compiler/PhutilSearchQueryCompilerSyntaxException.php
new file mode 100644
index 0000000000..b09d36d04f
--- /dev/null
+++ b/src/applications/search/compiler/PhutilSearchQueryCompilerSyntaxException.php
@@ -0,0 +1,4 @@
+<?php
+
+final class PhutilSearchQueryCompilerSyntaxException
+  extends Exception {}
diff --git a/src/applications/search/compiler/PhutilSearchQueryToken.php b/src/applications/search/compiler/PhutilSearchQueryToken.php
new file mode 100644
index 0000000000..b63427131e
--- /dev/null
+++ b/src/applications/search/compiler/PhutilSearchQueryToken.php
@@ -0,0 +1,37 @@
+<?php
+
+final class PhutilSearchQueryToken extends Phobject {
+
+  private $isQuoted;
+  private $value;
+  private $operator;
+  private $function;
+
+  public static function newFromDictionary(array $dictionary) {
+    $token = new self();
+
+    $token->isQuoted = $dictionary['quoted'];
+    $token->operator = $dictionary['operator'];
+    $token->value = $dictionary['value'];
+    $token->function = idx($dictionary, 'function');
+
+    return $token;
+  }
+
+  public function isQuoted() {
+    return $this->isQuoted;
+  }
+
+  public function getValue() {
+    return $this->value;
+  }
+
+  public function getOperator() {
+    return $this->operator;
+  }
+
+  public function getFunction() {
+    return $this->function;
+  }
+
+}
diff --git a/src/applications/search/compiler/PhutilSearchStemmer.php b/src/applications/search/compiler/PhutilSearchStemmer.php
new file mode 100644
index 0000000000..3255336ff0
--- /dev/null
+++ b/src/applications/search/compiler/PhutilSearchStemmer.php
@@ -0,0 +1,74 @@
+<?php
+
+final class PhutilSearchStemmer
+  extends Phobject {
+
+  public function stemToken($token) {
+    $token = $this->normalizeToken($token);
+    return $this->applyStemmer($token);
+  }
+
+  public function stemCorpus($corpus) {
+    $corpus = $this->normalizeCorpus($corpus);
+    $tokens = preg_split('/[^a-zA-Z0-9\x7F-\xFF._]+/', $corpus);
+
+    $words = array();
+    foreach ($tokens as $key => $token) {
+      $token = trim($token, '._');
+
+      if (strlen($token) < 3) {
+        continue;
+      }
+
+      $words[$token] = $token;
+    }
+
+    $stems = array();
+    foreach ($words as $word) {
+      $stems[] = $this->applyStemmer($word);
+    }
+
+    return implode(' ', $stems);
+  }
+
+  private function normalizeToken($token) {
+    return phutil_utf8_strtolower($token);
+  }
+
+  private function normalizeCorpus($corpus) {
+    return phutil_utf8_strtolower($corpus);
+  }
+
+  /**
+   * @phutil-external-symbol class Porter
+   */
+  private function applyStemmer($normalized_token) {
+    // If the token has internal punctuation, handle it literally. This
+    // deals with things like domain names, Conduit API methods, and other
+    // sorts of informal tokens.
+    if (preg_match('/[._]/', $normalized_token)) {
+      return $normalized_token;
+    }
+
+    static $loaded;
+
+    if ($loaded === null) {
+      $root = dirname(phutil_get_library_root('phabricator'));
+      require_once $root.'/externals/porter-stemmer/src/Porter.php';
+      $loaded = true;
+    }
+
+
+    $stem = Porter::stem($normalized_token);
+
+    // If the stem is too short, it won't be a candidate for indexing. These
+    // tokens are also likely to be acronyms (like "DNS") rather than real
+    // English words.
+    if (strlen($stem) < 3) {
+      return $normalized_token;
+    }
+
+    return $stem;
+  }
+
+}
diff --git a/src/applications/search/compiler/__tests__/PhutilSearchQueryCompilerTestCase.php b/src/applications/search/compiler/__tests__/PhutilSearchQueryCompilerTestCase.php
new file mode 100644
index 0000000000..647386f52c
--- /dev/null
+++ b/src/applications/search/compiler/__tests__/PhutilSearchQueryCompilerTestCase.php
@@ -0,0 +1,220 @@
+<?php
+
+final class PhutilSearchQueryCompilerTestCase
+  extends PhutilTestCase {
+
+  public function testCompileQueries() {
+    $tests = array(
+      '' => null,
+      'cat dog' => '+"cat" +"dog"',
+      'cat -dog' => '+"cat" -"dog"',
+      'cat-dog' => '+"cat-dog"',
+
+      // If there are spaces after an operator, the operator applies to the
+      // next search term.
+      'cat - dog' => '+"cat" -"dog"',
+
+      // Double quotes serve as delimiters even if there is no whitespace
+      // between terms.
+      '"cat"dog' => '+"cat" +"dog"',
+
+      // This query is too long.
+      str_repeat('x', 2048) => false,
+
+      // Multiple operators are not permitted.
+      '++cat' => false,
+      '+-cat' => false,
+      '--cat' => false,
+
+      // Stray operators are not permitted.
+      '+' => false,
+      'cat +' => false,
+
+      // Double quotes must be paired.
+      '"' => false,
+      'cat "' => false,
+      '"cat' => false,
+      'A"' => false,
+      'A"B"' => '+"A" +"B"',
+    );
+
+    $this->assertCompileQueries($tests);
+
+    // Test that we compile queries correctly if the operators have been
+    // swapped to use "AND" by default.
+    $operator_tests = array(
+      'cat dog' => '"cat" "dog"',
+      'cat -dog' => '"cat" -"dog"',
+    );
+    $this->assertCompileQueries($operator_tests, ' |-><()~*:""&\'');
+
+
+    // Test that we compile queries correctly if the quote operators have
+    // been swapped to differ.
+    $quote_tests = array(
+      'cat dog' => '+[cat] +[dog]',
+      'cat -dog' => '+[cat] -[dog]',
+    );
+    $this->assertCompileQueries($quote_tests, '+ -><()~*:[]&|');
+
+  }
+
+  public function testCompileQueriesWithStemming() {
+    $stemming_tests = array(
+      'cat dog' => array(
+        null,
+        '+"cat" +"dog"',
+      ),
+      'cats dogs' => array(
+        null,
+        '+"cat" +"dog"',
+      ),
+      'cats "dogs"' => array(
+        '+"dogs"',
+        '+"cat"',
+      ),
+      '"blessed blade" of the windseeker' => array(
+        '+"blessed blade"',
+        '+"of" +"the" +"windseek"',
+      ),
+      'mailing users for mentions on tasks' => array(
+        null,
+        '+"mail" +"user" +"for" +"mention" +"on" +"task"',
+      ),
+    );
+
+    $stemmer = new PhutilSearchStemmer();
+    $this->assertCompileQueries($stemming_tests, null, $stemmer);
+  }
+
+  public function testCompileQueriesWithFunctions() {
+    $op_and = PhutilSearchQueryCompiler::OPERATOR_AND;
+    $op_sub = PhutilSearchQueryCompiler::OPERATOR_SUBSTRING;
+    $op_exact = PhutilSearchQueryCompiler::OPERATOR_EXACT;
+
+    $mao = "\xE7\x8C\xAB";
+
+    $function_tests = array(
+      'cat' => array(
+        array(null, $op_and, 'cat'),
+      ),
+      ':cat' => array(
+        array(null, $op_and, 'cat'),
+      ),
+      'title:cat' => array(
+        array('title', $op_and, 'cat'),
+      ),
+      'title:cat:dog' => array(
+        array('title', $op_and, 'cat:dog'),
+      ),
+      'title:~cat' => array(
+        array('title', $op_sub, 'cat'),
+      ),
+      'cat title:="Meow Meow"' => array(
+        array(null, $op_and, 'cat'),
+        array('title', $op_exact, 'Meow Meow'),
+      ),
+      'title:cat title:dog' => array(
+        array('title', $op_and, 'cat'),
+        array('title', $op_and, 'dog'),
+      ),
+      '~"core and seven years ag"' => array(
+        array(null, $op_sub, 'core and seven years ag'),
+      ),
+      $mao => array(
+        array(null, $op_sub, $mao),
+      ),
+      '+'.$mao => array(
+        array(null, $op_and, $mao),
+      ),
+      '~'.$mao => array(
+        array(null, $op_sub, $mao),
+      ),
+      '"'.$mao.'"' => array(
+        array(null, $op_and, $mao),
+      ),
+    );
+
+    $this->assertCompileFunctionQueries($function_tests);
+  }
+
+  private function assertCompileQueries(
+    array $tests,
+    $operators = null,
+    PhutilSearchStemmer $stemmer = null) {
+    foreach ($tests as $input => $expect) {
+      $caught = null;
+
+      $query = null;
+      $literal_query = null;
+      $stemmed_query = null;
+
+      try {
+        $compiler = new PhutilSearchQueryCompiler();
+
+        if ($operators !== null) {
+          $compiler->setOperators($operators);
+        }
+
+        if ($stemmer !== null) {
+          $compiler->setStemmer($stemmer);
+        }
+
+        $tokens = $compiler->newTokens($input);
+
+        if ($stemmer) {
+          $literal_query = $compiler->compileLiteralQuery($tokens);
+          $stemmed_query = $compiler->compileStemmedQuery($tokens);
+        } else {
+          $query = $compiler->compileQuery($tokens);
+        }
+      } catch (PhutilSearchQueryCompilerSyntaxException $ex) {
+        $caught = $ex;
+      }
+
+      if ($caught !== null) {
+        $query = false;
+        $literal_query = false;
+        $stemmed_query = false;
+      }
+
+      if (!$stemmer) {
+        $this->assertEqual(
+          $expect,
+          $query,
+          pht('Compilation of query: %s', $input));
+      } else {
+        $this->assertEqual(
+          $expect,
+          ($literal_query === false)
+            ? false
+            : array($literal_query, $stemmed_query),
+          pht('Stemmed compilation of query: %s', $input));
+      }
+    }
+  }
+
+  private function assertCompileFunctionQueries(array $tests) {
+    foreach ($tests as $input => $expect) {
+      $compiler = id(new PhutilSearchQueryCompiler())
+        ->setEnableFunctions(true);
+
+      $tokens = $compiler->newTokens($input);
+
+      $result = array();
+      foreach ($tokens as $token) {
+        $result[] = array(
+          $token->getFunction(),
+          $token->getOperator(),
+          $token->getValue(),
+        );
+      }
+
+      $this->assertEqual(
+        $expect,
+        $result,
+        pht('Function compilation of query: %s', $input));
+    }
+  }
+
+}
diff --git a/src/applications/search/compiler/__tests__/PhutilSearchStemmerTestCase.php b/src/applications/search/compiler/__tests__/PhutilSearchStemmerTestCase.php
new file mode 100644
index 0000000000..34154d9ca3
--- /dev/null
+++ b/src/applications/search/compiler/__tests__/PhutilSearchStemmerTestCase.php
@@ -0,0 +1,85 @@
+<?php
+
+final class PhutilSearchStemmerTestCase
+  extends PhutilTestCase {
+
+  public function testStemTokens() {
+    $tests = array(
+      // Various real-world cases collected from users before we implemented
+      // stemming.
+      'tokens' => 'token',
+      'panels' => 'panel',
+
+      'renames' => 'renam',
+      'rename' => 'renam',
+
+      'components' => 'compon',
+      'component' => 'compon',
+
+      'implementation' => 'implement',
+      'implements' => 'implement',
+      'implementing' => 'implement',
+      'implementer' => 'implement',
+
+      'deleting' => 'delet',
+      'deletion' => 'delet',
+      'delete' => 'delet',
+
+      'erratically' => 'errat',
+      'erratic' => 'errat',
+
+      // Stems should be normalized.
+      'DOG' => 'dog',
+
+      // If stemming would bring a token under 3 characters, it should not
+      // be stemmed.
+      'dns' => 'dns',
+      'nis' => 'nis',
+
+      // Complex tokens with internal punctuation should be left untouched;
+      // these are usually things like domain names, API calls, informal tags,
+      // etc.
+      'apples' => 'appl',
+      'bananas' => 'banana',
+      'apples_bananas' => 'apples_bananas',
+      'apples_bananas.apples_bananas' => 'apples_bananas.apples_bananas',
+    );
+
+    $stemmer = new PhutilSearchStemmer();
+    foreach ($tests as $input => $expect) {
+      $stem = $stemmer->stemToken($input);
+      $this->assertEqual(
+        $expect,
+        $stem,
+        pht('Token stem of "%s".', $input));
+    }
+  }
+
+  public function testStemDocuments() {
+    $tests = array(
+      'The wild boar meandered erratically.' =>
+        'the wild boar meander errat',
+      'Fool me onc, shame on you. Fool me twice, shame on me.' =>
+        'fool onc shame you twice',
+      'Fireball is a seventh-level spell which deals 2d16 points of damage '.
+      'in a 1-meter radius around a target.' =>
+        'firebal seventh level spell which deal 2d16 point damag meter '.
+        'radiu around target',
+      'apples-bananas' => 'appl banana',
+      'apples_bananas' => 'apples_bananas',
+      'apples.bananas' => 'apples.bananas',
+      'oddly-proportioned' => 'oddli proport',
+    );
+
+    $stemmer = new PhutilSearchStemmer();
+    foreach ($tests as $input => $expect) {
+      $stem = $stemmer->stemCorpus($input);
+      $this->assertEqual(
+        $expect,
+        $stem,
+        pht('Corpus stem of: %s', $input));
+    }
+  }
+
+
+}

From db6b4ca480ad07590b11ac9ec54238dadbfddeff Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Jan 2020 12:02:58 -0800
Subject: [PATCH 02/17] Update deprecated array access syntax in Porter stemmer

Summary: Fixes T13472. This library uses `$a{0}`, but this is deprecated in favor of `$a[0]`.

Test Plan:
Ran `bin/search index Txxx --force` on a task with "filing" in the title (this term reaches the "m" rule of the stemmer).

(I'm not on new enough PHP for this to actually raise an error, but I'll follow up with the reporting user.)

Maniphest Tasks: T13472

Differential Revision: https://secure.phabricator.com/D20941
---
 externals/porter-stemmer/src/Porter.php | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/externals/porter-stemmer/src/Porter.php b/externals/porter-stemmer/src/Porter.php
index 0720c53ba0..b8715dade1 100644
--- a/externals/porter-stemmer/src/Porter.php
+++ b/externals/porter-stemmer/src/Porter.php
@@ -402,7 +402,7 @@ class Porter
     {
         $c = self::$regex_consonant;
 
-        return preg_match("#$c{2}$#", $str, $matches) AND $matches[0]{0} == $matches[0]{1};
+        return preg_match("#$c{2}$#", $str, $matches) AND $matches[0][0] == $matches[0][1];
     }
 
     /**
@@ -419,8 +419,8 @@ class Porter
 
         return     preg_match("#($c$v$c)$#", $str, $matches)
                AND strlen($matches[1]) == 3
-               AND $matches[1]{2} != 'w'
-               AND $matches[1]{2} != 'x'
-               AND $matches[1]{2} != 'y';
+               AND $matches[1][2] != 'w'
+               AND $matches[1][2] != 'x'
+               AND $matches[1][2] != 'y';
     }
 }

From 138ba87031475e04b134be47956fc077021437f0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Jan 2020 12:17:08 -0800
Subject: [PATCH 03/17] Guard call to "get_magic_quotes_gpc()" with "@" to
 silence PHP 7.4+ warning

Summary:
Fixes T13471. Recent versions of PHP raise a warning when this function is called.

We're only calling it so we can instantly fatal if it's enabled, so use "@" to silence the warning.

Test Plan: Loaded site; see also T13471 for a user reporting that this fix is effective.

Maniphest Tasks: T13471

Differential Revision: https://secure.phabricator.com/D20942
---
 support/startup/PhabricatorStartup.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/startup/PhabricatorStartup.php b/support/startup/PhabricatorStartup.php
index 4c577ca20c..c166162310 100644
--- a/support/startup/PhabricatorStartup.php
+++ b/support/startup/PhabricatorStartup.php
@@ -523,7 +523,7 @@ final class PhabricatorStartup {
         "'{$required_version}'.");
     }
 
-    if (get_magic_quotes_gpc()) {
+    if (@get_magic_quotes_gpc()) {
       self::didFatal(
         "Your server is configured with PHP 'magic_quotes_gpc' enabled. This ".
         "feature is 'highly discouraged' by PHP's developers and you must ".

From f806528983b6cd0323286918cc3c45e9533c068b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 15 Jan 2020 08:26:28 -0800
Subject: [PATCH 04/17] Allow the Herald Rule Editor to apply generic "Edge"
 transactions

Summary: Fixes T13469. Currently, "Mute" applies a generic edge transaction but the editor doesn't whitelist them.

Test Plan: Muted a Herald rule.

Maniphest Tasks: T13469

Differential Revision: https://secure.phabricator.com/D20943
---
 src/applications/herald/editor/HeraldRuleEditor.php | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/applications/herald/editor/HeraldRuleEditor.php b/src/applications/herald/editor/HeraldRuleEditor.php
index 1039d7432a..ea099cc6f3 100644
--- a/src/applications/herald/editor/HeraldRuleEditor.php
+++ b/src/applications/herald/editor/HeraldRuleEditor.php
@@ -30,6 +30,12 @@ final class HeraldRuleEditor
     return true;
   }
 
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
+    $types[] = PhabricatorTransactions::TYPE_EDGE;
+    return $types;
+  }
+
   protected function getMailTo(PhabricatorLiskDAO $object) {
     $phids = array();
 

From d0b01a41f2498fb2a6487c2d6704dc7acfd4675f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 15 Jan 2020 08:47:03 -0800
Subject: [PATCH 05/17] Fix two issues with missing whitespace when elements
 stack on top of each other while wrapping

Summary: Fixes T13476. Policy tags in object headers and "Visible To" controls in some dialog contexts may stack and wrap oddly. Improve spacing so they don't overlap visually when wrapping.

Test Plan: Viewed affected interfaces in narrow and wide windows.

Maniphest Tasks: T13476

Differential Revision: https://secure.phabricator.com/D20944
---
 resources/celerity/map.php                     | 14 +++++++-------
 webroot/rsrc/css/phui/phui-form.css            |  3 +--
 webroot/rsrc/css/phui/phui-header-view.css     |  1 +
 webroot/rsrc/css/phui/phui-two-column-view.css |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 23f002e36b..e94df8d522 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => 'b88ac037',
+    'core.pkg.css' => 'fd35999c',
     'core.pkg.js' => '705aec2c',
     'differential.pkg.css' => '607c84be',
     'differential.pkg.js' => '1b97518d',
@@ -153,9 +153,9 @@ return array(
     'rsrc/css/phui/phui-feed-story.css' => 'a0c05029',
     'rsrc/css/phui/phui-fontkit.css' => '1ec937e5',
     'rsrc/css/phui/phui-form-view.css' => '01b796c0',
-    'rsrc/css/phui/phui-form.css' => '159e2d9c',
+    'rsrc/css/phui/phui-form.css' => '1f177cb7',
     'rsrc/css/phui/phui-head-thing.css' => 'd7f293df',
-    'rsrc/css/phui/phui-header-view.css' => 'be09cc83',
+    'rsrc/css/phui/phui-header-view.css' => '36c86a58',
     'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0',
     'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec',
     'rsrc/css/phui/phui-icon.css' => '4cbc684a',
@@ -176,7 +176,7 @@ return array(
     'rsrc/css/phui/phui-status.css' => 'e5ff8be0',
     'rsrc/css/phui/phui-tag-view.css' => '8519160a',
     'rsrc/css/phui/phui-timeline-view.css' => '1e348e4b',
-    'rsrc/css/phui/phui-two-column-view.css' => '01e6991e',
+    'rsrc/css/phui/phui-two-column-view.css' => '0a876b9e',
     'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308',
     'rsrc/css/phui/workboards/phui-workboard.css' => '74fc9d98',
     'rsrc/css/phui/workboards/phui-workcard.css' => '913441b6',
@@ -840,10 +840,10 @@ return array(
     'phui-feed-story-css' => 'a0c05029',
     'phui-font-icon-base-css' => 'd7994e06',
     'phui-fontkit-css' => '1ec937e5',
-    'phui-form-css' => '159e2d9c',
+    'phui-form-css' => '1f177cb7',
     'phui-form-view-css' => '01b796c0',
     'phui-head-thing-view-css' => 'd7f293df',
-    'phui-header-view-css' => 'be09cc83',
+    'phui-header-view-css' => '36c86a58',
     'phui-hovercard' => '074f0783',
     'phui-hovercard-view-css' => '6ca90fa0',
     'phui-icon-set-selector-css' => '7aa5f3ec',
@@ -873,7 +873,7 @@ return array(
     'phui-tag-view-css' => '8519160a',
     'phui-theme-css' => '35883b37',
     'phui-timeline-view-css' => '1e348e4b',
-    'phui-two-column-view-css' => '01e6991e',
+    'phui-two-column-view-css' => '0a876b9e',
     'phui-workboard-color-css' => 'e86de308',
     'phui-workboard-view-css' => '74fc9d98',
     'phui-workcard-view-css' => '913441b6',
diff --git a/webroot/rsrc/css/phui/phui-form.css b/webroot/rsrc/css/phui/phui-form.css
index 1c587b5d3d..bb92df78ba 100644
--- a/webroot/rsrc/css/phui/phui-form.css
+++ b/webroot/rsrc/css/phui/phui-form.css
@@ -152,7 +152,7 @@ textarea[disabled],
 }
 
 .aphront-space-select-control-knob {
-  margin: 0 8px 0 0;
+  margin: 0 8px 4px 0;
   float: left;
 }
 
@@ -162,7 +162,6 @@ textarea[disabled],
 }
 
 .device .aphront-space-select-control-knob {
-  margin-bottom: 8px;
   float: none;
 }
 
diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css
index e621d38134..7131db5640 100644
--- a/webroot/rsrc/css/phui/phui-header-view.css
+++ b/webroot/rsrc/css/phui/phui-header-view.css
@@ -203,6 +203,7 @@ body .phui-header-shell.phui-bleed-header
   margin-right: 8px;
   -webkit-font-smoothing: auto;
   border-color: transparent;
+  line-height: 28px;
 }
 
 
diff --git a/webroot/rsrc/css/phui/phui-two-column-view.css b/webroot/rsrc/css/phui/phui-two-column-view.css
index f9f86d950a..3e58f58173 100644
--- a/webroot/rsrc/css/phui/phui-two-column-view.css
+++ b/webroot/rsrc/css/phui/phui-two-column-view.css
@@ -50,7 +50,7 @@
 }
 
 .phui-two-column-header .phui-header-subheader {
-  margin-top: 12px;
+  margin-top: 8px;
 }
 
 .phui-two-column-subheader {

From 6ccb6a6463f78e135455332401d2f84950409828 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 16 Jan 2020 11:33:45 -0800
Subject: [PATCH 06/17] Update "git rev-parse" invocation to work in Git 2.25.0

Summary:
Fixes T13479. The behavior of "git rev-parse --show-toplevel" has changed in Git 2.25.0, and it now fails in bare repositories.

Instead, use "git rev-parse --git-dir" to sanity-check the working copy. This appears to have more stable behavior across Git versions, although it's a little more complicated for our purposes.

Test Plan:
  - Ran `bin/repository update ...` on an observed, bare repository.
  - ...on an observed, non-bare ("legacy") repository.
  - ...on a hosted, bare repository.

Maniphest Tasks: T13479

Differential Revision: https://secure.phabricator.com/D20945
---
 .../PhabricatorRepositoryPullEngine.php       | 49 ++++++++++++++-----
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
index 2b008b630b..7d216421d7 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -275,11 +275,39 @@ final class PhabricatorRepositoryPullEngine
   private function executeGitUpdate() {
     $repository = $this->getRepository();
 
+    // See T13479. We previously used "--show-toplevel", but this stopped
+    // working in Git 2.25.0 when run in a bare repository.
+
+    // NOTE: As of Git 2.21.1, "git rev-parse" can not parse "--" in its
+    // argument list, so we can not specify arguments unambiguously. Any
+    // version of Git which does not recognize the "--git-dir" flag will
+    // treat this as a request to parse the literal refname "--git-dir".
+
     list($err, $stdout) = $repository->execLocalCommand(
-      'rev-parse --show-toplevel');
+      'rev-parse --git-dir');
+
+    $repository_root = null;
+    $path = $repository->getLocalPath();
+
+    if (!$err) {
+      $repository_root = Filesystem::resolvePath(
+        rtrim($stdout, "\n"),
+        $path);
+
+      // If we're in a bare Git repository, the "--git-dir" will be the
+      // root directory. If we're in a working copy, the "--git-dir" will
+      // be the ".git/" directory.
+
+      // Test if the result is the root directory. If it is, we're in good
+      // shape and appear to be inside a bare repository. If not, take the
+      // parent directory to get out of the ".git/" folder.
+
+      if (!Filesystem::pathsAreEquivalent($repository_root, $path)) {
+        $repository_root = dirname($repository_root);
+      }
+    }
 
     $message = null;
-    $path = $repository->getLocalPath();
     if ($err) {
       // Try to raise a more tailored error message in the more common case
       // of the user creating an empty directory. (We could try to remove it,
@@ -313,15 +341,14 @@ final class PhabricatorRepositoryPullEngine
           $path);
       }
     } else {
-      $repo_path = rtrim($stdout, "\n");
 
-      if (empty($repo_path)) {
-        // This can mean one of two things: we're in a bare repository, or
-        // we're inside a git repository inside another git repository. Since
-        // the first is dramatically more likely now that we perform bare
-        // clones and I don't have a great way to test for the latter, assume
-        // we're OK.
-      } else if (!Filesystem::pathsAreEquivalent($repo_path, $path)) {
+      // Prior to Git 2.25.0, we used "--show-toplevel", which had a weird
+      // case here when the working copy was inside another working copy.
+      // The switch to "--git-dir" seems to have resolved this; we now seem
+      // to find the nearest git directory and thus the correct repository
+      // root.
+
+      if (!Filesystem::pathsAreEquivalent($repository_root, $path)) {
         $err = true;
         $message = pht(
           'Expected to find a Git repository at "%s", but the actual Git '.
@@ -329,7 +356,7 @@ final class PhabricatorRepositoryPullEngine
           'misconfigured. This directory should be writable by the daemons '.
           'and not inside another Git repository.',
           $path,
-          $repo_path);
+          $repository_root);
       }
     }
 

From 6c4500046f7c9f9dfe7f5afe67f83f758a480ec3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 21 Jan 2020 11:22:05 -0800
Subject: [PATCH 07/17] Add "Project tags added" and "Project tags removed"
 fields in Herald

Summary: Ref T13480. These fields don't serve a specific strong use case, but are broadly reasonable capabilities after "state" vs "change" actions were relaxed by T13283.

Test Plan: Wrote rules using the new fields, added and removed projects (and did neither) to fire them / not fire them. Inspected the transcripts to see the project PHIDs making it to the field values.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20946
---
 src/__phutil_library_map__.php                |  8 +++-
 src/applications/herald/field/HeraldField.php | 45 +++++++++++++++++++
 .../project/herald/HeraldProjectsField.php    | 19 +-------
 .../PhabricatorProjectTagsAddedField.php      | 23 ++++++++++
 .../herald/PhabricatorProjectTagsField.php    | 27 +++++++++++
 .../PhabricatorProjectTagsRemovedField.php    | 23 ++++++++++
 6 files changed, 127 insertions(+), 18 deletions(-)
 create mode 100644 src/applications/project/herald/PhabricatorProjectTagsAddedField.php
 create mode 100644 src/applications/project/herald/PhabricatorProjectTagsField.php
 create mode 100644 src/applications/project/herald/PhabricatorProjectTagsRemovedField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 9b15a81c49..718b33c822 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4382,6 +4382,9 @@ phutil_register_library_map(array(
     'PhabricatorProjectSubprojectsProfileMenuItem' => 'applications/project/menuitem/PhabricatorProjectSubprojectsProfileMenuItem.php',
     'PhabricatorProjectSubtypeDatasource' => 'applications/project/typeahead/PhabricatorProjectSubtypeDatasource.php',
     'PhabricatorProjectSubtypesConfigType' => 'applications/project/config/PhabricatorProjectSubtypesConfigType.php',
+    'PhabricatorProjectTagsAddedField' => 'applications/project/herald/PhabricatorProjectTagsAddedField.php',
+    'PhabricatorProjectTagsField' => 'applications/project/herald/PhabricatorProjectTagsField.php',
+    'PhabricatorProjectTagsRemovedField' => 'applications/project/herald/PhabricatorProjectTagsRemovedField.php',
     'PhabricatorProjectTestDataGenerator' => 'applications/project/lipsum/PhabricatorProjectTestDataGenerator.php',
     'PhabricatorProjectTransaction' => 'applications/project/storage/PhabricatorProjectTransaction.php',
     'PhabricatorProjectTransactionEditor' => 'applications/project/editor/PhabricatorProjectTransactionEditor.php',
@@ -7679,7 +7682,7 @@ phutil_register_library_map(array(
     'HeraldPreCommitContentAdapter' => 'HeraldPreCommitAdapter',
     'HeraldPreCommitRefAdapter' => 'HeraldPreCommitAdapter',
     'HeraldPreventActionGroup' => 'HeraldActionGroup',
-    'HeraldProjectsField' => 'HeraldField',
+    'HeraldProjectsField' => 'PhabricatorProjectTagsField',
     'HeraldRecursiveConditionsException' => 'Exception',
     'HeraldRelatedFieldGroup' => 'HeraldFieldGroup',
     'HeraldRemarkupFieldValue' => 'HeraldFieldValue',
@@ -10943,6 +10946,9 @@ phutil_register_library_map(array(
     'PhabricatorProjectSubprojectsProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorProjectSubtypeDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorProjectSubtypesConfigType' => 'PhabricatorJSONConfigType',
+    'PhabricatorProjectTagsAddedField' => 'PhabricatorProjectTagsField',
+    'PhabricatorProjectTagsField' => 'HeraldField',
+    'PhabricatorProjectTagsRemovedField' => 'PhabricatorProjectTagsField',
     'PhabricatorProjectTestDataGenerator' => 'PhabricatorTestDataGenerator',
     'PhabricatorProjectTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorProjectTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index 2a1e89d558..fc021d3386 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -241,6 +241,51 @@ abstract class HeraldField extends Phobject {
     return false;
   }
 
+  final protected function getAppliedTransactionsOfTypes(array $types) {
+    $types = array_fuse($types);
+    $xactions = $this->getAdapter()->getAppliedTransactions();
+
+    $result = array();
+    foreach ($xactions as $key => $xaction) {
+      $xaction_type = $xaction->getTransactionType();
+      if (isset($types[$xaction_type])) {
+        $result[$key] = $xaction;
+      }
+    }
+
+    return $result;
+  }
+
+  final protected function getAppliedEdgeTransactionOfType($edge_type) {
+    $edge_xactions = $this->getAppliedTransactionsOfTypes(
+      array(
+        PhabricatorTransactions::TYPE_EDGE,
+      ));
+
+    $results = array();
+    foreach ($edge_xactions as $edge_xaction) {
+      $xaction_edge_type = $edge_xaction->getMetadataValue('edge:type');
+      if ($xaction_edge_type == $edge_type) {
+        $results[] = $edge_xaction;
+      }
+    }
+
+    if (count($results) > 1) {
+      throw new Exception(
+        pht(
+          'Found more than one ("%s") applied edge transactions with given '.
+          'edge type ("%s"); expected zero or one.',
+          phutil_count($results),
+          $edge_type));
+    }
+
+    if ($results) {
+      return head($results);
+    }
+
+    return null;
+  }
+
   public function isFieldAvailable() {
     return true;
   }
diff --git a/src/applications/project/herald/HeraldProjectsField.php b/src/applications/project/herald/HeraldProjectsField.php
index 7f66a00cf3..504bde8967 100644
--- a/src/applications/project/herald/HeraldProjectsField.php
+++ b/src/applications/project/herald/HeraldProjectsField.php
@@ -1,6 +1,7 @@
 <?php
 
-final class HeraldProjectsField extends HeraldField {
+final class HeraldProjectsField
+  extends PhabricatorProjectTagsField {
 
   const FIELDCONST = 'projects';
 
@@ -8,26 +9,10 @@ final class HeraldProjectsField extends HeraldField {
     return pht('Project tags');
   }
 
-  public function getFieldGroupKey() {
-    return HeraldSupportFieldGroup::FIELDGROUPKEY;
-  }
-
-  public function supportsObject($object) {
-    return ($object instanceof PhabricatorProjectInterface);
-  }
-
   public function getHeraldFieldValue($object) {
     return PhabricatorEdgeQuery::loadDestinationPHIDs(
       $object->getPHID(),
       PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
   }
 
-  protected function getHeraldFieldStandardType() {
-    return self::STANDARD_PHID_LIST;
-  }
-
-  protected function getDatasource() {
-    return new PhabricatorProjectDatasource();
-  }
-
 }
diff --git a/src/applications/project/herald/PhabricatorProjectTagsAddedField.php b/src/applications/project/herald/PhabricatorProjectTagsAddedField.php
new file mode 100644
index 0000000000..61a974aba2
--- /dev/null
+++ b/src/applications/project/herald/PhabricatorProjectTagsAddedField.php
@@ -0,0 +1,23 @@
+<?php
+
+final class PhabricatorProjectTagsAddedField
+  extends PhabricatorProjectTagsField {
+
+  const FIELDCONST = 'projects.added';
+
+  public function getHeraldFieldName() {
+    return pht('Project tags added');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $xaction = $this->getProjectTagsTransaction();
+    if (!$xaction) {
+      return array();
+    }
+
+    $record = PhabricatorEdgeChangeRecord::newFromTransaction($xaction);
+
+    return $record->getAddedPHIDs();
+  }
+
+}
diff --git a/src/applications/project/herald/PhabricatorProjectTagsField.php b/src/applications/project/herald/PhabricatorProjectTagsField.php
new file mode 100644
index 0000000000..36ea103b00
--- /dev/null
+++ b/src/applications/project/herald/PhabricatorProjectTagsField.php
@@ -0,0 +1,27 @@
+<?php
+
+abstract class PhabricatorProjectTagsField
+  extends HeraldField {
+
+  public function getFieldGroupKey() {
+    return HeraldSupportFieldGroup::FIELDGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return ($object instanceof PhabricatorProjectInterface);
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorProjectDatasource();
+  }
+
+  final protected function getProjectTagsTransaction() {
+    return $this->getAppliedEdgeTransactionOfType(
+      PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
+  }
+
+}
diff --git a/src/applications/project/herald/PhabricatorProjectTagsRemovedField.php b/src/applications/project/herald/PhabricatorProjectTagsRemovedField.php
new file mode 100644
index 0000000000..7e249c266b
--- /dev/null
+++ b/src/applications/project/herald/PhabricatorProjectTagsRemovedField.php
@@ -0,0 +1,23 @@
+<?php
+
+final class PhabricatorProjectTagsRemovedField
+  extends PhabricatorProjectTagsField {
+
+  const FIELDCONST = 'projects.removed';
+
+  public function getHeraldFieldName() {
+    return pht('Project tags removed');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $xaction = $this->getProjectTagsTransaction();
+    if (!$xaction) {
+      return array();
+    }
+
+    $record = PhabricatorEdgeChangeRecord::newFromTransaction($xaction);
+
+    return $record->getRemovedPHIDs();
+  }
+
+}

From b38449ce8f4ba84de8f30858cb74d11582ddcc76 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 21 Jan 2020 11:37:40 -0800
Subject: [PATCH 08/17] Implement an "Author's packages" Herald field for
 Differential

Summary: Ref T13480. Add an "Author's packages" field to Herald to support writing rules like "if affected packages include X, and author's packages do not include X, raise the alarm".

Test Plan: Wrote and executed rules with the field, saw a sensible field value in the transcript.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20947
---
 src/__phutil_library_map__.php                |  2 ++
 ...ntialRevisionAuthorPackagesHeraldField.php | 32 +++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 src/applications/differential/herald/DifferentialRevisionAuthorPackagesHeraldField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 718b33c822..b073642683 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -603,6 +603,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionActionTransaction' => 'applications/differential/xaction/DifferentialRevisionActionTransaction.php',
     'DifferentialRevisionAffectedFilesHeraldField' => 'applications/differential/herald/DifferentialRevisionAffectedFilesHeraldField.php',
     'DifferentialRevisionAuthorHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorHeraldField.php',
+    'DifferentialRevisionAuthorPackagesHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorPackagesHeraldField.php',
     'DifferentialRevisionAuthorProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorProjectsHeraldField.php',
     'DifferentialRevisionBuildableTransaction' => 'applications/differential/xaction/DifferentialRevisionBuildableTransaction.php',
     'DifferentialRevisionCloseDetailsController' => 'applications/differential/controller/DifferentialRevisionCloseDetailsController.php',
@@ -6595,6 +6596,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionActionTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionAffectedFilesHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionAuthorPackagesHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionAuthorProjectsHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionBuildableTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
diff --git a/src/applications/differential/herald/DifferentialRevisionAuthorPackagesHeraldField.php b/src/applications/differential/herald/DifferentialRevisionAuthorPackagesHeraldField.php
new file mode 100644
index 0000000000..89a7df4cdb
--- /dev/null
+++ b/src/applications/differential/herald/DifferentialRevisionAuthorPackagesHeraldField.php
@@ -0,0 +1,32 @@
+<?php
+
+final class DifferentialRevisionAuthorPackagesHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.author.packages';
+
+  public function getHeraldFieldName() {
+    return pht("Author's packages");
+  }
+
+  public function getHeraldFieldValue($object) {
+    $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
+
+    $packages = id(new PhabricatorOwnersPackageQuery())
+      ->setViewer($viewer)
+      ->withAuthorityPHIDs(array($object->getAuthorPHID()))
+      ->execute();
+
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorOwnersPackageDatasource();
+  }
+
+}

From 7a1681b8dae1e58c8efd26fe38e3c1c565ec1e9d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 29 Jan 2020 08:49:12 -0800
Subject: [PATCH 09/17] Don't use "line-through" style for completed items in
 remarkup checklists

Summary: Fixes T13482. Although this style makes physical sense by relationship to a written checklist, it seems to do more harm than good in practice.

Test Plan: Wrote a checklist with a checked-off item in remarkup, saw no more line-through.

Maniphest Tasks: T13482

Differential Revision: https://secure.phabricator.com/D20954
---
 resources/celerity/map.php         | 6 +++---
 webroot/rsrc/css/core/remarkup.css | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index e94df8d522..9b8013c826 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => 'fd35999c',
+    'core.pkg.css' => 'ad8fc332',
     'core.pkg.js' => '705aec2c',
     'differential.pkg.css' => '607c84be',
     'differential.pkg.js' => '1b97518d',
@@ -112,7 +112,7 @@ return array(
     'rsrc/css/application/tokens/tokens.css' => 'ce5a50bd',
     'rsrc/css/application/uiexample/example.css' => 'b4795059',
     'rsrc/css/core/core.css' => '1b29ed61',
-    'rsrc/css/core/remarkup.css' => 'f06cc20e',
+    'rsrc/css/core/remarkup.css' => 'c286eaef',
     'rsrc/css/core/syntax.css' => '220b85f9',
     'rsrc/css/core/z-index.css' => '99c0f5eb',
     'rsrc/css/diviner/diviner-shared.css' => '4bd263b0',
@@ -794,7 +794,7 @@ return array(
     'phabricator-object-selector-css' => 'ee77366f',
     'phabricator-phtize' => '2f1db1ed',
     'phabricator-prefab' => '5793d835',
-    'phabricator-remarkup-css' => 'f06cc20e',
+    'phabricator-remarkup-css' => 'c286eaef',
     'phabricator-search-results-css' => '9ea70ace',
     'phabricator-shaped-request' => 'abf88db8',
     'phabricator-slowvote-css' => '1694baed',
diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css
index 3354be9663..6e1c4b3627 100644
--- a/webroot/rsrc/css/core/remarkup.css
+++ b/webroot/rsrc/css/core/remarkup.css
@@ -155,7 +155,6 @@
 
 .phabricator-remarkup .remarkup-list-with-checkmarks .remarkup-checked-item {
   color: {$lightgreytext};
-  text-decoration: line-through;
 }
 
 .phabricator-remarkup ul.remarkup-list ol.remarkup-list,

From a5a9a5e0027f5dbafd1d12429ae965a07d999ff5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Jan 2020 11:32:11 -0800
Subject: [PATCH 10/17] Remove legacy pre-loading of handles from Herald
 rendering

Summary: Ref T13480. When Herald renders rules, it partly uses a very old handle pre-loading mechanism where PHIDs are extracted and loaded upfront. This was obsoleted a long time ago and was pretty shaky even when it worked. Get rid of it to simplify the code a little.

Test Plan: Viewed Herald rules rendered into static text with PHID list actions, saw handles. Grepped for all affected methods.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20948
---
 .../herald/adapter/HeraldAdapter.php          | 107 ++----------------
 .../controller/HeraldRuleViewController.php   |   5 +-
 2 files changed, 13 insertions(+), 99 deletions(-)

diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index 70f6e3d5cb..5c6f8c9954 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -942,7 +942,6 @@ abstract class HeraldAdapter extends Phobject {
 
   public function renderRuleAsText(
     HeraldRule $rule,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     require_celerity_resource('herald-css');
@@ -973,7 +972,7 @@ abstract class HeraldAdapter extends Phobject {
         ),
         array(
           $icon,
-          $this->renderConditionAsText($condition, $handles, $viewer),
+          $this->renderConditionAsText($condition, $viewer),
         ));
     }
 
@@ -1004,7 +1003,7 @@ abstract class HeraldAdapter extends Phobject {
         ),
         array(
           $icon,
-          $this->renderActionAsText($viewer, $action, $handles),
+          $this->renderActionAsText($viewer, $action),
         ));
     }
 
@@ -1018,7 +1017,6 @@ abstract class HeraldAdapter extends Phobject {
 
   private function renderConditionAsText(
     HeraldCondition $condition,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     $field_type = $condition->getFieldName();
@@ -1033,7 +1031,7 @@ abstract class HeraldAdapter extends Phobject {
     $condition_type = $condition->getFieldCondition();
     $condition_name = idx($this->getConditionNameMap(), $condition_type);
 
-    $value = $this->renderConditionValueAsText($condition, $handles, $viewer);
+    $value = $this->renderConditionValueAsText($condition, $viewer);
 
     return array(
       $field_name,
@@ -1046,36 +1044,23 @@ abstract class HeraldAdapter extends Phobject {
 
   private function renderActionAsText(
     PhabricatorUser $viewer,
-    HeraldActionRecord $action,
-    PhabricatorHandleList $handles) {
+    HeraldActionRecord $action_record) {
 
-    $impl = $this->getActionImplementation($action->getAction());
-    if ($impl) {
-      $impl->setViewer($viewer);
+    $action_type = $action_record->getAction();
+    $action_value = $action_record->getTarget();
 
-      $value = $action->getTarget();
-      return $impl->renderActionDescription($value);
+    $action = $this->getActionImplementation($action_type);
+    if (!$action) {
+      return pht('Unknown Action ("%s")', $action_type);
     }
 
-    $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
+    $action->setViewer($viewer);
 
-    $action_type = $action->getAction();
-
-    $default = pht('(Unknown Action "%s") equals', $action_type);
-
-    $action_name = idx(
-      $this->getActionNameMap($rule_global),
-      $action_type,
-      $default);
-
-    $target = $this->renderActionTargetAsText($action, $handles);
-
-    return hsprintf('    %s %s', $action_name, $target);
+    return $action->renderActionDescription($action_value);
   }
 
   private function renderConditionValueAsText(
     HeraldCondition $condition,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     $field = $this->requireFieldImplementation($condition->getFieldName());
@@ -1086,76 +1071,6 @@ abstract class HeraldAdapter extends Phobject {
       $condition->getValue());
   }
 
-  private function renderActionTargetAsText(
-    HeraldActionRecord $action,
-    PhabricatorHandleList $handles) {
-
-    // TODO: This should be driven through HeraldAction.
-
-    $target = $action->getTarget();
-    if (!is_array($target)) {
-      $target = array($target);
-    }
-    foreach ($target as $index => $val) {
-      switch ($action->getAction()) {
-        default:
-          $handle = $handles->getHandleIfExists($val);
-          if ($handle) {
-            $target[$index] = $handle->renderLink();
-          }
-          break;
-      }
-    }
-    $target = phutil_implode_html(', ', $target);
-    return $target;
-  }
-
-  /**
-   * Given a @{class:HeraldRule}, this function extracts all the phids that
-   * we'll want to load as handles later.
-   *
-   * This function performs a somewhat hacky approach to figuring out what
-   * is and is not a phid - try to get the phid type and if the type is
-   * *not* unknown assume its a valid phid.
-   *
-   * Don't try this at home. Use more strongly typed data at home.
-   *
-   * Think of the children.
-   */
-  public static function getHandlePHIDs(HeraldRule $rule) {
-    $phids = array($rule->getAuthorPHID());
-    foreach ($rule->getConditions() as $condition) {
-      $value = $condition->getValue();
-      if (!is_array($value)) {
-        $value = array($value);
-      }
-      foreach ($value as $val) {
-        if (phid_get_type($val) !=
-            PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
-          $phids[] = $val;
-        }
-      }
-    }
-
-    foreach ($rule->getActions() as $action) {
-      $target = $action->getTarget();
-      if (!is_array($target)) {
-        $target = array($target);
-      }
-      foreach ($target as $val) {
-        if (phid_get_type($val) !=
-            PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
-          $phids[] = $val;
-        }
-      }
-    }
-
-    if ($rule->isObjectRule()) {
-      $phids[] = $rule->getTriggerObjectPHID();
-    }
-
-    return $phids;
-  }
 
 /* -(  Applying Effects  )--------------------------------------------------- */
 
diff --git a/src/applications/herald/controller/HeraldRuleViewController.php b/src/applications/herald/controller/HeraldRuleViewController.php
index 70a2d20224..49e0fd8e32 100644
--- a/src/applications/herald/controller/HeraldRuleViewController.php
+++ b/src/applications/herald/controller/HeraldRuleViewController.php
@@ -143,12 +143,11 @@ final class HeraldRuleViewController extends HeraldController {
   private function buildDescriptionView(HeraldRule $rule) {
     $viewer = $this->getRequest()->getUser();
     $view = id(new PHUIPropertyListView())
-      ->setUser($viewer);
+      ->setViewer($viewer);
 
     $adapter = HeraldAdapter::getAdapterForContentType($rule->getContentType());
     if ($adapter) {
-      $handles = $viewer->loadHandles(HeraldAdapter::getHandlePHIDs($rule));
-      $rule_text = $adapter->renderRuleAsText($rule, $handles, $viewer);
+      $rule_text = $adapter->renderRuleAsText($rule, $viewer);
       $view->addTextContent($rule_text);
       return $view;
     }

From 19662e33bc4565f69766dc2a16bb79d1c29e51fe Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Jan 2020 11:47:31 -0800
Subject: [PATCH 11/17] In Herald transcript rendering, don't store display
 labels in keys

Summary:
Ref T13480. Currently, when Herald renders a transcript, it puts display labels into array keys. This is a bad pattern for several reasons, notably that the values must be scalar (so you can't add icons or other markup later) and the values must be unique (which is easily violated because many values are translated).

Instead, keep values as list items.

Test Plan: Viewed Herald transcripts, saw no (meaningful) change in rendering output.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20949
---
 .../controller/HeraldTranscriptController.php | 51 +++++++++++++------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php
index 09a23408d3..bf4e6f635f 100644
--- a/src/applications/herald/controller/HeraldTranscriptController.php
+++ b/src/applications/herald/controller/HeraldTranscriptController.php
@@ -453,31 +453,47 @@ final class HeraldTranscriptController extends HeraldController {
 
     $object_xscript = $xscript->getObjectTranscript();
 
-    $data = array();
+    $rows = array();
     if ($object_xscript) {
       $phid = $object_xscript->getPHID();
       $handles = $this->handles;
 
-      $data += array(
-        pht('Object Name') => $object_xscript->getName(),
-        pht('Object Type') => $object_xscript->getType(),
-        pht('Object PHID') => $phid,
-        pht('Object Link') => $handles[$phid]->renderLink(),
+      $rows[] = array(
+        pht('Object Name'),
+        $object_xscript->getName(),
+      );
+
+      $rows[] = array(
+        pht('Object Type'),
+        $object_xscript->getType(),
+      );
+
+      $rows[] = array(
+        pht('Object PHID'),
+        $phid,
+      );
+
+      $rows[] = array(
+        pht('Object Link'),
+        $handles[$phid]->renderLink(),
       );
     }
 
-    $data += $xscript->getMetadataMap();
+    foreach ($xscript->getMetadataMap() as $key => $value) {
+      $rows[] = array(
+        $key,
+        $value,
+      );
+    }
 
     if ($object_xscript) {
       foreach ($object_xscript->getFields() as $field => $value) {
-        $field = idx($field_names, $field, '['.$field.'?]');
-        $data['Field: '.$field] = $value;
-      }
-    }
+        if (isset($field_names[$field])) {
+          $field_name = pht('Field: %s', $field_names[$field]);
+        } else {
+          $field_name = pht('Unknown Field ("%s")', $field_name);
+        }
 
-    $rows = array();
-    foreach ($data as $name => $value) {
-      if (!($value instanceof PhutilSafeHTML)) {
         if (!is_scalar($value) && !is_null($value)) {
           $value = implode("\n", $value);
         }
@@ -490,9 +506,12 @@ final class HeraldTranscriptController extends HeraldController {
             ),
             $value);
         }
-      }
 
-      $rows[] = array($name, $value);
+        $rows[] = array(
+          $field_name,
+          $value,
+        );
+      }
     }
 
     $property_list = new PHUIPropertyListView();

From a0a346be347e6250ed109f2265a2abc8fc1c166b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Jan 2020 15:34:17 -0800
Subject: [PATCH 12/17] In Herald transcripts, render some field values in a
 more readable way

Summary:
Ref T13480. Currently, some Herald field types are rendered in an unfriendly way on transcripts. Particularly, PHID lists are rendered as raw PHIDs.

Improve this by delegating rendering to Value objects and letting "PHID List" value objects render more sensible handle lists. Also improve "bool" fields a bit and make more fields render an explicit "None" / empty value rather than just rendering nothing.

Test Plan: Viewed various transcripts, including transcripts covering boolean values, the "Always" condition, large blocks of text, and PHID lists.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20951
---
 src/__phutil_library_map__.php                |  2 +
 .../herald/adapter/HeraldAdapter.php          | 20 +++++++++
 .../controller/HeraldTranscriptController.php | 29 +++++--------
 .../herald/field/HeraldAlwaysField.php        |  2 +-
 src/applications/herald/field/HeraldField.php | 15 ++++++-
 .../herald/value/HeraldBoolFieldValue.php     | 30 ++++++++++++++
 .../herald/value/HeraldFieldValue.php         |  4 ++
 .../herald/value/HeraldTextFieldValue.php     | 21 ++++++++++
 .../value/HeraldTokenizerFieldValue.php       | 41 ++++++++++++++-----
 9 files changed, 133 insertions(+), 31 deletions(-)
 create mode 100644 src/applications/herald/value/HeraldBoolFieldValue.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index b073642683..cf59036387 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1528,6 +1528,7 @@ phutil_register_library_map(array(
     'HeraldApplicationActionGroup' => 'applications/herald/action/HeraldApplicationActionGroup.php',
     'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php',
     'HeraldBasicFieldGroup' => 'applications/herald/field/HeraldBasicFieldGroup.php',
+    'HeraldBoolFieldValue' => 'applications/herald/value/HeraldBoolFieldValue.php',
     'HeraldBuildableState' => 'applications/herald/state/HeraldBuildableState.php',
     'HeraldCallWebhookAction' => 'applications/herald/action/HeraldCallWebhookAction.php',
     'HeraldCommentAction' => 'applications/herald/action/HeraldCommentAction.php',
@@ -7633,6 +7634,7 @@ phutil_register_library_map(array(
     'HeraldApplicationActionGroup' => 'HeraldActionGroup',
     'HeraldApplyTranscript' => 'Phobject',
     'HeraldBasicFieldGroup' => 'HeraldFieldGroup',
+    'HeraldBoolFieldValue' => 'HeraldFieldValue',
     'HeraldBuildableState' => 'HeraldState',
     'HeraldCallWebhookAction' => 'HeraldAction',
     'HeraldCommentAction' => 'HeraldAction',
diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index 5c6f8c9954..76955751c3 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -1071,6 +1071,26 @@ abstract class HeraldAdapter extends Phobject {
       $condition->getValue());
   }
 
+  public function renderFieldTranscriptValue(
+    PhabricatorUser $viewer,
+    $field_type,
+    $field_value) {
+
+    $field = $this->getFieldImplementation($field_type);
+    if ($field) {
+      return $field->renderTranscriptValue(
+        $viewer,
+        $field_value);
+    }
+
+    return phutil_tag(
+      'em',
+      array(),
+      pht(
+        'Unable to render value for unknown field type ("%s").',
+        $field_type));
+  }
+
 
 /* -(  Applying Effects  )--------------------------------------------------- */
 
diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php
index bf4e6f635f..6cbe4bd808 100644
--- a/src/applications/herald/controller/HeraldTranscriptController.php
+++ b/src/applications/herald/controller/HeraldTranscriptController.php
@@ -447,8 +447,9 @@ final class HeraldTranscriptController extends HeraldController {
   }
 
   private function buildObjectTranscriptPanel(HeraldTranscript $xscript) {
-
+    $viewer = $this->getViewer();
     $adapter = $this->getAdapter();
+
     $field_names = $adapter->getFieldNameMap();
 
     $object_xscript = $xscript->getObjectTranscript();
@@ -487,29 +488,21 @@ final class HeraldTranscriptController extends HeraldController {
     }
 
     if ($object_xscript) {
-      foreach ($object_xscript->getFields() as $field => $value) {
-        if (isset($field_names[$field])) {
-          $field_name = pht('Field: %s', $field_names[$field]);
+      foreach ($object_xscript->getFields() as $field_type => $value) {
+        if (isset($field_names[$field_type])) {
+          $field_name = pht('Field: %s', $field_names[$field_type]);
         } else {
-          $field_name = pht('Unknown Field ("%s")', $field_name);
+          $field_name = pht('Unknown Field ("%s")', $field_type);
         }
 
-        if (!is_scalar($value) && !is_null($value)) {
-          $value = implode("\n", $value);
-        }
-
-        if (strlen($value) > 256) {
-          $value = phutil_tag(
-            'textarea',
-            array(
-              'class' => 'herald-field-value-transcript',
-            ),
-            $value);
-        }
+        $field_value = $adapter->renderFieldTranscriptValue(
+          $viewer,
+          $field_type,
+          $value);
 
         $rows[] = array(
           $field_name,
-          $value,
+          $field_value,
         );
       }
     }
diff --git a/src/applications/herald/field/HeraldAlwaysField.php b/src/applications/herald/field/HeraldAlwaysField.php
index 13d3649ef2..bcc7a10e37 100644
--- a/src/applications/herald/field/HeraldAlwaysField.php
+++ b/src/applications/herald/field/HeraldAlwaysField.php
@@ -23,7 +23,7 @@ final class HeraldAlwaysField extends HeraldField {
   }
 
   public function getHeraldFieldValueType($condition) {
-    return new HeraldEmptyFieldValue();
+    return new HeraldBoolFieldValue();
   }
 
   public function supportsObject($object) {
diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index fc021d3386..cdfdd7e518 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -105,11 +105,16 @@ abstract class HeraldField extends Phobject {
   }
 
   public function getHeraldFieldValueType($condition) {
+
+    // NOTE: The condition type may be "null" to indicate that the caller
+    // wants a generic field value type. This is used when rendering field
+    // values in the object transcript.
+
     $standard_type = $this->getHeraldFieldStandardType();
     switch ($standard_type) {
       case self::STANDARD_BOOL:
       case self::STANDARD_PHID_BOOL:
-        return new HeraldEmptyFieldValue();
+        return new HeraldBoolFieldValue();
       case self::STANDARD_TEXT:
       case self::STANDARD_TEXT_LIST:
       case self::STANDARD_TEXT_MAP:
@@ -176,6 +181,14 @@ abstract class HeraldField extends Phobject {
     return $value_type->renderEditorValue($value);
   }
 
+  public function renderTranscriptValue(
+    PhabricatorUser $viewer,
+    $field_value) {
+    $value_type = $this->getHeraldFieldValueType($condition_type = null);
+    $value_type->setViewer($viewer);
+    return $value_type->renderTranscriptValue($field_value);
+  }
+
   public function getPHIDsAffectedByCondition(HeraldCondition $condition) {
     try {
       $standard_type = $this->getHeraldFieldStandardType();
diff --git a/src/applications/herald/value/HeraldBoolFieldValue.php b/src/applications/herald/value/HeraldBoolFieldValue.php
new file mode 100644
index 0000000000..83acf5aadf
--- /dev/null
+++ b/src/applications/herald/value/HeraldBoolFieldValue.php
@@ -0,0 +1,30 @@
+<?php
+
+final class HeraldBoolFieldValue
+  extends HeraldFieldValue {
+
+  public function getFieldValueKey() {
+    return 'bool';
+  }
+
+  public function getControlType() {
+    return self::CONTROL_NONE;
+  }
+
+  public function renderFieldValue($value) {
+    return null;
+  }
+
+  public function renderEditorValue($value) {
+    return null;
+  }
+
+  public function renderTranscriptValue($value) {
+    if ($value) {
+      return pht('true');
+    } else {
+      return pht('false');
+    }
+  }
+
+}
diff --git a/src/applications/herald/value/HeraldFieldValue.php b/src/applications/herald/value/HeraldFieldValue.php
index c88776f53a..5ca88b56ee 100644
--- a/src/applications/herald/value/HeraldFieldValue.php
+++ b/src/applications/herald/value/HeraldFieldValue.php
@@ -36,4 +36,8 @@ abstract class HeraldFieldValue extends Phobject {
     return array();
   }
 
+  public function renderTranscriptValue($value) {
+    return $this->renderFieldValue($value);
+  }
+
 }
diff --git a/src/applications/herald/value/HeraldTextFieldValue.php b/src/applications/herald/value/HeraldTextFieldValue.php
index fd032e33c0..4a9780fd0e 100644
--- a/src/applications/herald/value/HeraldTextFieldValue.php
+++ b/src/applications/herald/value/HeraldTextFieldValue.php
@@ -19,4 +19,25 @@ final class HeraldTextFieldValue
     return $value;
   }
 
+  public function renderTranscriptValue($value) {
+    if (is_array($value)) {
+      $value = implode('', $value);
+    }
+
+    if (!strlen($value)) {
+      return phutil_tag('em', array(), pht('None'));
+    }
+
+    if (strlen($value) > 256) {
+      $value = phutil_tag(
+        'textarea',
+        array(
+          'class' => 'herald-field-value-transcript',
+        ),
+        $value);
+    }
+
+    return $value;
+  }
+
 }
diff --git a/src/applications/herald/value/HeraldTokenizerFieldValue.php b/src/applications/herald/value/HeraldTokenizerFieldValue.php
index 215df29693..4f90267112 100644
--- a/src/applications/herald/value/HeraldTokenizerFieldValue.php
+++ b/src/applications/herald/value/HeraldTokenizerFieldValue.php
@@ -64,17 +64,7 @@ final class HeraldTokenizerFieldValue
   }
 
   public function renderFieldValue($value) {
-    $viewer = $this->getViewer();
-    $value = (array)$value;
-
-    if ($this->valueMap !== null) {
-      foreach ($value as $k => $v) {
-        $value[$k] = idx($this->valueMap, $v, $v);
-      }
-      return implode(', ', $value);
-    }
-
-    return $viewer->renderHandleList((array)$value)->setAsInline(true);
+    return $this->renderValueAsList($value, $for_transcript = false);
   }
 
   public function renderEditorValue($value) {
@@ -87,4 +77,33 @@ final class HeraldTokenizerFieldValue
     return $datasource->getWireTokens($value);
   }
 
+  public function renderTranscriptValue($value) {
+    return $this->renderValueAsList($value, $for_transcript = true);
+  }
+
+  private function renderValueAsList($value, $for_transcript) {
+    $viewer = $this->getViewer();
+    $value = (array)$value;
+
+    if (!$value) {
+      return phutil_tag('em', array(), pht('None'));
+    }
+
+    if ($this->valueMap !== null) {
+      foreach ($value as $k => $v) {
+        $value[$k] = idx($this->valueMap, $v, $v);
+      }
+
+      return implode(', ', $value);
+    }
+
+    $list = $viewer->renderHandleList($value);
+
+    if (!$for_transcript) {
+      $list->setAsInline(true);
+    }
+
+    return $list;
+  }
+
 }

From 41f143f7feeb2be0da6e7b35c75bcbd514f3c48c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Jan 2020 17:04:56 -0800
Subject: [PATCH 13/17] Respect repository identities when figuring out
 authors/committers in Herald pre-commit hook rules

Summary:
Ref T13480. Currently, Herald commit hook rules use a raw address resolution query to identify the author and committer for a commit. This will get the wrong answer when the raw identity string has been explicitly bound to some non-default user (most often, it will fail to identify an author when one exists).

Instead, use the "IdentityEngine" to properly resolve identities.

Test Plan: Authored a commit as `X <y@example.com>`, a raw identity with no "natural" matches to users (e.g., no user with that email or username). Bound the identity to a particular user in Diffusion. Wrote a Herald pre-commit content rule, pushed the commit. Saw Herald recognize the correct user when evaluating rules.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20953
---
 .../herald/HeraldPreCommitContentAdapter.php  | 38 +++++++++++++++++--
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
index cee5f97419..7e56074303 100644
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -8,6 +8,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
   private $revision = false;
 
   private $affectedPackages;
+  private $identityCache = array();
 
   public function getAdapterContentName() {
     return pht('Commit Hook: Commit Content');
@@ -166,10 +167,39 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
     }
   }
 
-  private function lookupUser($author) {
-    return id(new DiffusionResolveUserQuery())
-      ->withName($author)
-      ->execute();
+  private function lookupUser($raw_identity) {
+    // See T13480. After the move to repository identities, we want to look
+    // users up in the identity table. If you push a commit which is authored
+    // by "A Duck <duck@example.org>" and that identity is bound to user
+    // "@mallard" in the identity table, Herald should see the author of the
+    // commit as "@mallard" when evaluating pre-commit content rules.
+
+    if (!array_key_exists($raw_identity, $this->identityCache)) {
+      $repository = $this->getHookEngine()->getRepository();
+      $viewer = $this->getHookEngine()->getViewer();
+
+      $identity_engine = id(new DiffusionRepositoryIdentityEngine())
+        ->setViewer($viewer);
+
+      // We must provide a "sourcePHID" when resolving identities, but don't
+      // have a legitimate one yet. Just use the repository PHID as a
+      // reasonable value. This won't actually be written to storage.
+      $source_phid = $repository->getPHID();
+      $identity_engine->setSourcePHID($source_phid);
+
+      // If the identity doesn't exist yet, we don't want to create it if
+      // we haven't seen it before. It will be created later when we actually
+      // import the commit.
+      $identity_engine->setDryRun(true);
+
+      $author_identity = $identity_engine->newResolvedIdentity($raw_identity);
+
+      $effective_phid = $author_identity->getCurrentEffectiveUserPHID();
+
+      $this->identityCache[$raw_identity] = $effective_phid;
+    }
+
+    return $this->identityCache[$raw_identity];
   }
 
   private function getCommitFields() {

From 6628cd2b4f7b9f7c586ca9ad73d878718ac376ef Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 29 Jan 2020 10:22:48 -0800
Subject: [PATCH 14/17] In Herald "Commit" rules, use repository identities to
 identify authors and committers

Summary: Ref T13480. The Herald "Commit" rules still use raw commit data properties to identify authors and committers. Instead, use repository identities.

Test Plan: Wrote a Herald rule using all four fields, ran it against various commits with and without known authors. Checked transcript for sensible field values.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20955
---
 .../DiffusionCommitAuthorHeraldField.php      |  2 +-
 ...ffusionCommitAuthorProjectsHeraldField.php |  5 +-
 .../DiffusionCommitCommitterHeraldField.php   |  2 +-
 ...sionCommitCommitterProjectsHeraldField.php |  3 +-
 .../diffusion/herald/HeraldCommitAdapter.php  | 46 +++++++++++++------
 5 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/src/applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php
index ba1b636684..4c6fd74b31 100644
--- a/src/applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php
@@ -10,7 +10,7 @@ final class DiffusionCommitAuthorHeraldField
   }
 
   public function getHeraldFieldValue($object) {
-    return $object->getCommitData()->getCommitDetail('authorPHID');
+    return $this->getAdapter()->getAuthorPHID();
   }
 
   protected function getHeraldFieldStandardType() {
diff --git a/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
index a90480d50a..48afc5e1fe 100644
--- a/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
@@ -11,14 +11,13 @@ final class DiffusionCommitAuthorProjectsHeraldField
 
   public function getHeraldFieldValue($object) {
     $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
 
-    $phid = $object->getCommitData()->getCommitDetail('authorPHID');
+    $phid = $adapter->getAuthorPHID();
     if (!$phid) {
       return array();
     }
 
-    $viewer = $adapter->getViewer();
-
     $projects = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
       ->withMemberPHIDs(array($phid))
diff --git a/src/applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php
index 825648c46c..6f06cad07b 100644
--- a/src/applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php
@@ -10,7 +10,7 @@ final class DiffusionCommitCommitterHeraldField
   }
 
   public function getHeraldFieldValue($object) {
-    return $object->getCommitData()->getCommitDetail('committerPHID');
+    return $this->getAdapter()->getCommitterPHID();
   }
 
   protected function getHeraldFieldStandardType() {
diff --git a/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
index 97e604a481..8dc64cf110 100644
--- a/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
@@ -11,8 +11,9 @@ final class DiffusionCommitCommitterProjectsHeraldField
 
   public function getHeraldFieldValue($object) {
     $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
 
-    $phid = $object->getCommitData()->getCommitDetail('committerPHID');
+    $phid = $adapter->getCommitterPHID();
     if (!$phid) {
       return array();
     }
diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php
index 1755cb7701..d285d23f9b 100644
--- a/src/applications/diffusion/herald/HeraldCommitAdapter.php
+++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php
@@ -35,18 +35,6 @@ final class HeraldCommitAdapter
   }
 
   public function newTestAdapter(PhabricatorUser $viewer, $object) {
-    $object = id(new DiffusionCommitQuery())
-      ->setViewer($viewer)
-      ->withPHIDs(array($object->getPHID()))
-      ->needCommitData(true)
-      ->executeOne();
-    if (!$object) {
-      throw new Exception(
-        pht(
-          'Failed to reload commit ("%s") to fetch commit data.',
-          $object->getPHID()));
-    }
-
     return id(clone $this)
       ->setObject($object);
   }
@@ -56,7 +44,23 @@ final class HeraldCommitAdapter
   }
 
   public function setObject($object) {
-    $this->commit = $object;
+    $viewer = $this->getViewer();
+    $commit_phid = $object->getPHID();
+
+    $commit = id(new DiffusionCommitQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($commit_phid))
+      ->needCommitData(true)
+      ->needIdentities(true)
+      ->executeOne();
+    if (!$commit) {
+      throw new Exception(
+        pht(
+          'Failed to reload commit ("%s") to fetch commit data.',
+          $commit_phid));
+    }
+
+    $this->commit = $commit;
 
     return $this;
   }
@@ -352,6 +356,22 @@ final class HeraldCommitAdapter
     return $this->getObject()->getRepository();
   }
 
+  public function getAuthorPHID() {
+    return $this->getObject()->getEffectiveAuthorPHID();
+  }
+
+  public function getCommitterPHID() {
+    $commit = $this->getObject();
+
+    if ($commit->hasCommitterIdentity()) {
+      $identity = $commit->getCommitterIdentity();
+      return $identity->getCurrentEffectiveUserPHID();
+    }
+
+    return null;
+  }
+
+
 /* -(  HarbormasterBuildableAdapterInterface  )------------------------------ */
 
 

From c99485e8a00db37d5d449a0c12be43b7477ba8c2 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 29 Jan 2020 14:39:47 -0800
Subject: [PATCH 15/17] Add "Author's Packages" and "Committer's Packages"
 Herald rules for Commits and Hooks

Summary: Fixes T13480. Adds the remaining missing Owners package rules for Herald commit adapters.

Test Plan: Created hooks which care about these fields, pushed commits, saw sensible transcript values.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20957
---
 src/__phutil_library_map__.php                |  8 ++++
 ...ffusionCommitAuthorPackagesHeraldField.php | 37 +++++++++++++++++++
 ...ffusionCommitAuthorProjectsHeraldField.php |  6 +--
 ...sionCommitCommitterPackagesHeraldField.php | 37 +++++++++++++++++++
 ...sionCommitCommitterProjectsHeraldField.php |  8 ++--
 ...CommitContentAuthorPackagesHeraldField.php | 37 +++++++++++++++++++
 ...CommitContentAuthorProjectsHeraldField.php |  9 ++---
 ...mitContentCommitterPackagesHeraldField.php | 37 +++++++++++++++++++
 ...mitContentCommitterProjectsHeraldField.php |  9 ++---
 9 files changed, 170 insertions(+), 18 deletions(-)
 create mode 100644 src/applications/diffusion/herald/DiffusionCommitAuthorPackagesHeraldField.php
 create mode 100644 src/applications/diffusion/herald/DiffusionCommitCommitterPackagesHeraldField.php
 create mode 100644 src/applications/diffusion/herald/DiffusionPreCommitContentAuthorPackagesHeraldField.php
 create mode 100644 src/applications/diffusion/herald/DiffusionPreCommitContentCommitterPackagesHeraldField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index cf59036387..9b042e024a 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -737,12 +737,14 @@ phutil_register_library_map(array(
     'DiffusionCommitAuditorsHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuditorsHeraldField.php',
     'DiffusionCommitAuditorsTransaction' => 'applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php',
     'DiffusionCommitAuthorHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php',
+    'DiffusionCommitAuthorPackagesHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuthorPackagesHeraldField.php',
     'DiffusionCommitAuthorProjectsHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php',
     'DiffusionCommitAutocloseHeraldField' => 'applications/diffusion/herald/DiffusionCommitAutocloseHeraldField.php',
     'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php',
     'DiffusionCommitBranchesHeraldField' => 'applications/diffusion/herald/DiffusionCommitBranchesHeraldField.php',
     'DiffusionCommitBuildableTransaction' => 'applications/diffusion/xaction/DiffusionCommitBuildableTransaction.php',
     'DiffusionCommitCommitterHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php',
+    'DiffusionCommitCommitterPackagesHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterPackagesHeraldField.php',
     'DiffusionCommitCommitterProjectsHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php',
     'DiffusionCommitConcernTransaction' => 'applications/diffusion/xaction/DiffusionCommitConcernTransaction.php',
     'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php',
@@ -908,10 +910,12 @@ phutil_register_library_map(array(
     'DiffusionPhpExternalSymbolsSource' => 'applications/diffusion/symbol/DiffusionPhpExternalSymbolsSource.php',
     'DiffusionPreCommitContentAffectedFilesHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentAffectedFilesHeraldField.php',
     'DiffusionPreCommitContentAuthorHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentAuthorHeraldField.php',
+    'DiffusionPreCommitContentAuthorPackagesHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentAuthorPackagesHeraldField.php',
     'DiffusionPreCommitContentAuthorProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentAuthorProjectsHeraldField.php',
     'DiffusionPreCommitContentAuthorRawHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentAuthorRawHeraldField.php',
     'DiffusionPreCommitContentBranchesHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentBranchesHeraldField.php',
     'DiffusionPreCommitContentCommitterHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentCommitterHeraldField.php',
+    'DiffusionPreCommitContentCommitterPackagesHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentCommitterPackagesHeraldField.php',
     'DiffusionPreCommitContentCommitterProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentCommitterProjectsHeraldField.php',
     'DiffusionPreCommitContentCommitterRawHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentCommitterRawHeraldField.php',
     'DiffusionPreCommitContentDiffContentAddedHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentDiffContentAddedHeraldField.php',
@@ -6731,12 +6735,14 @@ phutil_register_library_map(array(
     'DiffusionCommitAuditorsHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitAuditorsTransaction' => 'DiffusionCommitTransactionType',
     'DiffusionCommitAuthorHeraldField' => 'DiffusionCommitHeraldField',
+    'DiffusionCommitAuthorPackagesHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitAuthorProjectsHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitAutocloseHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitBranchesController' => 'DiffusionController',
     'DiffusionCommitBranchesHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitBuildableTransaction' => 'DiffusionCommitTransactionType',
     'DiffusionCommitCommitterHeraldField' => 'DiffusionCommitHeraldField',
+    'DiffusionCommitCommitterPackagesHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitCommitterProjectsHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitConcernTransaction' => 'DiffusionCommitAuditTransaction',
     'DiffusionCommitController' => 'DiffusionController',
@@ -6905,10 +6911,12 @@ phutil_register_library_map(array(
     'DiffusionPhpExternalSymbolsSource' => 'DiffusionExternalSymbolsSource',
     'DiffusionPreCommitContentAffectedFilesHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentAuthorHeraldField' => 'DiffusionPreCommitContentHeraldField',
+    'DiffusionPreCommitContentAuthorPackagesHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentAuthorProjectsHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentAuthorRawHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentBranchesHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentCommitterHeraldField' => 'DiffusionPreCommitContentHeraldField',
+    'DiffusionPreCommitContentCommitterPackagesHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentCommitterProjectsHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentCommitterRawHeraldField' => 'DiffusionPreCommitContentHeraldField',
     'DiffusionPreCommitContentDiffContentAddedHeraldField' => 'DiffusionPreCommitContentHeraldField',
diff --git a/src/applications/diffusion/herald/DiffusionCommitAuthorPackagesHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitAuthorPackagesHeraldField.php
new file mode 100644
index 0000000000..ec4e89c716
--- /dev/null
+++ b/src/applications/diffusion/herald/DiffusionCommitAuthorPackagesHeraldField.php
@@ -0,0 +1,37 @@
+<?php
+
+final class DiffusionCommitAuthorPackagesHeraldField
+  extends DiffusionCommitHeraldField {
+
+  const FIELDCONST = 'diffusion.commit.author.packages';
+
+  public function getHeraldFieldName() {
+    return pht("Author's packages");
+  }
+
+  public function getHeraldFieldValue($object) {
+    $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
+
+    $author_phid = $adapter->getAuthorPHID();
+    if (!$author_phid) {
+      return array();
+    }
+
+    $packages = id(new PhabricatorOwnersPackageQuery())
+      ->setViewer($viewer)
+      ->withAuthorityPHIDs(array($author_phid))
+      ->execute();
+
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorOwnersPackageDatasource();
+  }
+
+}
diff --git a/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
index 48afc5e1fe..d61423edbc 100644
--- a/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitAuthorProjectsHeraldField.php
@@ -13,14 +13,14 @@ final class DiffusionCommitAuthorProjectsHeraldField
     $adapter = $this->getAdapter();
     $viewer = $adapter->getViewer();
 
-    $phid = $adapter->getAuthorPHID();
-    if (!$phid) {
+    $author_phid = $adapter->getAuthorPHID();
+    if (!$author_phid) {
       return array();
     }
 
     $projects = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
-      ->withMemberPHIDs(array($phid))
+      ->withMemberPHIDs(array($author_phid))
       ->execute();
 
     return mpull($projects, 'getPHID');
diff --git a/src/applications/diffusion/herald/DiffusionCommitCommitterPackagesHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitCommitterPackagesHeraldField.php
new file mode 100644
index 0000000000..5ef5824b63
--- /dev/null
+++ b/src/applications/diffusion/herald/DiffusionCommitCommitterPackagesHeraldField.php
@@ -0,0 +1,37 @@
+<?php
+
+final class DiffusionCommitCommitterPackagesHeraldField
+  extends DiffusionCommitHeraldField {
+
+  const FIELDCONST = 'diffusion.commit.committer.packages';
+
+  public function getHeraldFieldName() {
+    return pht("Committer's packages");
+  }
+
+  public function getHeraldFieldValue($object) {
+    $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
+
+    $committer_phid = $adapter->getAuthorPHID();
+    if (!$committer_phid) {
+      return array();
+    }
+
+    $packages = id(new PhabricatorOwnersPackageQuery())
+      ->setViewer($viewer)
+      ->withAuthorityPHIDs(array($committer_phid))
+      ->execute();
+
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorOwnersPackageDatasource();
+  }
+
+}
diff --git a/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
index 8dc64cf110..5f16a29f82 100644
--- a/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php
@@ -13,16 +13,14 @@ final class DiffusionCommitCommitterProjectsHeraldField
     $adapter = $this->getAdapter();
     $viewer = $adapter->getViewer();
 
-    $phid = $adapter->getCommitterPHID();
-    if (!$phid) {
+    $committer_phid = $adapter->getCommitterPHID();
+    if (!$committer_phid) {
       return array();
     }
 
-    $viewer = $adapter->getViewer();
-
     $projects = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
-      ->withMemberPHIDs(array($phid))
+      ->withMemberPHIDs(array($committer_phid))
       ->execute();
 
     return mpull($projects, 'getPHID');
diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorPackagesHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorPackagesHeraldField.php
new file mode 100644
index 0000000000..9832958faf
--- /dev/null
+++ b/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorPackagesHeraldField.php
@@ -0,0 +1,37 @@
+<?php
+
+final class DiffusionPreCommitContentAuthorPackagesHeraldField
+  extends DiffusionPreCommitContentHeraldField {
+
+  const FIELDCONST = 'diffusion.pre.commit.author.packages';
+
+  public function getHeraldFieldName() {
+    return pht("Author's packages");
+  }
+
+  public function getHeraldFieldValue($object) {
+    $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
+
+    $author_phid = $adapter->getAuthorPHID();
+    if (!$author_phid) {
+      return array();
+    }
+
+    $packages = id(new PhabricatorOwnersPackageQuery())
+      ->setViewer($viewer)
+      ->withAuthorityPHIDs(array($author_phid))
+      ->execute();
+
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorOwnersPackageDatasource();
+  }
+
+}
diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorProjectsHeraldField.php
index ec138c9bde..2d0268deba 100644
--- a/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionPreCommitContentAuthorProjectsHeraldField.php
@@ -11,17 +11,16 @@ final class DiffusionPreCommitContentAuthorProjectsHeraldField
 
   public function getHeraldFieldValue($object) {
     $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
 
-    $phid = $adapter->getAuthorPHID();
-    if (!$phid) {
+    $author_phid = $adapter->getAuthorPHID();
+    if (!$author_phid) {
       return array();
     }
 
-    $viewer = $adapter->getViewer();
-
     $projects = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
-      ->withMemberPHIDs(array($phid))
+      ->withMemberPHIDs(array($author_phid))
       ->execute();
 
     return mpull($projects, 'getPHID');
diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterPackagesHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterPackagesHeraldField.php
new file mode 100644
index 0000000000..cd4674ef07
--- /dev/null
+++ b/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterPackagesHeraldField.php
@@ -0,0 +1,37 @@
+<?php
+
+final class DiffusionPreCommitContentCommitterPackagesHeraldField
+  extends DiffusionPreCommitContentHeraldField {
+
+  const FIELDCONST = 'diffusion.pre.commit.committer.packages';
+
+  public function getHeraldFieldName() {
+    return pht("Committer's packages");
+  }
+
+  public function getHeraldFieldValue($object) {
+    $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
+
+    $committer_phid = $adapter->getCommitterPHID();
+    if (!$committer_phid) {
+      return array();
+    }
+
+    $packages = id(new PhabricatorOwnersPackageQuery())
+      ->setViewer($viewer)
+      ->withAuthorityPHIDs(array($committer_phid))
+      ->execute();
+
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new PhabricatorOwnersPackageDatasource();
+  }
+
+}
diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterProjectsHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterProjectsHeraldField.php
index 9b0f012f3c..109649241f 100644
--- a/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterProjectsHeraldField.php
+++ b/src/applications/diffusion/herald/DiffusionPreCommitContentCommitterProjectsHeraldField.php
@@ -11,17 +11,16 @@ final class DiffusionPreCommitContentCommitterProjectsHeraldField
 
   public function getHeraldFieldValue($object) {
     $adapter = $this->getAdapter();
+    $viewer = $adapter->getViewer();
 
-    $phid = $adapter->getCommitterPHID();
-    if (!$phid) {
+    $committer_phid = $adapter->getCommitterPHID();
+    if (!$committer_phid) {
       return array();
     }
 
-    $viewer = $adapter->getViewer();
-
     $projects = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
-      ->withMemberPHIDs(array($phid))
+      ->withMemberPHIDs(array($committer_phid))
       ->execute();
 
     return mpull($projects, 'getPHID');

From 12c337098872b6af75ed7591d5a2611f860331d0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Jan 2020 07:26:20 -0800
Subject: [PATCH 16/17] When issuing a "no-op" MFA token because no MFA is
 configured, don't give the timeline story a badge

Summary:
Fixes T13475. Sometimes, we issue a "no op" / "default permit" / "unchallenged" MFA token, when a user with no MFA configured does something which is configured to attempt (but not strictly require) MFA.

An example of this kind of action is changing a username: usernames may be changed even if MFA is not set up.

(Some other operations, notably "Sign With MFA", strictly require that MFA actually be set up.)

When a user with no MFA configured takes a "try MFA" action, we see that they have no factors configured and issue a token so they can continue. This is correct. However, this token causes the assocaited timeline story to get an MFA badge.

This badge is incorrect or at least wildly misleading, since the technical assertion it currently makes ("the user answered any configured MFA challenge to do this, if one exists") isn't explained properly and isn't useful anyway.

Instead, only badge the story if the user actually has MFA and actually responded to some kind of MFA challege. The badge now asserts "this user responded to an MFA challenge", which is expected/desired.

Test Plan:
  - As a user with no MFA, renamed a user. Before patch: badged story. After patch: no badge.
  - As a user with MFA, renamed a user. Got badged stories in both cases.

Maniphest Tasks: T13475

Differential Revision: https://secure.phabricator.com/D20958
---
 .../data/PhabricatorAuthHighSecurityToken.php    | 16 +++++++++++++++-
 .../auth/engine/PhabricatorAuthSessionEngine.php |  3 ++-
 .../PhabricatorApplicationTransactionEditor.php  |  8 +++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php b/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php
index 8ea1ed97f8..9d44411795 100644
--- a/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php
+++ b/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php
@@ -1,3 +1,17 @@
 <?php
 
-final class PhabricatorAuthHighSecurityToken extends Phobject {}
+final class PhabricatorAuthHighSecurityToken
+  extends Phobject {
+
+  private $isUnchallengedToken = false;
+
+  public function setIsUnchallengedToken($is_unchallenged_token) {
+    $this->isUnchallengedToken = $is_unchallenged_token;
+    return $this;
+  }
+
+  public function getIsUnchallengedToken() {
+    return $this->isUnchallengedToken;
+  }
+
+}
diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
index 7358a61a40..251c8284ef 100644
--- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
+++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@@ -493,7 +493,8 @@ final class PhabricatorAuthSessionEngine extends Phobject {
     // adds an auth factor, existing sessions won't get a free pass into hisec,
     // since they never actually got marked as hisec.
     if (!$factors) {
-      return $this->issueHighSecurityToken($session, true);
+      return $this->issueHighSecurityToken($session, true)
+        ->setIsUnchallengedToken(true);
     }
 
     $this->request = $request;
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index c17ac1ec72..422605f761 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -5152,12 +5152,14 @@ abstract class PhabricatorApplicationTransactionEditor
           'an MFA check.'));
     }
 
-    id(new PhabricatorAuthSessionEngine())
+    $token = id(new PhabricatorAuthSessionEngine())
       ->setWorkflowKey($workflow_key)
       ->requireHighSecurityToken($actor, $request, $cancel_uri);
 
-    foreach ($xactions as $xaction) {
-      $xaction->setIsMFATransaction(true);
+    if (!$token->getIsUnchallengedToken()) {
+      foreach ($xactions as $xaction) {
+        $xaction->setIsMFATransaction(true);
+      }
     }
   }
 

From ccf28a81121ea21d72b342cfb8ab2eee4e56bf86 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Jan 2020 08:14:10 -0800
Subject: [PATCH 17/17] Fix an issue where the last line of block-based diffs
 could be incorrectly hidden

Summary:
Fixes T13468. See that task for discussion. The older source-rendering code mixes "line number" / "1-based" lists with "block number" / "0-based" lists and then has other bugs which cancel this out.

For block-based diffs, build an explicit block-based mask with only block numbers. This sort of sidesteps the whole issue.

Test Plan: Viewed the diff with the original reproduction case, plus various other block-based diffs, including one-block image diffs, in unified and side-by-side mode. Didn't spot any oddities.

Maniphest Tasks: T13468

Differential Revision: https://secure.phabricator.com/D20959
---
 .../parser/DifferentialHunkParser.php         | 64 +++++++++++++++++++
 .../diff/PhabricatorDocumentEngineBlocks.php  | 11 +---
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php
index 6cdadf69e7..924789ca83 100644
--- a/src/applications/differential/parser/DifferentialHunkParser.php
+++ b/src/applications/differential/parser/DifferentialHunkParser.php
@@ -354,6 +354,69 @@ final class DifferentialHunkParser extends Phobject {
     return $this;
   }
 
+  public function generateVisibleBlocksMask($lines_context) {
+
+    // See T13468. This is similar to "generateVisibleLinesMask()", but
+    // attempts to work around a series of bugs which cancel each other
+    // out but make a mess of the intermediate steps.
+
+    $old = $this->getOldLines();
+    $new = $this->getNewLines();
+
+    $length = max(count($old), count($new));
+
+    $visible_lines = array();
+    for ($ii = 0; $ii < $length; $ii++) {
+      $old_visible = (isset($old[$ii]) && $old[$ii]['type']);
+      $new_visible = (isset($new[$ii]) && $new[$ii]['type']);
+
+      $visible_lines[$ii] = ($old_visible || $new_visible);
+    }
+
+    $mask = array();
+    $reveal_cursor = -1;
+    for ($ii = 0; $ii < $length; $ii++) {
+
+      // If this line isn't visible, it isn't going to reveal anything.
+      if (!$visible_lines[$ii]) {
+
+        // If it hasn't been revealed by a nearby line, mark it as masked.
+        if (empty($mask[$ii])) {
+          $mask[$ii] = false;
+        }
+
+        continue;
+      }
+
+      // If this line is visible, reveal all the lines nearby.
+
+      // First, compute the minimum and maximum offsets we want to reveal.
+      $min_reveal = max($ii - $lines_context, 0);
+      $max_reveal = min($ii + $lines_context, $length - 1);
+
+      // Naively, we'd do more work than necessary when revealing context for
+      // several adjacent visible lines: we would mark all the overlapping
+      // lines as revealed several times.
+
+      // To avoid duplicating work, keep track of the largest line we've
+      // revealed to. Since we reveal context by marking every consecutive
+      // line, we don't need to touch any line above it.
+      $min_reveal = max($min_reveal, $reveal_cursor);
+
+      // Reveal the remaining unrevealed lines.
+      for ($jj = $min_reveal; $jj <= $max_reveal; $jj++) {
+        $mask[$jj] = true;
+      }
+
+      // Move the cursor to the next line which may still need to be revealed.
+      $reveal_cursor = $max_reveal + 1;
+    }
+
+    $this->setVisibleLinesMask($mask);
+
+    return $mask;
+  }
+
   public function generateVisibleLinesMask($lines_context) {
     $old = $this->getOldLines();
     $new = $this->getNewLines();
@@ -361,6 +424,7 @@ final class DifferentialHunkParser extends Phobject {
     $visible = false;
     $last = 0;
     $mask = array();
+
     for ($cursor = -$lines_context; $cursor < $max_length; $cursor++) {
       $offset = $cursor + $lines_context;
       if ((isset($old[$offset]) && $old[$offset]['type']) ||
diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php
index 47ddf194ee..2847b53c0d 100644
--- a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php
+++ b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php
@@ -59,7 +59,7 @@ final class PhabricatorDocumentEngineBlocks
       ->parseHunksForLineData($changeset->getHunks())
       ->reparseHunksForSpecialAttributes();
 
-    $hunk_parser->generateVisibleLinesMask(2);
+    $hunk_parser->generateVisibleBlocksMask(2);
     $mask = $hunk_parser->getVisibleLinesMask();
 
     $old_lines = $hunk_parser->getOldLines();
@@ -72,14 +72,7 @@ final class PhabricatorDocumentEngineBlocks
       $old_line = idx($old_lines, $ii);
       $new_line = idx($new_lines, $ii);
 
-      $is_visible = !empty($mask[$ii + 1]);
-
-      // TODO: There's currently a bug where one-line files get incorrectly
-      // masked. This causes images to completely fail to render. Just ignore
-      // the mask if it came back empty.
-      if (!$mask) {
-        $is_visible = true;
-      }
+      $is_visible = !empty($mask[$ii]);
 
       if ($old_line) {
         $old_hash = rtrim($old_line['text'], "\n");