many tweaks to the UI of history search

moved scrollIntoView into a new util.js module
-- now it's perfect!

Author: Mark Lentczner

history notes

@@ -126,21 +126,23 @@ kill command
 
 [x] entering history search should focus on command line if needed
 [x] return should enter current focus into command line
-[] ensure that focused entry is always scrolled into view
+[x] ensure that focused entry is always scrolled into view
+[x] better css
+    [-] for when in history mode
+    [x] for the focused item
+[x] corner cases better
+    [x] when the selection goes empty shouldn't loose focus place
+    [x] when focus at bottom with dir 1 goes out of match, then back in
+[x] bindings
+    [x] should ESC key enter or cancel - cancel
+    [x] should ALT+/ and ALT+\ enter history search - yes
+    [x] what key should be bound to clear the search field - C-u
+
 [] scroll list so that it is at bottom, not top of screen
-[] better css
-    [] for when in history mode
-    [] for the focused item
-[] corner cases better
-    [] when the selection goes empty shouldn't loose focus place
-    [] when focus at bottom with dir 1 goes out of match, then back in
-[] animage hiding/showing of output ?
-[] bindings
-    [] should ESC key enter or cancel ?
-    [] should ALT+/ and ALT+\ enter history search ?
-    [] what key should be bound to clear the search field?
-[] display focused entry overlaid onto command line ala completion ?
-[] restore commandline on cancel search?
+
+[-] animate hiding/showing of output ?
+[-] display focused entry overlaid onto command line ala completion ?
+[-] restore commandline on cancel search?
 
 
 From readline's History UI

static/css/plush.css

@@ -439,6 +439,7 @@ h1,h2,h3,h4,blockquote,q,.maia-nav ul {
 
 .job {
   border-left: 2px solid #fff;
+  margin: 0.5em 0;
 }
 .job.topic {
   border-left-color: #ddd;
@@ -586,6 +587,10 @@ h1,h2,h3,h4,blockquote,q,.maia-nav ul {
   display: none;
 }
 
+.history-search .job.history-focus {
+  background-color: #ffe;  
+}
+
 .history-search .job.history-focus .command {
   font-weight: bold;
   padding-left: 0.5em;

static/js/history.js

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-define(['jobs'], function(jobs) {
+define(['util', 'jobs'], function(util, jobs) {
   "use strict";
 
   var api;
@@ -102,34 +102,38 @@ define(['jobs'], function(jobs) {
 
   var searchMode = false;
   var searchFocus = null;
+  var searchPriorFocus = null;
   var searchLastDir = -1;
+  var scrollback;
   var commandline;
 
   function startSearch(dir) {
-    $('#scrollback').addClass('history-search');
     searchMode = true;
     searchFocus = null;
     searchLastDir = dir;
+    scrollback = $('#scrollback');
+    scrollback.addClass('history-search');
     commandline = $('#commandline');
-    search(commandline.val());
     commandline.focus();
+    search();
   }
 
   function cancelSearch() {
-    $('#scrollback').removeClass('history-search');
-    var j = $('#scrollback .job');
+    scrollback.removeClass('history-search');
+    var j = scrollback.find('.job');
     j.show('fast')
     j.removeClass('history-match history-focus');
     searchMode = false;
     searchFocus = null;
+    searchPriorFocus = null;
     commandline.focus();
   }
 
   var ANIM_SPEED = 100;
 
-  function search(s) {
-    s = s || '';
-    $('#scrollback .job')
+  function search() {
+    var s = commandline.val() || '';
+    scrollback.find('.job')
       .each(function(idx) {
           var n = $(this);
           if (s.length === 0 || n.find('.command').text().indexOf(s) >= 0) {
@@ -143,6 +147,18 @@ define(['jobs'], function(jobs) {
     if (!searchFocus || !(searchFocus.hasClass('history-match'))) {
       nextFocus(searchLastDir);
     }
+    setTimeout(rescrollMatch, ANIM_SPEED);
+  }
+
+  function rescrollMatch() {
+    if (searchFocus) {
+      util.scrollIntoView(scrollback, searchFocus);
+    }
+  }
+ 
+  function clearSearch() {
+    commandline.val('');
+    search();
   }
 
   function endSearch() {
@@ -156,24 +172,38 @@ define(['jobs'], function(jobs) {
     cancelSearch();
   }
 
+  function focusEdge(dir) {
+    var js = scrollback.find('.history-match');
+    switch (dir) {
+      case -1: return js.first();
+      case  1: return js.last();
+    }
+  }
   function nextFocus(dir) {
     searchLastDir = dir;
     var next = null;
     if (searchFocus) {
+      searchPriorFocus = searchFocus;
       switch (dir) {
         case -1: next = searchFocus.prevAll('.history-match').first();  break;
         case  1: next = searchFocus.nextAll('.history-match').first(); break;
       }
       searchFocus.removeClass('history-focus');
+      if (next === null || next.length === 0) {
+        next = focusEdge(dir);
+      }
     } else {
-      var js = $('#scrollback .history-match');
-      switch (dir) {
-        case -1: next = js.last();  break;
-        case  1: next = js.first(); break;
+      if (searchPriorFocus && searchPriorFocus.hasClass('history-match')) {
+        next = searchPriorFocus;
+      } else {
+        next = focusEdge(-dir);
+          // -dir because if there is no focus, and the user goes UP,
+          // they want to start with the last item.
       }
     }
     if (next && next.length > 0) {
       next.addClass('history-focus');
+      util.scrollIntoView(scrollback, next);
       searchFocus = next;
     } else {
       searchFocus = null;
@@ -187,6 +217,11 @@ define(['jobs'], function(jobs) {
           case 82: startSearch(-1); return false; // CTRL+R
           case 83: startSearch(1);  return false; // CTRL+S
         }
+      } else if (e.altKey && !(e.ctrlKey || e.shiftKey || e.metaKey)) {
+        switch (e.which) {
+          case 191: startSearch(-1);  return false; // ALT+/
+          case 220: startSearch(1);   return false; // ALT+\
+        }
       }
     } else {
       if (!(e.ctrlKey || e.shiftKey || e.metaKey)) { // ALT+ is optional
@@ -198,21 +233,30 @@ define(['jobs'], function(jobs) {
       if (!(e.ctrlKey || e.altKey || e.shiftKey || e.metaKey)) {
         switch (e.which) {
           case 13: endSearch();     return false; // RETURN
-          case 27: endSearch();     return false; // ESC
+          case 27: cancelSearch();  return false; // ESC
+              // Standard readline bindings would be endSearch, but this
+              // seems to be what most people expect in this context
         }
       } else if (e.ctrlKey && !(e.altKey || e.shiftKey || e.metaKey)) {
         switch (e.which) {
           case 71: cancelSearch();  return false; // CTRL+G
           case 82: nextFocus(-1);   return false; // CTRL+R
           case 83: nextFocus(1);    return false; // CTRL+S
+          case 85: clearSearch();   return false; // CTRL+U
         }
-      } 
+      } else if (e.altKey && !(e.ctrlKey || e.shiftKey || e.metaKey)) {
+        switch (e.which) {
+          case 191: nextFocus(-1);  return false; // ALT+/
+          case 220: nextFocus(1);   return false; // ALT+\
+        }
+      }
+
     }
   }
 
-  function commandChange(s) {
+  function commandChange() {
     if (searchMode) {
-      search(s);
+      search();
       return false;
     }
   }

static/js/jobs.js

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-define(['jquery', 'hterm'], function($){
+define(['jquery', 'util', 'hterm'], function($, util){
   'use strict';
 
   var LINES_IN_TINY = 3;
@@ -39,30 +39,6 @@ define(['jquery', 'hterm'], function($){
     return $(elem).closest('.job').data('jobPrivate');
   }
 
-  function scrollIntoView(scroller, inner, offset) {
-    var sTop = scroller.scrollTop();
-    var sBottom = sTop + scroller.height();
-    var sTop0 = sTop;
-
-    var iTop = inner.offset().top - scroller.children().offset().top;
-      // TODO(mzero): this doesn't work if the scroller has non-scrolled content
-    var iBottom = iTop + inner.outerHeight(true);
-
-    if (offset) {
-      iTop += offset;
-    }
-
-    if (iBottom > sBottom) {
-      sTop += iBottom - sBottom;
-    }
-    if (iTop < sTop) {
-      sTop -= sTop - iTop;
-    }
-    if (sTop != sTop0) {
-      scroller.scrollTop(sTop);
-    }
-  }
-
   var currentTopic = null;
 
   function blurAll() {
@@ -77,7 +53,7 @@ define(['jquery', 'hterm'], function($){
     currentTopic = nextTopic;
     if (currentTopic) {
       currentTopic.addClass('topic focus');
-      scrollIntoView(scrollback, currentTopic);
+      util.scrollIntoView(scrollback, currentTopic);
     }
   }
 
@@ -245,7 +221,7 @@ define(['jquery', 'hterm'], function($){
       } else {
         output.removeClass('output-hide output-tiny output-page output-full');
         output.addClass('output-' + m);
-        setTimeout(function() { scrollIntoView(scrollback, node); }, 100);
+        setTimeout(function() { util.scrollIntoView(scrollback, node); }, 100);
           // have to wait until layout has been recomputed for new size
       }
     };
@@ -387,7 +363,7 @@ define(['jquery', 'hterm'], function($){
       adjustOutput();
 
       if (wasAtEnd && lastOutputTime >= scrollIfAfter) {
-        scrollIntoView(output, outputArea, spanOffset);
+        util.scrollIntoView(output, outputArea, spanOffset);
       }
     }
 

static/js/shell.js

@@ -315,7 +315,7 @@ define(['history', 'cwd', 'jobs', 'jquery'], function(history, cwd, jobs, $){
   });
 
   commandline.keyup(function(e) {
-    var r = history.commandChange($(this).val());
+    var r = history.commandChange();
     if (r !== undefined) return r;
 
     requestRunComplete();

static/js/util.js

@@ -0,0 +1,45 @@
+// Copyright 2012 Google Inc. All Rights Reserved.
+//
+// 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.
+
+define(['jquery'], function($){
+  'use strict';
+
+  function scrollIntoView(scroller, inner, offset) {
+    var sTop = scroller.scrollTop();
+    var sBottom = sTop + scroller.height();
+    var sTop0 = sTop;
+
+    var iTop = inner.offset().top - scroller.children().offset().top;
+      // TODO(mzero): this doesn't work if the scroller has non-scrolled content
+    var iBottom = iTop + inner.outerHeight(true);
+
+    if (offset) {
+      iTop += offset;
+    }
+
+    if (iBottom > sBottom) {
+      sTop += iBottom - sBottom;
+    }
+    if (iTop < sTop) {
+      sTop -= sTop - iTop;
+    }
+    if (sTop != sTop0) {
+      scroller.scrollTop(sTop);
+    }
+  }
+
+  return {
+    scrollIntoView: scrollIntoView
+  };
+});