Muck around with Diviner method documentation display

Summary:
Ref T988. Not sure about this, feel free to push back or tweak it or whatever, but I want to reduce the amount of meta-text in the method documentation. Primarily this:

  - Shortens "From parent implementation in ClassName:" to "ClassName".
  - Tries to tweak the styles a bit so that it's relatively obvious what that means (hopefully?).
  - Fixes an issue with tasks where some methods could be ignored.

Test Plan: {F57565}

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D6911

src/__celerity_resource_map__.php

@@ -1149,7 +1149,7 @@ celerity_register_resource_map(array(
   ),
   'diviner-shared-css' =>
   array(
-    'uri' => '/res/686727d1/rsrc/css/diviner/diviner-shared.css',
+    'uri' => '/res/4237d999/rsrc/css/diviner/diviner-shared.css',
     'type' => 'css',
     'requires' =>
     array(

src/applications/diviner/controller/DivinerAtomController.php

@@ -134,16 +134,20 @@ final class DivinerAtomController extends DivinerController {
       if ($tasks) {
         $methods_by_task = igroup($methods, 'task');
 
-        $section = id(new DivinerSectionView())
-          ->setHeader(pht('Tasks'));
-
-        if (isset($methods_by_task[''])) {
-          $tasks[''] = array(
-            'name' => '',
-            'title' => pht('Other Methods'),
+        // Add phantom tasks for methods which have a "@task" name that isn't
+        // documented anywhere, or methods that have no "@task" name.
+        foreach ($methods_by_task as $task => $ignored) {
+          if (empty($tasks[$task])) {
+            $tasks[$task] = array(
+              'name' => $task,
+              'title' => $task ? $task : pht('Other Methods'),
               'defined' => $symbol,
             );
           }
+        }
+
+        $section = id(new DivinerSectionView())
+          ->setHeader(pht('Tasks'));
 
         foreach ($tasks as $spec) {
           $section->addContent(
@@ -169,7 +173,7 @@ final class DivinerAtomController extends DivinerController {
                 $item = array(
                   $item,
                   " \xE2\x80\x94 ",
-                  phutil_safe_html($atom->getSummary()));
+                  $atom->getSummary());
               }
 
               $list_items[] = phutil_tag('li', array(), $item);
@@ -429,7 +433,15 @@ final class DivinerAtomController extends DivinerController {
       }
     }
 
-    return $task_specs + $extends_task_specs;
+    $specs = $task_specs + $extends_task_specs;
+
+    // Reorder "@tasks" in original declaration order. Basically, we want to
+    // use the documentation of the closest subclass, but put tasks which
+    // were declared by parents first.
+    $keys = array_keys($extends_task_specs);
+    $specs = array_select_keys($specs, $keys) + $specs;
+
+    return $specs;
   }
 
   private function renderFullSignature(
@@ -616,21 +628,29 @@ final class DivinerAtomController extends DivinerController {
         if (!strlen(trim($symbol->getMarkupText($field)))) {
           continue;
         }
-        $out[] = id(new PHUIBoxView())
-          ->addPadding(PHUI::PADDING_LARGE_LEFT)
-          ->addPadding(PHUI::PADDING_LARGE_RIGHT)
-          ->addClass('diviner-method-implementation-header')
-          ->appendChild(
-            pht('From parent implementation in %s:', $impl->getName()));
-      } else if ($out) {
-        $out[] = id(new PHUIBoxView())
-          ->addPadding(PHUI::PADDING_LARGE_LEFT)
-          ->addPadding(PHUI::PADDING_LARGE_RIGHT)
-          ->addClass('diviner-method-implementation-header')
-          ->appendChild(
-            pht('From this implementation:'));
       }
-      $out[] = $this->renderDocumentationText($symbol, $engine);
+
+      $doc = $this->renderDocumentationText($symbol, $engine);
+
+      if (($impl !== $parent) || $out) {
+        $where = id(new PHUIBoxView())
+          ->addPadding(PHUI::PADDING_MEDIUM_LEFT)
+          ->addPadding(PHUI::PADDING_MEDIUM_RIGHT)
+          ->addClass('diviner-method-implementation-header')
+          ->appendChild($impl->getName());
+        $doc = array($where, $doc);
+
+        if ($impl !== $parent) {
+          $doc = phutil_tag(
+            'div',
+            array(
+              'class' => 'diviner-method-implementation-inherited',
+            ),
+            $doc);
+        }
+      }
+
+      $out[] = $doc;
     }
 
     // If we only have inherited implementations but none have documentation,

src/applications/files/engine/PhabricatorFileStorageEngine.php

@@ -9,12 +9,18 @@
  *
  * For more information, see @{article:File Storage Technical Documentation}.
  *
+ * @task construct Constructing an Engine
  * @task meta Engine Metadata
  * @task file Managing File Data
  * @group filestorage
  */
 abstract class PhabricatorFileStorageEngine {
 
+  /**
+   * Construct a new storage engine.
+   *
+   * @task construct
+   */
   final public function __construct() {
     // <empty>
   }

src/applications/files/engine/PhabricatorS3FileStorageEngine.php

@@ -4,20 +4,17 @@
  * Amazon S3 file storage engine. This engine scales well but is relatively
  * high-latency since data has to be pulled off S3.
  *
- * @task impl       Implementation
  * @task internal Internals
- * @group filestorage
  */
 final class PhabricatorS3FileStorageEngine
   extends PhabricatorFileStorageEngine {
 
+
 /* -(  Implementation  )----------------------------------------------------- */
 
 
   /**
-   * This engine identifies as "amazon-s3".
-   *
-   * @task impl
+   * This engine identifies as `amazon-s3`.
    */
   public function getEngineIdentifier() {
     return 'amazon-s3';
@@ -25,8 +22,7 @@ final class PhabricatorS3FileStorageEngine
 
 
   /**
-   * Write file data into S3.
-   * @task impl
+   * Writes file data into Amazon S3.
    */
   public function writeFile($data, array $params) {
     $s3 = $this->newS3API();
@@ -55,8 +51,7 @@ final class PhabricatorS3FileStorageEngine
 
 
   /**
-   * Load a stored blob from S3.
-   * @task impl
+   * Load a stored blob from Amazon S3.
    */
   public function readFile($handle) {
     $result = $this->newS3API()->getObject(
@@ -74,8 +69,7 @@ final class PhabricatorS3FileStorageEngine
 
 
   /**
-   * Delete a blob from S3.
-   * @task impl
+   * Delete a blob from Amazon S3.
    */
   public function deleteFile($handle) {
     AphrontWriteGuard::willWrite();

webroot/rsrc/css/diviner/diviner-shared.css

@@ -71,10 +71,6 @@
   margin: 16px;
 }
 
-.diviner-method-implementation-header {
-  color: {$lightgreytext};
-}
-
 .diviner-atom-signature {
   font-weight: normal;
 }
@@ -90,3 +86,24 @@
 .diviner-list a {
   font-weight: bold;
 }
+
+.diviner-method-implementation-header {
+  color: {$greytext};
+  margin-bottom: -8px;
+}
+
+.diviner-method-implementation-inherited {
+  color: {$darkgreytext};
+}
+
+.diviner-method-implementation-inherited .diviner-method-implementation-header {
+  color: {$lightgreytext};
+}
+
+/**
+ * Fix excessive padding between method headers and method documentation for
+ * methods with no inherited context.
+ */
+.diviner-document-section .phabricator-header-shell + .phabricator-remarkup {
+  padding-top: 0;
+}