Move procedural code from views_ui.module and admin.inc to ViewUI

+++ b/includes/admin.incundefined
@@ -2930,11 +1884,11 @@ function views_ui_reorder_displays_form($form, &$form_state) {
-        '#default_value' => isset($display->deleted),
+        '#default_value' => isset($display['deleted']),
       ),
     );
 
-    if (isset($display->deleted) && $display->deleted) {

oh, i'm sorry

+++ b/lib/Drupal/views/Plugin/views/access/Role.phpundefined
@@ -65,7 +65,7 @@ public function buildOptionsForm(&$form, &$form_state) {
-      '#options' => array_map('check_plain', views_ui_get_roles()),
+      '#options' => array_map('check_plain', $this->getRoles()),
       '#description' => t('Only the checked roles will be able to access this display. Note that users with "access all views" can see any view, regardless of role.'),
     );
   }
@@ -81,4 +81,22 @@ public function submitOptionsForm(&$form, &$form_state) {

@@ -81,4 +81,22 @@ public function submitOptionsForm(&$form, &$form_state) {
     $form_state['values']['access_options']['role'] = array_filter($form_state['values']['access_options']['role']);
   }
 
+  /**
+   * Get a list of roles in the system.
+   */
+  protected function getRoles() {
+    static $roles = NULL;
+    if (!isset($roles)) {
+      $roles = array();
+      // Uses db_query() rather than db_select() because the query is static and
+      // does not include any variables.
+      $result = $result = db_query("SELECT r.rid, r.name FROM {role} r ORDER BY r.name");
+      foreach ($result as $obj) {
+        $roles[$obj->rid] = $obj->name;
+      }
+    }
+
+    return $roles;

What about using array_map('check_plain', user_roles(TRUE)), instead? The argument validator is doing that already.

+++ b/lib/Drupal/views/ViewExecutable.phpundefined
@@ -1820,14 +1820,15 @@ public function createDuplicate() {
+  public function cloneView($ui = FALSE) {
+    $storage = clone $this->storage;

Why not extending/overriding the cloneView method in the UI class?

It is always a question whether you have just a broken code or also a broken test,
especially the addDisplay test is a bit confusing why it fails.

Oh gosh... both of the patches in #1793510: Move views toward D8 documentation standard in longer apply after this lands. Both of those cleaned up much of the documentation for admin.inc and views_ui.module.

Understood @tim.

Less procedural crap in .inc and module files++

To big to review in a patch. But +1 for the idea.

This is impressive work so far! Warning: this is no complete review, but as aspilicious said it is hard to review

+++ b/includes/admin.incundefined
@@ -81,298 +19,17 @@ function views_ui_preview(ViewUI $view, $display_id, $args = array()) {
+  $form_state['build_info']['callback'] = array('Drupal\views\ViewUI', 'addForm');

In general i'm wondering whether we should use the same naming as on the plugins, so buildAddForm, validateAddForm etc.

Update status, a deeper review this evening.

add back.

./includes/admin.inc:1209:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1241:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1256:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1481:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1517:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1783:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:1906:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2090:      $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2210:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2264:  $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2340:      $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2441:      $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:2512:      $types = ViewUI::viewsHandlerTypes();
./includes/admin.inc:3186:  $handler_types = ViewUI::viewsHandlerTypes();

I stumbled upon these changes but yeah it kind of makes sense because this file currently uses just ViewUI, so this is fine.

We should really get this in!

+++ b/lib/Drupal/views/ViewUI.phpundefined
index 9a4a227..7ab31f7 100644
--- a/theme/theme.inc

The problem with a theme/theme.inc is that this code is loaded on all pages, which involves a view, not only in the admin interface, maybe we want to change this?

+++ b/views_ui/theme/theme.incundefined
diff --git a/theme/views-ui-display-tab-bucket.tpl.php b/views_ui/theme/views-ui-display-tab-bucket.tpl.php
rename from theme/views-ui-display-tab-bucket.tpl.php
rename to views_ui/theme/views-ui-display-tab-bucket.tpl.php

It is great that you renamed and moved all the files ...

I think this can go in, it is a great improvement!

... well

Back to rtbc