From 90c39417bbe56f67cf3fca8de7af3e739dc29829 Mon Sep 17 00:00:00 2001
From: Peter Cottle <pcottle@fb.com>
Date: Wed, 13 Aug 2014 19:22:45 -0700
Subject: [PATCH] PR #196 code cleanup and solution description rather than
 reordering

---
 src/js/git/index.js        | 19 +++++----------
 src/js/views/rebaseView.js | 48 +++++++-------------------------------
 src/style/main.css         |  6 +++++
 src/template.index.html    |  6 +++++
 4 files changed, 27 insertions(+), 52 deletions(-)

diff --git a/src/js/git/index.js b/src/js/git/index.js
index b47a81e4..9c260382 100644
--- a/src/js/git/index.js
+++ b/src/js/git/index.js
@@ -2184,26 +2184,19 @@ GitEngine.prototype.rebaseInteractive = function(targetSource, currentLocation,
   if (options.initialCommitOrdering && options.initialCommitOrdering.length > 0) {
     var rebaseMap = {};
     _.each(toRebase, function(commit) {
-      var id = commit.get('id');
-      rebaseMap[id] = commit;
+      rebaseMap[commit.get('id')] = true;
     });
     
     // Verify each chosen commit exists in the list of commits given to the user
-    var extraCommits = [];
     initialCommitOrdering = [];
     _.each(options.initialCommitOrdering[0].split(','), function(id) {
-      if (id in rebaseMap) {
-        initialCommitOrdering.push(rebaseMap[id]);
-      } else {
-        extraCommits.push(id);
+      if (!rebaseMap[id]) {
+        throw new GitError({
+          msg: intl.todo('Hey those commits dont exist in the set!')
+        });
       }
+      initialCommitOrdering.push(id);
     });
-    
-    if (extraCommits.length > 0) {
-      throw new GitError({
-        msg: intl.todo('Hey those commits dont exist in the set!')
-      });
-    }
   }
   
   // The rebase view expects the commits reversed, so do that here
diff --git a/src/js/views/rebaseView.js b/src/js/views/rebaseView.js
index 24aec9fc..69cb1b46 100644
--- a/src/js/views/rebaseView.js
+++ b/src/js/views/rebaseView.js
@@ -13,54 +13,24 @@ var InteractiveRebaseView = ContainedBase.extend({
   tagName: 'div',
   template: _.template($('#interactive-rebase-template').html()),
 
-  createRebaseEntries: function() {
+  initialize: function(options) {
+    this.deferred = options.deferred;
     this.rebaseMap = {};
     this.entryObjMap = {};
-    this.rebaseEntries = new RebaseEntryCollection();
-    
-    // If we are displaying a solution, we potentially only want to pick certain commits, and reorder
-    // the ones that are picked. The commits we want to pick and the order are contained in the options.initialCommitOrdering,
-    // the list of all the commits that are part of the rebase are in options.toRebase
-    var commitsToUse = this.options.initialCommitOrdering === undefined ? this.options.toRebase
-                                                                        : this.options.initialCommitOrdering;
+    this.options = options;
 
-    _.each(commitsToUse, function(commit) {
+    this.rebaseEntries = new RebaseEntryCollection();
+    options.toRebase.reverse();
+    _.each(options.toRebase, function(commit) {
       var id = commit.get('id');
       this.rebaseMap[id] = commit;
 
       // make basic models for each commit
       this.entryObjMap[id] = new RebaseEntry({
-        id: id,
-        pick: true
+        id: id
       });
       this.rebaseEntries.add(this.entryObjMap[id]);
     }, this);
-    
-    // If we are using the initialCommitOrdering, we might not have picked all of the commits,
-    // but we would still want to see the other unpicked ones. Just show them as unpicked by default
-    if (this.options.initialCommitOrdering !== undefined) {
-      _.each(this.options.toRebase, function(commit) {
-        var id = commit.get('id');
-        
-        if (!(id in this.rebaseMap)) {
-          this.rebaseMap[id] = commit;
-
-          // make basic models for each commit
-          this.entryObjMap[id] = new RebaseEntry({
-            id: id,
-            pick: false
-          });
-        }
-        this.rebaseEntries.add(this.entryObjMap[id]);
-      }, this);
-    }
-  },
-  
-  initialize: function(options) {
-    this.deferred = options.deferred;
-    this.options = options;
-    
-    this.createRebaseEntries();
 
     this.container = new ModalTerminal({
       title: 'Interactive Rebase'
@@ -110,7 +80,8 @@ var InteractiveRebaseView = ContainedBase.extend({
 
   render: function() {
     var json = {
-      num: _.keys(this.rebaseMap).length
+      num: _.keys(this.rebaseMap).length,
+      solutionOrder: this.options.initialCommitOrdering
     };
 
     var destination = this.container.getInsideElement();
@@ -200,7 +171,6 @@ var RebaseEntryView = Backbone.View.extend({
 
     // hacky :( who would have known jquery barfs on ids with %'s and quotes
     this.listEntry = this.$el.children(':last');
-    this.listEntry.toggleClass('notPicked', !this.model.get('pick'));
 
     this.listEntry.delegate('#toggleButton', 'click', _.bind(function() {
       this.toggle();
diff --git a/src/style/main.css b/src/style/main.css
index 1fb9d674..51d0e8ad 100644
--- a/src/style/main.css
+++ b/src/style/main.css
@@ -692,6 +692,12 @@ div.terminal-text p.helperText,
   font-size: 10px;
 }
 
+.iRebaseDialog p.solutionText {
+  color: lime;
+  font-weight: bold;
+  margin: 0;
+}
+
 .iRebaseDialog p.helperText {
   color: #999;
   margin: 0;
diff --git a/src/template.index.html b/src/template.index.html
index c88b33d9..e697ac0f 100644
--- a/src/template.index.html
+++ b/src/template.index.html
@@ -413,6 +413,12 @@
       <p>
         Rebasing <%= num %> Commits
       </p>
+      <% if (solutionOrder && solutionOrder.length) { %>
+        <p class="solutionText">
+          For the solution, order the commits as
+            <%= solutionOrder.join(', ') %>
+        </p>
+      <% } %>
       <p class="helperText">
         (Drag and drop to re-order. Toggle the "pick" button to omit or re-add a commit)
       </p>