Addressing review comments from Prot.

3 files changed, 31 insertions, 39 deletions

diff --git a/buildbot-utils.el b/buildbot-utils.el
index 17dc085..d016330 100644
--- a/buildbot-utils.el
+++ b/buildbot-utils.el
@@ -47,9 +47,12 @@
     (kill-region (point) (progn (re-search-forward "\r?\n\r?\n")
                                 (point)))))
 
-(defun buildbot-url-fetch-json (url &optional decompression with-header)
-  "Fetch and parse a json object from URL.
+(defun buildbot-url-fetch-internal (url processor &optional
+                                        decompression with-header)
+  "Fetch from URL and process the response payload using PROCESSOR.
 
+PROCESSOR is a function that takes no argument and processes the
+current buffer.
 With non-nil DECOMPRESSION, decompress the response.
 With non-nil WITH-HEADER, include the header in the result."
   (with-current-buffer (get-buffer-create buildbot-client-buffer-name)
@@ -73,39 +76,24 @@ With non-nil WITH-HEADER, include the header in the result."
             (if with-header
                 (list
                  (cons 'header fields)
-                 (cons 'json (json-read)))
-              (json-read)))
+                 (cons 'json (funcall processor)))
+              (funcall processor)))
         (error "HTTP error: %s" (buffer-substring (point) (point-max)))))))
 
+(defun buildbot-url-fetch-json (url &optional decompression with-header)
+  "Fetch and parse a json object from URL.
+
+With non-nil DECOMPRESSION, decompress the response.
+With non-nil WITH-HEADER, include the header in the result."
+  (buildbot-url-fetch-internal url 'json-read decompression with-header))
+
 (defun buildbot-url-fetch-raw (url &optional decompression with-header)
   "Fetch from URL.
 
 With non-nil DECOMPRESSION, decompress the response.
 With non-nil WITH-HEADER, include the header in the result."
-  (with-current-buffer (get-buffer-create buildbot-client-buffer-name)
-    (goto-char (point-max))
-    (insert "[" (current-time-string) "] Request: " url "\n"))
-  (with-current-buffer (url-retrieve-synchronously url t)
-    (let ((header) (status) (fields))
-      (buildbot-delete-http-header)
-      (goto-char (point-min))
-      (setq header (buildbot-parse-http-header (car kill-ring))
-            status (alist-get 'status header)
-            fields (alist-get 'fields header))
-      (with-current-buffer buildbot-client-buffer-name
-        (insert "[" (current-time-string) "] Response: " status "\n"))
-      (when decompression
-        (call-process-region (point) (point-max) "gunzip" t t t)
-        (goto-char (point-min)))
-      (call-interactively 'delete-trailing-whitespace)
-      (if (string= status "200")
-          (unless (= (point) (point-max))
-            (if with-header
-                (list
-                 (cons 'header fields)
-                 (cons 'json (buffer-string)))
-              (buffer-string)))
-        (error "HTTP error: %s" (buffer-substring (point) (point-max)))))))
+  (buildbot-url-fetch-internal url 'buffer-string decompression
+                               with-header))
 
 (defun buildbot-format-attr (attr)
   "Format an alist ATTR into a url query string."
diff --git a/buildbot-view.el b/buildbot-view.el
index 1be8bce..4f2c278 100644
--- a/buildbot-view.el
+++ b/buildbot-view.el
@@ -38,6 +38,19 @@
 (defvar-local buildbot-view-type nil)
 (defvar-local buildbot-view-data nil)
 
+(defvar buildbot-view-mode-map
+  (let ((kmap (make-sparse-keymap)))
+    (define-key kmap (kbd "M-n") #'buildbot-view-next-header)
+    (define-key kmap "n" #'buildbot-view-next-failed-header)
+    (define-key kmap "f" #'buildbot-view-next-header-same-thing)
+    (define-key kmap (kbd "M-p") #'buildbot-view-previous-header)
+    (define-key kmap "p" #'buildbot-view-previous-failed-header)
+    (define-key kmap (kbd "b") #'buildbot-view-previous-header-same-thing)
+    (define-key kmap "g" #'buildbot-view-reload)
+    (define-key kmap (kbd "<return>") #'buildbot-view-open-thing-at-point)
+    kmap)
+  "Keymap for `buildbot-view-mode'.")
+
 (define-derived-mode buildbot-view-mode special-mode "Buildbot"
   "A Buildbot client for Emacs.")
 
@@ -48,7 +61,6 @@
     (end-of-line 1)
     (re-search-forward buildbot-view-header-regex)
     (beginning-of-line 1)))
-(define-key buildbot-view-mode-map (kbd "M-n") #'buildbot-view-next-header)
 
 (defun buildbot-view-next-failed-header (n)
   "Move forward N headers with failed states."
@@ -57,7 +69,6 @@
     (end-of-line 1)
     (text-property-search-forward 'face 'error)
     (beginning-of-line 1)))
-(define-key buildbot-view-mode-map "n" #'buildbot-view-next-failed-header)
 
 (defun buildbot-view-next-header-same-thing (n)
   "Move forward N headers of the same type."
@@ -68,8 +79,6 @@
       (buildbot-view-next-header 1)
       (while (not (eq (get-text-property (point) 'type) type))
         (buildbot-view-next-header 1)))))
-(define-key buildbot-view-mode-map "f"
-  #'buildbot-view-next-header-same-thing)
 
 (defun buildbot-view-previous-header (n)
   "Move backward N headers."
@@ -79,7 +88,6 @@
     (re-search-backward buildbot-view-header-regex))
   (dotimes (_ n)
     (re-search-backward buildbot-view-header-regex)))
-(define-key buildbot-view-mode-map (kbd "M-p") #'buildbot-view-previous-header)
 
 (defun buildbot-view-previous-failed-header (n)
   "Move back N headers of failed states."
@@ -90,7 +98,6 @@
   (dotimes (_ n)
     (text-property-search-backward 'face 'error))
   (beginning-of-line 1))
-(define-key buildbot-view-mode-map "p" #'buildbot-view-previous-failed-header)
 
 (defun buildbot-view-previous-header-same-thing (n)
   "Move back N headers of the same type."
@@ -101,8 +108,6 @@
       (buildbot-view-previous-header 1)
       (while (not (eq (get-text-property (point) 'type) type))
         (buildbot-view-previous-header 1)))))
-(define-key buildbot-view-mode-map (kbd "b")
-  #'buildbot-view-previous-header-same-thing)
 
 (defun buildbot-view-format-revision-info (revision-info)
   "Format REVISION-INFO header in the view."
@@ -311,7 +316,6 @@ With a non-nil FORCE, reload the view buffer if exists."
   "Reload a view buffer."
   (interactive)
   (buildbot-view-update))
-(define-key buildbot-view-mode-map "g" #'buildbot-view-reload)
 
 ;;;###autoload
 (defun buildbot-revision-open (revision)
@@ -429,8 +433,6 @@ With a non-nil FORCE, refresh the opened buffer if exists."
        (setf (alist-get 'log data)
              (get-text-property (point) 'log))
        (buildbot-view-open 'log data force)))))
-(define-key buildbot-view-mode-map (kbd "<return>")
-  #'buildbot-view-open-thing-at-point)
 
 (provide 'buildbot-view)
 ;;; buildbot-view.el ends here
diff --git a/buildbot.el b/buildbot.el
index 21c7c23..14e0e2d 100644
--- a/buildbot.el
+++ b/buildbot.el
@@ -46,8 +46,10 @@
 ;; available builders, and opens up a view of recent builds by this
 ;; builder.
 
+;;; Code:
 
 (require 'buildbot-client)
 (require 'buildbot-view)
 
 (provide 'buildbot)
+;;; buildbot.el ends here