Provide transaction-based edits of repository encoding

Summary: Adds support for the "encoding" field to the new transactional interface.

Test Plan:
{F44189}

{F44190}

Some of the encodings in the second screen are from testing, and can no longer be set.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

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

src/__phutil_library_map__.php

@@ -480,6 +480,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryController' => 'applications/diffusion/controller/DiffusionRepositoryController.php',
     'DiffusionRepositoryEditBasicController' => 'applications/diffusion/controller/DiffusionRepositoryEditBasicController.php',
     'DiffusionRepositoryEditController' => 'applications/diffusion/controller/DiffusionRepositoryEditController.php',
+    'DiffusionRepositoryEditEncodingController' => 'applications/diffusion/controller/DiffusionRepositoryEditEncodingController.php',
     'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php',
     'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
     'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
@@ -2283,6 +2284,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryController' => 'DiffusionController',
     'DiffusionRepositoryEditBasicController' => 'DiffusionController',
     'DiffusionRepositoryEditController' => 'DiffusionController',
+    'DiffusionRepositoryEditEncodingController' => 'DiffusionController',
     'DiffusionSetupException' => 'AphrontUsageException',
     'DiffusionStableCommitNameQuery' => 'DiffusionQuery',
     'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery',
@@ -3075,6 +3077,7 @@ phutil_register_library_map(array(
     array(
       0 => 'PhabricatorRepositoryDAO',
       1 => 'PhabricatorPolicyInterface',
+      2 => 'PhabricatorMarkupInterface',
     ),
     'PhabricatorRepositoryArcanistProject' => 'PhabricatorRepositoryDAO',
     'PhabricatorRepositoryArcanistProjectDeleteController' => 'PhabricatorRepositoryController',

src/applications/diffusion/application/PhabricatorApplicationDiffusion.php

@@ -65,6 +65,7 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication {
           'edit/' => array(
             '' => 'DiffusionRepositoryEditController',
             'basic/' => 'DiffusionRepositoryEditBasicController',
+            'encoding/' => 'DiffusionRepositoryEditEncodingController',
           ),
         ),
         'inline/' => array(

src/applications/diffusion/controller/DiffusionRepositoryEditController.php

@@ -24,6 +24,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     $content[] = $this->buildBasicActions($repository);
     $content[] = $this->buildBasicProperties($repository);
 
+    $content[] = id(new PhabricatorHeaderView())
+      ->setHeader(pht('Text Encoding'));
+
+    $content[] = $this->buildEncodingActions($repository);
+    $content[] = $this->buildEncodingProperties($repository);
 
     $content[] = id(new PhabricatorHeaderView())
       ->setHeader(pht('Edit History'));
@@ -86,8 +91,7 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     $user = $this->getRequest()->getUser();
 
     $view = id(new PhabricatorPropertyListView())
-      ->setUser($user)
+      ->setUser($user);
-      ->setObject($repository);
 
     $view->addProperty(pht('Name'), $repository->getName());
     $view->addProperty(pht('ID'), $repository->getID());
@@ -114,6 +118,44 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     return $view;
   }
 
+  private function buildEncodingActions(PhabricatorRepository $repository) {
+    $user = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorActionListView())
+      ->setUser($user);
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $user,
+      $repository,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $edit = id(new PhabricatorActionView())
+      ->setIcon('edit')
+      ->setName(pht('Edit Text Encoding'))
+      ->setHref(
+        $this->getRepositoryControllerURI($repository, 'edit/encoding/'))
+      ->setDisabled(!$can_edit);
+    $view->addAction($edit);
+
+    return $view;
+  }
+
+  private function buildEncodingProperties(PhabricatorRepository $repository) {
+    $user = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorPropertyListView())
+      ->setUser($user);
+
+    $encoding = $repository->getDetail('encoding');
+    if (!$encoding) {
+      $encoding = phutil_tag('em', array(), pht('Use Default (UTF-8)'));
+    }
+
+    $view->addProperty(pht('Encoding'), $encoding);
+
+    return $view;
+  }
+
 
 
 }

src/applications/diffusion/controller/DiffusionRepositoryEditEncodingController.php

@@ -0,0 +1,120 @@
+<?php
+
+final class DiffusionRepositoryEditEncodingController
+  extends DiffusionController {
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+    $drequest = $this->diffusionRequest;
+    $repository = $drequest->getRepository();
+
+    $repository = id(new PhabricatorRepositoryQuery())
+      ->setViewer($user)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->withIDs(array($repository->getID()))
+      ->executeOne();
+
+    if (!$repository) {
+      return new Aphront404Response();
+    }
+
+    $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
+
+    $v_encoding = $repository->getDetail('encoding');
+    $e_encoding = null;
+    $errors = array();
+
+    if ($request->isFormPost()) {
+      $v_encoding = $request->getStr('encoding');
+
+      if (!$errors) {
+        $xactions = array();
+        $template = id(new PhabricatorRepositoryTransaction());
+
+        $type_encoding = PhabricatorRepositoryTransaction::TYPE_ENCODING;
+
+        $xactions[] = id(clone $template)
+          ->setTransactionType($type_encoding)
+          ->setNewValue($v_encoding);
+
+        try {
+          id(new PhabricatorRepositoryEditor())
+            ->setContinueOnNoEffect(true)
+            ->setContentSourceFromRequest($request)
+            ->setActor($user)
+            ->applyTransactions($repository, $xactions);
+
+          return id(new AphrontRedirectResponse())->setURI($edit_uri);
+        } catch (Exception $ex) {
+          $errors[] = $ex->getMessage();
+        }
+      }
+    }
+
+    $content = array();
+
+    $crumbs = $this->buildCrumbs();
+    $crumbs->addCrumb(
+      id(new PhabricatorCrumbView())
+        ->setName(pht('Edit Encoding')));
+    $content[] = $crumbs;
+
+    $title = pht('Edit %s', $repository->getName());
+
+    if ($errors) {
+      $content[] = id(new AphrontErrorView())
+        ->setTitle(pht('Form Errors'))
+        ->setErrors($errors);
+    }
+
+    $form = id(new AphrontFormView())
+      ->setUser($user)
+      ->setFlexible(true)
+      ->appendRemarkupInstructions($this->getEncodingInstructions())
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setName('encoding')
+          ->setLabel(pht('Text Encoding'))
+          ->setValue($v_encoding)
+          ->setError($e_encoding))
+      ->appendChild(
+        id(new AphrontFormSubmitControl())
+          ->setValue(pht('Save Encoding'))
+          ->addCancelButton($edit_uri));
+
+    $content[] = $form;
+
+    return $this->buildApplicationPage(
+      $content,
+      array(
+        'title' => $title,
+        'dust' => true,
+        'device' => true,
+      ));
+  }
+
+  private function getEncodingInstructions() {
+    return pht(<<<EOT
+If source code in this repository uses a character encoding other than UTF-8
+(for example, `ISO-8859-1`), specify it here.
+
+**Normally, you can leave this field blank.** If your source code is written in
+ASCII or UTF-8, everything will work correctly.
+
+Source files will be translated from the specified encoding to UTF-8 when they
+are read from the repository, before they are displayed in Diffusion.
+
+See [[%s | UTF-8 and Character Encoding]] for more information on how
+Phabricator handles text encodings.
+EOT
+    ,
+    PhabricatorEnv::getDoclink(
+      'article/User_Guide_UTF-8_and_Character_Encoding.html'));
+  }
+
+}

src/applications/repository/editor/PhabricatorRepositoryEditor.php

@@ -8,6 +8,7 @@ final class PhabricatorRepositoryEditor
 
     $types[] = PhabricatorRepositoryTransaction::TYPE_NAME;
     $types[] = PhabricatorRepositoryTransaction::TYPE_DESCRIPTION;
+    $types[] = PhabricatorRepositoryTransaction::TYPE_ENCODING;
 
     return $types;
   }
@@ -21,6 +22,8 @@ final class PhabricatorRepositoryEditor
         return $object->getName();
       case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
         return $object->getDetail('description');
+      case PhabricatorRepositoryTransaction::TYPE_ENCODING:
+        return $object->getDetail('encoding');
     }
   }
 
@@ -31,6 +34,7 @@ final class PhabricatorRepositoryEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorRepositoryTransaction::TYPE_NAME:
       case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
+      case PhabricatorRepositoryTransaction::TYPE_ENCODING:
         return $xaction->getNewValue();
     }
   }
@@ -46,6 +50,27 @@ final class PhabricatorRepositoryEditor
       case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
         $object->setDetail('description', $xaction->getNewValue());
         break;
+      case PhabricatorRepositoryTransaction::TYPE_ENCODING:
+        // Make sure the encoding is valid by converting to UTF-8. This tests
+        // that the user has mbstring installed, and also that they didn't type
+        // a garbage encoding name. Note that we're converting from UTF-8 to
+        // the target encoding, because mbstring is fine with converting from
+        // a nonsense encoding.
+        $encoding = $xaction->getNewValue();
+        if (strlen($encoding)) {
+          try {
+            phutil_utf8_convert('.', $encoding, 'UTF-8');
+          } catch (Exception $ex) {
+            throw new PhutilProxyException(
+              pht(
+                "Error setting repository encoding '%s': %s'",
+                $encoding,
+                $ex->getMessage()),
+              $ex);
+          }
+        }
+        $object->setDetail('encoding', $encoding);
+        break;
     }
   }
 

src/applications/repository/storage/PhabricatorRepositoryTransaction.php

@@ -5,6 +5,7 @@ final class PhabricatorRepositoryTransaction
 
   const TYPE_NAME         = 'repo:name';
   const TYPE_DESCRIPTION  = 'repo:description';
+  const TYPE_ENCODING     = 'repo:encoding';
 
   public function getApplicationName() {
     return 'repository';
@@ -39,6 +40,24 @@ final class PhabricatorRepositoryTransaction
         return pht(
           '%s updated the description of this repository.',
           $this->renderHandleLink($author_phid));
+      case self::TYPE_ENCODING:
+        if (strlen($old) && !strlen($new)) {
+          return pht(
+            '%s removed the "%s" encoding configured for this repository.',
+            $this->renderHandleLink($author_phid),
+            $old);
+        } else if (strlen($new) && !strlen($old)) {
+          return pht(
+            '%s set the encoding for this repository to "%s".',
+            $this->renderHandleLink($author_phid),
+            $new);
+        } else {
+          return pht(
+            '%s changed the repository encoding from "%s" to "%s".',
+            $this->renderHandleLink($author_phid),
+            $old,
+            $new);
+        }
     }
 
     return parent::getTitle();

src/docs/userguide/utf8.diviner

@@ -11,12 +11,21 @@ this means that you should write your source code in UTF-8. In most cases this
 does not require you to change anything, because ASCII text is a subset of
 UTF-8.
 
+If you have a repository with source files that do not have UTF-8, you have two
+options:
+
+  - Convert all files in the repository to ASCII or UTF-8 (see "Detecting and
+    Repairing Files" below). This is recommended, especially if the encoding
+    problems are accidental.
+  - Configure Phabricator to convert files into UTF-8 from whatever encoding
+    your repository is in when it needs to (see "Support for Alternate
+    Encodings" below). This is not completely supported, and repositories with
+    files that have multiple encodings are not supported.
+
 = Detecting and Repairing Files =
 
 It is recommended that you write source files only in ASCII text, but
-Phabricator fully supports UTF-8 source files. However, it won't currently do
+Phabricator fully supports UTF-8 source files.
-encoding transformation, so if you have source files which are not valid UTF-8
-you may run into issues.
 
 If you have a project which isn't valid UTF-8 because a few files have random
 binary nonsense in them, there is a script in libphutil which can help you
@@ -55,8 +64,17 @@ Phabricator doesn't include any default tools to help you process them in a
 systematic way. You could hack up ##utf8.php## as a starting point, or use other
 tools to batch-process your source files.
 
-NOTE: If you have a project which uses a //different encoding// for source
+= Support for Alternate Encodings =
-files, there is no easy way to get it working with Phabricator or Arcanist right
+
-now. If it's not reasonable to switch to UTF-8, tell us more about your use case
+Phabricator has some support for encodings other than UTF-8.
-and we can evaluate supporting it. Since tools like Git don't work well with
+
-other encodings, the prevailing assumption is that this is a rare situation.
+NOTE: Alternate encodings are not completely supported, and a few features will
+not work correctly. Codebases with files that have multiple different encodings
+(for example, some files in ISO-8859-1 and some files in Shift-JIS) are not
+supported at all.
+
+To use an alternate encoding, edit the repository in Diffusion and specify the
+encoding to use.
+
+Optionally, you can use the `--encoding` flag when running `arc`, or set
+`encoding` in your `.arcconfig`.