Merge pull request #126 from vermiculus/improve-display-engine

Improve display engine

4 files changed, 133 insertions, 77 deletions

diff --git a/sx-interaction.el b/sx-interaction.el
index 305e61c..b67e0df 100644
--- a/sx-interaction.el
+++ b/sx-interaction.el
@@ -29,13 +29,20 @@
 
 
 ;;; Using data in buffer
-(defun sx--data-here ()
-  "Get the text property `sx--data-here'."
-  (or (get-char-property (point) 'sx--data-here)
+(defun sx--data-here (&optional noerror)
+  "Get data for the question or other object under point.
+If NOERROR is non-nil, don't throw an error on failure.
+
+This looks at the text property `sx--data-here'. If it's not set,
+it looks at a few other reasonable variables. If those fail too,
+it throws an error."
+  (or (get-text-property (point) 'sx--data-here)
       (and (derived-mode-p 'sx-question-list-mode)
            (tabulated-list-get-id))
       (and (derived-mode-p 'sx-question-mode)
-           sx-question-mode--data)))
+           sx-question-mode--data)
+      (and (null noerror)
+           (error "No question data found here"))))
 
 (defun sx--maybe-update-display ()
   "Refresh the question list if we're inside it."
@@ -51,6 +58,8 @@ Only fields contained in TO are copied."
   (setcar to (car from))
   (setcdr to (cdr from)))
 
+
+;;; Visiting
 (defun sx-visit (data &optional copy-as-kill)
   "Visit DATA in a web browser.
 DATA can be a question, answer, or comment. Interactively, it is
@@ -73,6 +82,30 @@ If DATA is a question, also mark it as read."
       (sx-question--mark-read data)
       (sx--maybe-update-display))))
 
+
+;;; Displaying
+(defun sx-display-question (&optional data focus window)
+  "Display question given by DATA, on WINDOW.
+When DATA is nil, display question under point. When FOCUS is
+non-nil (the default when called interactively), also focus the
+relevant window. 
+
+If WINDOW nil, the window is decided by
+`sx-question-mode-display-buffer-function'."
+  (interactive (list (sx--data-here) t))
+  (when (sx-question--mark-read data)
+    (sx--maybe-update-display))
+  ;; Display the question.
+  (setq window
+        (get-buffer-window
+         (sx-question-mode--display data window)))
+  (when focus
+    (if (window-live-p window)
+        (select-window window)
+      (switch-to-buffer sx-question-mode--buffer))))
+
+
+;;; Voting
 (defun sx-toggle-upvote (data)
   "Apply or remove upvote from DATA.
 DATA can be a question, answer, or comment. Interactively, it is
diff --git a/sx-question-list.el b/sx-question-list.el
index 852c11a..e2eb2b6 100644
--- a/sx-question-list.el
+++ b/sx-question-list.el
@@ -145,9 +145,6 @@ Also see `sx-question-list-refresh'."
           .title
           'face (if (sx-question--read-p question-data)
                     'sx-question-list-read-question
-                  ;; Increment `sx-question-list--unread-count' for
-                  ;; the mode-line.
-                  (cl-incf sx-question-list--unread-count)
                   'sx-question-list-unread-question))
          (propertize " " 'display "\n   ")
          (propertize favorite 'face 'sx-question-list-favorite)
@@ -305,7 +302,7 @@ into consideration.
    ("d" sx-toggle-downvote)
    ("h" sx-question-list-hide)
    ("m" sx-question-list-mark-read)
-   ([?\r] sx-question-list-display-question)))
+   ([?\r] sx-display-question)))
 
 (defun sx-question-list-hide (data)
   "Hide question under point.
@@ -335,10 +332,6 @@ Non-interactively, DATA is a question alist."
   ;; "Unanswered", etc.
   "Variable describing current tab being viewed.")
 
-(defvar sx-question-list--unread-count 0
-  "Holds the number of unread questions in the current buffer.")
-(make-variable-buffer-local 'sx-question-list--unread-count)
-
 (defvar sx-question-list--total-count 0
   "Holds the total number of questions in the current buffer.")
 (make-variable-buffer-local 'sx-question-list--total-count)
@@ -352,7 +345,7 @@ Non-interactively, DATA is a question alist."
     " ["
     "Unread: "
     (:propertize
-     (:eval (int-to-string sx-question-list--unread-count))
+     (:eval (sx-question-list--unread-count))
      face mode-line-buffer-id)
     ", "
     "Total: "
@@ -362,6 +355,12 @@ Non-interactively, DATA is a question alist."
     "] ")
   "Mode-line construct to use in question-list buffers.")
 
+(defun sx-question-list--unread-count ()
+  "Number of unread questions in current dataset, as a string."
+  (int-to-string
+   (cl-count-if-not
+    #'sx-question--read-p sx-question-list--dataset)))
+
 (defun sx-question-list--update-mode-line ()
   "Fill the mode-line with useful information."
   ;; All the data we need is right in the buffer.
@@ -381,7 +380,6 @@ If the prefix argument NO-UPDATE is nil, query StackExchange for
 a new list before redisplaying."
   (interactive "p\nP")
   ;; Reset the mode-line unread count (we rebuild it here).
-  (setq sx-question-list--unread-count 0)
   (unless no-update
     (setq sx-question-list--pages-so-far 1))
   (let* ((question-list
@@ -424,7 +422,37 @@ Displayed in `sx-question-mode--window', replacing any question
 that may currently be there."
   (interactive "p")
   (sx-question-list-next n)
-  (sx-question-list-display-question))
+  (sx-display-question
+   (tabulated-list-get-id)
+   nil 
+   (sx-question-list--create-question-window)))
+
+(defun sx-question-list--create-question-window ()
+  "Create or find a window where a question can be displayed.
+
+If any current window displays a question, that window is
+returned. If none do, a new one is created such that the
+question-list window remains `sx-question-list-height' lines
+high (if possible)."
+  (or (sx-question-mode--get-window)
+      ;; Create a proper window.
+      (let ((window
+             (condition-case er
+                 (split-window (selected-window) sx-question-list-height 'below)
+               (error
+                ;; If the window is too small to split, use any one.
+                (if (string-match
+                     "Window #<window .*> too small for splitting"
+                     (car (cdr-safe er)))
+                    (next-window)
+                  (error (cdr er)))))))
+        ;; Configure the window to be closed on `q'.
+        (set-window-prev-buffers window nil)
+        (set-window-parameter
+         window 'quit-restore
+         ;; See (info "(elisp) Window Parameters")
+         `(window window ,(selected-window) ,sx-question-mode--buffer))
+        window)))
 
 (defun sx-question-list-next (n)
   "Move cursor down N questions.
@@ -436,7 +464,21 @@ This does not update `sx-question-mode--window'."
     ;; If we were trying to move forward, but we hit the end.
     (when (eobp)
       ;; Try to get more questions.
-      (sx-question-list-next-page))))
+      (sx-question-list-next-page))
+    (sx-question-list--ensure-line-good-line-position)))
+
+(defun sx-question-list--ensure-line-good-line-position ()
+  "Scroll window such that current line is a good place.
+Check if we're at least 6 lines from the bottom. Scroll up if
+we're not. Do the same for 3 lines from the top."
+  ;; At least one entry below us.
+  (let ((lines-to-bottom (count-screen-lines (point) (window-end))))
+    (unless (>= lines-to-bottom 6)
+      (recenter (- 6))))
+  ;; At least one entry above us.
+  (let ((lines-to-top (count-screen-lines (point) (window-start))))
+    (unless (>= lines-to-top 3)
+      (recenter 3))))
 
 (defun sx-question-list-next-page ()
   "Fetch and display the next page of questions."
@@ -486,44 +528,6 @@ This does not update `sx-question-mode--window'."
   (interactive "p")
   (sx-question-list-next-far (- n)))
 
-(defun sx-question-list-display-question (&optional data focus)
-  "Display question given by DATA.
-When DATA is nil, display question under point.  When FOCUS is
-non-nil (the default when called interactively), also focus the
-relevant window."
-  (interactive '(nil t))
-  (unless data (setq data (tabulated-list-get-id)))
-  (unless data (error "No question here!"))
-  (unless (sx-question--read-p data)
-    (cl-decf sx-question-list--unread-count)
-    (sx-question--mark-read data)
-    (sx-question-list-refresh 'redisplay 'no-update))
-  (unless (and (window-live-p sx-question-mode--window)
-               (null (equal sx-question-mode--window (selected-window))))
-    (setq sx-question-mode--window
-          (condition-case er
-              (split-window (selected-window) sx-question-list-height 'below)
-            (error
-             ;; If the window is too small to split, use current one.
-             (if (string-match
-                  "Window #<window .*> too small for splitting"
-                  (car (cdr-safe er)))
-                 nil
-               (error (cdr er)))))))
-  ;; Display the question.
-  (sx-question-mode--display data sx-question-mode--window)
-  ;; Configure the window to be closed on `q'.
-  (set-window-prev-buffers sx-question-mode--window nil)
-  (set-window-parameter
-   sx-question-mode--window
-   'quit-restore
-   ;; See (info "(elisp) Window Parameters")
-   `(window window ,(selected-window) ,sx-question-mode--buffer))
-  (when focus
-    (if sx-question-mode--window
-        (select-window sx-question-mode--window)
-      (switch-to-buffer sx-question-mode--buffer))))
-
 (defun sx-question-list-switch-site (site)
   "Switch the current site to SITE and display its questions.
 Use `ido-completing-read' if variable `ido-mode' is active.  
diff --git a/sx-question-mode.el b/sx-question-mode.el
index c44519c..91044ff 100644
--- a/sx-question-mode.el
+++ b/sx-question-mode.el
@@ -30,8 +30,13 @@
 
 
 ;;; Displaying a question
-(defvar sx-question-mode--window nil
-  "Window where the content of questions is displayed.")
+(defcustom sx-question-mode-display-buffer-function #'switch-to-buffer
+  "Function used to display the question buffer.
+Called, for instance, when hitting \\<sx-question-list-mode-map>`\\[sx-question-list-display-question]' on an entry in the
+question list.
+This is not used when navigating the question list with `\\[sx-question-list-view-next]."
+  :type 'function
+  :group 'sx-question-mode)
 
 (defvar sx-question-mode--buffer nil
   "Buffer being used to display questions.")
@@ -39,6 +44,14 @@
 (defvar sx-question-mode--data nil
   "The data of the question being displayed.")
 
+(defun sx-question-mode--get-window ()
+  "Return a window displaying a question, or nil."
+  (car-safe
+   (cl-member-if
+    (lambda (x) (with-selected-window x
+             (derived-mode-p 'sx-question-mode)))
+    (window-list nil 'never nil))))
+
 (defun sx-question-mode--display (data &optional window)
   "Display question given by DATA on WINDOW.
 If WINDOW is nil, use selected one.
@@ -71,7 +84,8 @@ If WINDOW is given, use that to display the buffer."
    ;; No window, but the buffer is already being displayed somewhere.
    ((get-buffer-window sx-question-mode--buffer 'visible))
    ;; Neither, so we create the window.
-   (t (switch-to-buffer sx-question-mode--buffer)))
+   (t (funcall sx-question-mode-display-buffer-function
+        sx-question-mode--buffer)))
   sx-question-mode--buffer)
 
 
diff --git a/sx-question.el b/sx-question.el
index 00b5f7f..c4b2445 100644
--- a/sx-question.el
+++ b/sx-question.el
@@ -88,27 +88,32 @@ See `sx-question--user-read-list'."
 
 (defun sx-question--mark-read (question)
   "Mark QUESTION as being read until it is updated again.
+Returns nil if question (in its current state) was already marked
+read, i.e., if it was `sx-question--read-p'.
 See `sx-question--user-read-list'."
-  (sx-assoc-let question
-    (sx-question--ensure-read-list .site)
-    (let ((site-cell (assoc .site sx-question--user-read-list))
-          (q-cell (cons .question_id .last_activity_date))
-          cell)
-      (cond
-       ;; First question from this site.
-       ((null site-cell)
-        (push (list .site q-cell) sx-question--user-read-list))
-       ;; Question already has an older time.
-       ((setq cell (assoc .question_id site-cell))
-        (setcdr cell .last_activity_date))
-       ;; Question wasn't present.
-       (t
-        (sx-sorted-insert-skip-first
-         q-cell site-cell (lambda (x y) (> (car x) (car y))))))))
-  ;; Save the results.
-  ;; @TODO This causes a small lag on `j' and `k' as the list gets
-  ;; large.  Should we do this on a timer?
-  (sx-cache-set 'read-questions sx-question--user-read-list))
+  (prog1
+      (sx-assoc-let question
+        (sx-question--ensure-read-list .site)
+        (let ((site-cell (assoc .site sx-question--user-read-list))
+              (q-cell (cons .question_id .last_activity_date))
+              cell)
+          (cond
+           ;; First question from this site.
+           ((null site-cell)
+            (push (list .site q-cell) sx-question--user-read-list))
+           ;; Question already present.
+           ((setq cell (assoc .question_id site-cell))
+            ;; Current version is newer than cached version.
+            (when (> .last_activity_date (cdr cell))
+              (setcdr cell .last_activity_date)))
+           ;; Question wasn't present.
+           (t
+            (sx-sorted-insert-skip-first
+             q-cell site-cell (lambda (x y) (> (car x) (car y))))))))
+    ;; Save the results.
+    ;; @TODO This causes a small lag on `j' and `k' as the list gets
+    ;; large.  Should we do this on a timer?
+    (sx-cache-set 'read-questions sx-question--user-read-list)))
 
 
 ;;;; Hidden