Split paste create/edit and list views

Summary: We have this hybrid "create / last few pastes" landing screen right now but I ~never use the list at the bottom and it makes the controller kind of complicated. I want to let you edit pastes too, and this generally simplifies things. Also makes the textarea monospaced and cleans up the fork logic a bit. Test Plan: Created, forked pastes. Viewed paste lists. Viewed pastes. Reviewers: vrana, btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1690 Differential Revision: https://secure.phabricator.com/D3375
2024-11-22 14:52:41 +01:00 · 2012-08-24 13:19:14 -07:00 · 2012-08-24 13:19:14 -07:00 · fed30dfb4c
commit fed30dfb4c
parent d9a133d716
10 changed files with 194 additions and 249 deletions
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@ -1196,7 +1196,6 @@ return array(
    'erb' => 'Embedded Ruby/ERB',
    'erlang' => 'Erlang',
    'html' => 'HTML',
-    'infer' => 'Infer from title (extension)',
    'java' => 'Java',
    'js' => 'Javascript',
    'mysql' => 'MySQL',
@ -1210,8 +1209,6 @@ return array(
    'xml' => 'XML',
  ),

-  'pygments.dropdown-default' => 'infer',
-
  // This is an override list of regular expressions which allows you to choose
  // what language files are highlighted as. If your projects have certain rules
  // about filenames or use unusual or ambiguous language extensions, you can
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -892,6 +892,7 @@ phutil_register_library_map(array(
    'PhabricatorPaste' => 'applications/paste/storage/PhabricatorPaste.php',
    'PhabricatorPasteController' => 'applications/paste/controller/PhabricatorPasteController.php',
    'PhabricatorPasteDAO' => 'applications/paste/storage/PhabricatorPasteDAO.php',
+    'PhabricatorPasteEditController' => 'applications/paste/controller/PhabricatorPasteEditController.php',
    'PhabricatorPasteListController' => 'applications/paste/controller/PhabricatorPasteListController.php',
    'PhabricatorPasteQuery' => 'applications/paste/query/PhabricatorPasteQuery.php',
    'PhabricatorPasteViewController' => 'applications/paste/controller/PhabricatorPasteViewController.php',
@ -1992,6 +1993,7 @@ phutil_register_library_map(array(
    ),
    'PhabricatorPasteController' => 'PhabricatorController',
    'PhabricatorPasteDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorPasteEditController' => 'PhabricatorPasteController',
    'PhabricatorPasteListController' => 'PhabricatorPasteController',
    'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyQuery',
    'PhabricatorPasteViewController' => 'PhabricatorPasteController',
--- a/src/applications/paste/application/PhabricatorApplicationPaste.php
+++ b/src/applications/paste/application/PhabricatorApplicationPaste.php
@ -34,7 +34,7 @@ final class PhabricatorApplicationPaste extends PhabricatorApplication {
    return array(
      '/P(?P<id>\d+)' => 'PhabricatorPasteViewController',
      '/paste/' => array(
-        '' => 'PhabricatorPasteListController',
+        '' => 'PhabricatorPasteEditController',
        'filter/(?P<filter>\w+)/' => 'PhabricatorPasteListController',
      ),
    );
--- a/src/applications/paste/controller/PhabricatorPasteController.php
+++ b/src/applications/paste/controller/PhabricatorPasteController.php
@ -28,7 +28,7 @@ abstract class PhabricatorPasteController extends PhabricatorController {
    }

    $nav->addLabel('Create');
-    $nav->addFilter('create', 'New Paste');
+    $nav->addFilter('edit', 'New Paste', $this->getApplicationURI());

    $nav->addSpacer();
    $nav->addLabel('Pastes');
--- a/src/applications/paste/controller/PhabricatorPasteEditController.php
+++ b/src/applications/paste/controller/PhabricatorPasteEditController.php
@ -0,0 +1,154 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorPasteEditController extends PhabricatorPasteController {
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $paste = new PhabricatorPaste();
+    $title = 'Create Paste';
+
+    $parent_id = $request->getStr('parent');
+    $parent = null;
+    if ($parent_id) {
+      // NOTE: If the Paste is forked from a paste which the user no longer
+      // has permission to see, we still let them edit it.
+      $parent = id(new PhabricatorPasteQuery())
+        ->setViewer($user)
+        ->withIDs(array($parent_id))
+        ->execute();
+      $parent = head($parent);
+
+      if ($parent) {
+        $paste->setParentPHID($parent->getPHID());
+      }
+    }
+
+    $text = null;
+    $e_text = true;
+    $errors = array();
+    if ($request->isFormPost()) {
+      $text = $request->getStr('text');
+      if (!strlen($text)) {
+        $e_text = 'Required';
+        $errors[] = 'The paste may not be blank.';
+      } else {
+        $e_text = null;
+      }
+
+      $paste->setTitle($request->getStr('title'));
+      $paste->setLanguage($request->getStr('language'));
+
+      if (!$errors) {
+        $paste_file = PhabricatorFile::newFromFileData(
+          $text,
+          array(
+            'name' => $title,
+            'mime-type' => 'text/plain; charset=utf-8',
+            'authorPHID' => $user->getPHID(),
+          ));
+        $paste->setFilePHID($paste_file->getPHID());
+        $paste->setAuthorPHID($user->getPHID());
+        $paste->save();
+
+        return id(new AphrontRedirectResponse())->setURI($paste->getURI());
+      }
+    } else {
+      if ($parent) {
+        $paste->setTitle('Fork of '.$parent->getFullName());
+        $paste->setLanguage($parent->getLanguage());
+
+        $parent_file = id(new PhabricatorFile())->loadOneWhere(
+          'phid = %s',
+          $parent->getFilePHID());
+        $text = $parent_file->loadFileData();
+      }
+    }
+
+    $error_view = null;
+    if ($errors) {
+      $error_view = id(new AphrontErrorView())
+        ->setTitle('A fatal omission!')
+        ->setErrors($errors);
+    }
+
+    $form = new AphrontFormView();
+    $form->setFlexible(true);
+
+    $langs = array(
+      '' => '(Detect With Wizardly Powers)',
+    ) + PhabricatorEnv::getEnvConfig('pygments.dropdown-choices');
+
+    $submit = id(new AphrontFormSubmitControl())
+      ->setValue('Create Paste');
+
+    $form
+      ->setUser($user)
+      ->addHiddenInput('parent', $parent_id)
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel('Title')
+          ->setValue($paste->getTitle())
+          ->setName('title'))
+      ->appendChild(
+        id(new AphrontFormSelectControl())
+          ->setLabel('Language')
+          ->setName('language')
+          ->setValue($paste->getLanguage())
+          ->setOptions($langs))
+      ->appendChild(
+        id(new AphrontFormTextAreaControl())
+          ->setLabel('Text')
+          ->setError($e_text)
+          ->setValue($text)
+          ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
+          ->setCustomClass('PhabricatorMonospaced')
+          ->setName('text'))
+
+    /* TODO: Doesn't have any useful options yet.
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setLabel('Visible To')
+          ->setUser($user)
+          ->setValue(
+            $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW))
+          ->setName('policy'))
+    */
+
+      ->appendChild($submit);
+
+    $nav = $this->buildSideNavView();
+    $nav->selectFilter('edit');
+    $nav->appendChild(
+      array(
+        id(new PhabricatorHeaderView())->setHeader('Create Paste'),
+        $error_view,
+        $form,
+      ));
+
+    return $this->buildApplicationPage(
+      $nav,
+      array(
+        'title' => $title,
+        'device' => true,
+      ));
+  }
+
+}
--- a/src/applications/paste/controller/PhabricatorPasteListController.php
+++ b/src/applications/paste/controller/PhabricatorPasteListController.php
@ -17,282 +17,60 @@
 */

 final class PhabricatorPasteListController extends PhabricatorPasteController {
+
  private $filter;

-  private $errorView;
-  private $errorText;
-  private $paste;
-  private $pasteText;
-
-  private function setFilter($filter) {
-    $this->filter = $filter;
-    return $this;
-  }
-  private function getFilter() {
-    return $this->filter;
-  }
-
-  private function setErrorView($error_view) {
-    $this->errorView = $error_view;
-    return $this;
-  }
-
-  private function getErrorView() {
-    return $this->errorView;
-  }
-
-  private function setErrorText($error_text) {
-    $this->errorText = $error_text;
-    return $this;
-  }
-  private function getErrorText() {
-    return $this->errorText;
-  }
-
-  private function setPaste(PhabricatorPaste $paste) {
-    $this->paste = $paste;
-    return $this;
-  }
-  private function getPaste() {
-    return $this->paste;
-  }
-
-  private function setPasteText($paste_text) {
-    $this->pasteText = $paste_text;
-    return $this;
-  }
-  private function getPasteText() {
-    return $this->pasteText;
-  }
-
  public function willProcessRequest(array $data) {
-    $this->setFilter(idx($data, 'filter', 'create'));
+    $this->filter = idx($data, 'filter');
  }

  public function processRequest() {
    $request = $this->getRequest();
    $user = $request->getUser();

-    $pager = new AphrontCursorPagerView();
-    $pager->readFromRequest($request);
-
    $query = new PhabricatorPasteQuery();
    $query->setViewer($user);

-    switch ($this->getFilter()) {
-      case 'create':
-      default:
-        // if we successfully create a paste, we redirect to view it
-        $created_paste_redirect = $this->processCreateRequest();
-        if ($created_paste_redirect) {
-          return $created_paste_redirect;
-        }
+    $nav = $this->buildSideNavView();
+    $filter = $nav->selectFilter($this->filter, 'my');

-        $query->setLimit(10);
-        $paste_list = $query->execute();
-
-        $pager = null;
-        break;
+    switch ($filter) {
      case 'my':
        $query->withAuthorPHIDs(array($user->getPHID()));
-        $paste_list = $query->executeWithCursorPager($pager);
+        $title = 'My Pastes';
        break;
      case 'all':
-        $paste_list = $query->executeWithCursorPager($pager);
+        $title = 'All Pastes';
        break;
    }

-    $side_nav = $this->buildSideNavView();
-    $side_nav->selectFilter($this->getFilter());
+    $pager = new AphrontCursorPagerView();
+    $pager->readFromRequest($request);
+    $pastes = $query->executeWithCursorPager($pager);

-    if ($this->getErrorView()) {
-      $side_nav->appendChild($this->getErrorView());
-    }

-    switch ($this->getFilter()) {
-      case 'create':
-      default:
-        $side_nav->appendChild($this->renderCreatePaste());
-        $see_all = phutil_render_tag(
-          'a',
-          array(
-            'href' => '/paste/filter/all',
-          ),
-          'See all Pastes');
-        $header = "Recent Pastes";
-        break;
-      case 'my':
-        $header = 'Your Pastes';
-        break;
-      case 'all':
-        $header = 'All Pastes';
-        break;
-    }
-
-    $this->loadHandles(mpull($paste_list, 'getAuthorPHID'));
-
-    $list = $this->buildPasteList($paste_list);
-    $list->setHeader($header);
+    $list = $this->buildPasteList($pastes);
+    $list->setHeader($title);
    $list->setPager($pager);

-    $side_nav->appendChild($list);
+    $nav->appendChild($list);

    return $this->buildApplicationPage(
-      $side_nav,
+      $nav,
      array(
-        'title' => 'Paste',
+        'title' => $title,
        'device' => true,
      )
    );
  }

-  private function processCreateRequest() {
-    $request = $this->getRequest();
-    $user = $request->getUser();
-    $fork = $request->getInt('fork');
-
-    $error_view = null;
-    $e_text = true;
-    $new_paste = new PhabricatorPaste();
-    $new_paste_text = null;
-    $new_paste_language = PhabricatorEnv::getEnvConfig(
-      'pygments.dropdown-default');
-
-    if ($request->isFormPost()) {
-      $errors = array();
-
-      $text = $request->getStr('text');
-      if (!strlen($text)) {
-        $e_text = 'Required';
-        $errors[] = 'The paste may not be blank.';
-      } else {
-        $e_text = null;
-      }
-
-      $parent_phid = $request->getStr('parent');
-      if ($parent_phid) {
-        $parent = id(new PhabricatorPaste())->loadOneWhere('phid = %s',
-                                                           $parent_phid);
-        if ($parent) {
-          $new_paste->setParentPHID($parent->getPHID());
-        }
-      }
-
-      $title = $request->getStr('title');
-      $new_paste->setTitle($title);
-
-      $new_paste_language = $request->getStr('language');
-
-      if (!$errors) {
-        if ($new_paste_language == 'infer') {
-          // If it's infer, store an empty string. Otherwise, store the
-          // language name. We do this so we can refer to 'infer' elsewhere
-          // in the code (such as default value) while retaining backwards
-          // compatibility with old posts with no language stored.
-          $new_paste_language = '';
-        }
-        $new_paste->setLanguage($new_paste_language);
-
-        $new_paste_file = PhabricatorFile::newFromFileData(
-          $text,
-          array(
-            'name' => $title,
-            'mime-type' => 'text/plain; charset=utf-8',
-            'authorPHID' => $user->getPHID(),
-          ));
-        $new_paste->setFilePHID($new_paste_file->getPHID());
-        $new_paste->setAuthorPHID($user->getPHID());
-        $new_paste->save();
-
-        return id(new AphrontRedirectResponse())
-          ->setURI('/P'.$new_paste->getID());
-      } else {
-        $error_view = new AphrontErrorView();
-        $error_view->setErrors($errors);
-        $error_view->setTitle('A problem has occurred!');
-      }
-    } else if ($fork) {
-      $fork_paste = id(new PhabricatorPaste())->load($fork);
-      if ($fork_paste) {
-        $new_paste->setTitle('Fork of '.$fork_paste->getID().': '.
-                             $fork_paste->getTitle());
-        $fork_file = id(new PhabricatorFile())->loadOneWhere(
-          'phid = %s',
-          $fork_paste->getFilePHID());
-        $new_paste_text = $fork_file->loadFileData();
-        $new_paste_language = nonempty($fork_paste->getLanguage(), 'infer');
-        $new_paste->setParentPHID($fork_paste->getPHID());
-      }
-    }
-    $this->setErrorView($error_view);
-    $this->setErrorText($e_text);
-    $this->setPasteText($new_paste_text);
-    $new_paste->setLanguage($new_paste_language);
-    $this->setPaste($new_paste);
-  }
-
-  private function renderCreatePaste() {
-    $request = $this->getRequest();
-    $user = $request->getUser();
-
-    $new_paste = $this->getPaste();
-
-    $form = new AphrontFormView();
-    $form->setFlexible(true);
-
-    $available_languages = PhabricatorEnv::getEnvConfig(
-      'pygments.dropdown-choices');
-    asort($available_languages);
-    $language_select = id(new AphrontFormSelectControl())
-      ->setLabel('Language')
-      ->setName('language')
-      ->setValue($new_paste->getLanguage())
-      ->setOptions($available_languages);
-
-    $form
-      ->setUser($user)
-      ->setAction($request->getRequestURI()->getPath())
-      ->addHiddenInput('parent', $new_paste->getParentPHID())
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setLabel('Title')
-          ->setValue($new_paste->getTitle())
-          ->setName('title'))
-      ->appendChild($language_select)
-      ->appendChild(
-        id(new AphrontFormTextAreaControl())
-          ->setLabel('Text')
-          ->setError($this->getErrorText())
-          ->setValue($this->getPasteText())
-          ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
-          ->setName('text'))
-
-    /* TODO: Doesn't have any useful options yet.
-      ->appendChild(
-        id(new AphrontFormPolicyControl())
-          ->setLabel('Visible To')
-          ->setUser($user)
-          ->setValue(
-            $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW))
-          ->setName('policy'))
-    */
-
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-          ->addCancelButton('/paste/')
-          ->setValue('Create Paste'));
-
-    return array(
-      id(new PhabricatorHeaderView())->setHeader('Create Paste'),
-      $form,
-    );
-  }
-
  private function buildPasteList(array $pastes) {
    assert_instances_of($pastes, 'PhabricatorPaste');

    $user = $this->getRequest()->getUser();

+    $this->loadHandles(mpull($pastes, 'getAuthorPHID'));
+
    $list = new PhabricatorObjectItemListView();
    foreach ($pastes as $paste) {
      $created = phabricator_datetime($paste->getDateCreated(), $user);
--- a/src/applications/paste/controller/PhabricatorPasteViewController.php
+++ b/src/applications/paste/controller/PhabricatorPasteViewController.php
@ -97,7 +97,7 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController {
        id(new PhabricatorActionView())
          ->setName(pht('Fork This Paste'))
          ->setIcon('fork')
-          ->setHref($this->getApplicationURI('?fork='.$paste->getID())))
+          ->setHref($this->getApplicationURI('?parent='.$paste->getID())))
      ->addAction(
        id(new PhabricatorActionView())
          ->setName(pht('View Raw File'))
--- a/src/applications/paste/storage/PhabricatorPaste.php
+++ b/src/applications/paste/storage/PhabricatorPaste.php
@ -26,6 +26,10 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
  protected $language;
  protected $parentPHID;

+  public function getURI() {
+    return '/P'.$this->getID();
+  }
+
  public function getConfiguration() {
    return array(
      self::CONFIG_AUX_PHID => true,
@ -54,7 +58,7 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
  public function getFullName() {
    $title = $this->getTitle();
    if (!$title) {
-      $title = 'Untitled Masterwork';
+      $title = '(An Untitled Masterwork)';
    }
    return 'P'.$this->getID().' '.$title;
  }
--- a/src/applications/phid/handle/PhabricatorObjectHandleData.php
+++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php
@ -482,7 +482,7 @@ final class PhabricatorObjectHandleData {
            } else {
              $paste = $pastes[$phid];
              $handle->setName($paste->getTitle());
-              $handle->setFullName('P'.$paste->getID().': '.$paste->getTitle());
+              $handle->setFullName($paste->getFullName());
              $handle->setURI('/P'.$paste->getID());
              $handle->setComplete(true);
            }
--- a/src/view/form/control/AphrontFormTextAreaControl.php
+++ b/src/view/form/control/AphrontFormTextAreaControl.php
@ -25,7 +25,7 @@ final class AphrontFormTextAreaControl extends AphrontFormControl {
  private $height;
  private $readOnly;
  private $enableDragAndDropFileUploads;
-
+  private $customClass;

  public function setHeight($height) {
    $this->height = $height;
@ -50,6 +50,11 @@ final class AphrontFormTextAreaControl extends AphrontFormControl {
    return $this;
  }

+  public function setCustomClass($custom_class) {
+    $this->customClass = $custom_class;
+    return $this;
+  }
+
  protected function renderInput() {

    $height_class = null;
@ -61,6 +66,11 @@ final class AphrontFormTextAreaControl extends AphrontFormControl {
        break;
    }

+    $classes = array();
+    $classes[] = $height_class;
+    $classes[] = $this->customClass;
+    $classes = trim(implode(' ', $classes));
+
    $id = $this->getID();
    if ($this->enableDragAndDropFileUploads) {
      if (!$id) {
@ -81,7 +91,7 @@ final class AphrontFormTextAreaControl extends AphrontFormControl {
        'name'      => $this->getName(),
        'disabled'  => $this->getDisabled() ? 'disabled' : null,
        'readonly'  => $this->getReadonly() ? 'readonly' : null,
-        'class'     => $height_class,
+        'class'     => $classes,
        'style'     => $this->getControlStyle(),
        'id'        => $id,
      ),