Re: [ELPA] New package: Peek

[Philip Kaludercic]

Meow King <mr.meowking@anche.no> writes:

> I have created a new package ~Peek~, which allows users to create a peek
> view below/above cursor point to show things. 
> I think it may be useful to others, so I propose it to GNU ELPA.
>
> Package URL: https://git.sr.ht/~meow_king/peek

Here are a few comments:

diff --git a/peek.el b/peek.el
index 8e8e6f9..23c077c 100644
--- a/peek.el
+++ b/peek.el
@@ -48,6 +48,7 @@
 (require 'display-line-numbers)
 (require 'cl-lib)
 (require 'xref)
+(require 'subr-x)                      ;for `hash-table-keys'
 
 (defgroup peek nil
   "Peek mode."
@@ -56,47 +57,39 @@
 (defcustom peek-method 'overlay
   "Preferred method to display peek view."
   :type '(choice (const :tag "use overlay" overlay)
-                 (const :tag "use child frame" frame))
-  :group 'peek)
+                 (const :tag "use child frame" frame)))
 
 (defcustom peek-overlay-position 'above
   "Specify whether the overlay should be laid above the point or below the 
point."
   :type '(choice (const :tag "above the point" above)
-                 (const :tag "below the point" below))
-  :group 'peek)
+                 (const :tag "below the point" below)))
 
 (defcustom peek-overlay-distance 4
   "Number of the lines between the peek overlay window and the point.
 0 means directly above/below the current line."
-  :type 'natnum
-  :group 'peek)
+  :type 'natnum)
 
 (defcustom peek-overlay-border-symbol ?\N{BOX DRAWINGS LIGHT HORIZONTAL}
   "Specify symbol for peek overlay window border."
-  :type 'character
-  :group 'peek)
+  :type 'character)
 
 (defcustom peek-clean-dead-overlays-secs 3600
   "Every the given seconds to perform `peek-clean-dead-overlays' function."
-  :type 'natnum
-  :group 'peek)
+  :type 'natnum)
 
 (defcustom peek-overlay-window-size 11
   "Height of the peek overlay window.  A value of 0 may cause undefined 
behavior."
-  :type 'natnum
-  :group 'peek)
+  :type 'natnum)
 
 (defcustom peek-definition-surrounding-above-lines 1
   "Number of Lines above the xref definition to be shown in peek view.
 This value should be less than the
 `peek-overlay-window-size', otherwise undefined behavior."
-  :type 'natnum
-  :group 'peek)
+  :type 'natnum)
 
 (defcustom peek-live-update t
   "Whether to automatically update content when text in marked region changes."
-  :type 'boolean
-  :group 'peek)
+  :type 'boolean)
 
 (defcustom peek-mode-keymap
   (let ((map (make-sparse-keymap)))
@@ -105,8 +98,7 @@ This value should be less than the
     (define-key map (kbd "M-p") 'peek-prev-line)
     map)
   "Keymap used for peek mode."
-  :type 'keymap
-  :group 'typst)
+  :type 'keymap)
 
 (defface peek-overlay-border-face
   ;; '((((background light))
@@ -114,37 +106,32 @@ This value should be less than the
   ;;   (t
   ;;    :inherit font-lock-doc-face :foreground "#ecf0f1"))
   '((t (:inherit font-lock-doc-face)))
-  "Face for borders of peek overlay window."
-  :group 'peek)
+  "Face for borders of peek overlay window.")
 
 (defface peek-overlay-content-face
   '((((background light))
      :background "#ecf0f1" :extend t)
     (t
      :background "#95a5a6" :extend t))
-  "Additional face for content text of peek overlay window."
-  :group 'peek)
+  "Additional face for content text of peek overlay window.")
 
 (defcustom peek-enable-eldoc-message-integration nil
   "Show eldoc message on a peek view.
 Related function: `eldoc-message-function'."
-  :type 'boolean
-  :group 'peek)
+  :type 'boolean)
 
 (defcustom peek-enable-eldoc-display-integration nil
   "Show eldoc docs inside a peek view.
 Note you need Emacs version >= 28.1.
 Related function: `eldoc-display-functions'."
-  :type 'boolean
-  :group 'peek)
+  :type 'boolean)
 
 (defcustom peek-eldoc-message-overlay-position 2
   "Number of the lines between the peek eldoc message overlay window and the 
point.
 0 means directly above the current line.
 < 0 means above the current line;
 > 0 means below the current line."
-  :type 'integer
-  :group 'peek)
+  :type 'integer)
 
 (defvar-local peek-eldoc-message-overlay nil
   "Special overlay for handling eldoc message.
@@ -165,6 +152,9 @@ the `global-peek-mode'.")
   ;; (make-hash-table :test 'equal) ;; we need to manually set hash table for 
each buffer, otherwise we always change its global value
   "This variable shouldn't be customized by user.
 Variable structure: { window: overlay }.")
+;; Shouldn't you document the variable, even if users aren't supposed
+;; to use it.  Also, it is conventional to use a -- prefix for
+;; internal variables, e.g. `peek--window-overlay-map'.
 
 (defvar-local peek-live-update-associated-overlays nil
   "Associated overlays for this buffer.
@@ -288,6 +278,9 @@ WDW: window body width"
   (let* ((window-body-width (if wdw
                                 wdw
                               (window-body-width)))
+        ;; This approach seems to not work if the frame is resized.
+        ;; Perhaps you could take a similar approach like what
+        ;; `make-separator-line' does?
          (total-column-number (if (display-graphic-p)
                                   window-body-width
                                 ;; terminal Emacs will pad '\' at the line end
@@ -368,17 +361,15 @@ If WINDOW is nil, then show overlay in the current 
window."
   (let ((ol (peek-get-window-overlay window)))
     (peek-overlay--toggle-active ol)))
 
-(defun peek--regions-overlap (p1 p2 p3 p4)
+(defun peek--regions-overlap (r1s r1e r2s r2e)
   "Detect whether the two region are overlapped.
 The openness and closeness of the given regions should be the same as marker
 region.
-region1: P1, P2; region2: P3, P4
+region1: R1S, R1E; region2: R2S, R2E
 Return boolean."
-  (if (or (< p2 p1) (< p4 p3))
+  (if (or (< r1e r1s) (< r2e r2s))
       (error "P2 should >= p1, p4 should >= p3")
-    (if (or (<= p4 p1) (<= p2 p3))
-        nil
-      t)))
+    (not (or (<= r2e r1s) (<= r1e r2s)))))
 
 ;;; 
=============================================================================
 ;;; Main Functions
@@ -433,8 +424,7 @@ Return nil if there is no region."
            (me (make-marker)))
       (set-marker mb rb (current-buffer))
       (set-marker me re (current-buffer))
-      
-      (setq mark-active nil)  ; deactivate region mark
+      (deactivate-mark)
       (cons mb me))))
 
 (defun peek-overlay--get-supposed-position ()
@@ -511,8 +501,15 @@ Return position."
     (forward-line peek-eldoc-message-overlay-position)
     (point)))
 
+(declare-function peek-display-eldoc "peek" (docs interactive))
+
 ;;;###autoload
 (when (>= emacs-major-version 28)
+  ;; Are you sure you want to autoload the `when' expression?  Unless
+  ;; I am mistaken, that means that this entire expression will be
+  ;; copied into the autoload file, which can cause warnings due to
+  ;; `eldoc--format-doc-buffer' or `peek-get-or-create-window-overlay'
+  ;; not being defined.
   (defun peek-display-eldoc (docs interactive)
     "Related function: `eldoc-display-functions'.
 DOCS: docs passed by eldoc.
@@ -627,7 +624,7 @@ Only works when overlay is active/visible."
                           (string
                            (1- (length (overlay-get ol 'peek-lines))))
                           (definition
-                           1.0e+INF)  ; infinity
+                           1.0e+INF)  ; infinity (why?)
                           (t
                            (error "Invalid peek-type!")))))
     (overlay-put ol 'peek-offset (min (1+ offset) bound-max))

> ---
> More information:
>
> Some features of it:
> 1. Peek view follows your cursor.
> 2. Buffer and window local peek views. Also capable for content sharing 
> between different buffer and windows.
> 3. Store text of marked region, and then display it on the peek view.
> 4. Peek the destination of ~xref-find-definitions~.
> 5. ~eldoc-message-function~ and ~eldoc-display-functions~ integration.
> 6. Scroll up or down inside peek view.
> 7. live update

I tried it out briefly, but did not figure out how to disable the
"peek".  Perhaps you should rebind keyboard-quit to hide it?