Re: question regarding my emacs package

[Philip Kaludercic <philipk@posteo.net>]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

ram <chat@rj95.be> writes:

> hi, i am a professional programmer, but this is my first time
> submitting an emacs package. i have conversed a little with stefan
> monnier, and i believe the package i have meets the requirements for
> elpa submission, but i am unsure as to the appropriate licensure. i
> took some inspiration from two packages: dogears and gumshoe, neither
> of which is in elpa. however, i did not use any code from here; i just
> wanted some inspiration regarding the feature set

That should not be an issue, these packages appear to only be a source
of inspiration?

> it is unclear to me whether my package warrants inclusion in gnu or
> non gnu elpa, if at all, but i have certainly found it useful, despite
> its simplicity

Complexity is not a requirement for being useful :)

> here is the repo: https://github.com/gitrj95/breadcrumbs.el

Here are a few suggestions:


[-- Attachment #2: Type: text/plain, Size: 6620 bytes --]

diff -u /tmp/breadcrumbs.el.1 /tmp/breadcrumbs.el
--- /tmp/breadcrumbs.el.1	2023-06-07 17:32:50.162889593 +0200
+++ /tmp/breadcrumbs.el	2023-06-07 17:52:36.608268297 +0200
@@ -35,9 +35,9 @@
 ;; https://github.com/gitrj95/breadcrumbs.el
 ;;
 ;; In order to use breadcrumbs, `breadcrumbs-mode' must be enabled.
-;;
-;; Interface:
-;;
+
+;;; Interface:
+
 ;; `breadcrumbs-drop-breadcrumb' adds the current position in the
 ;; buffer to a ring.  If point is at a known breadcrumb, the existing
 ;; breadcrumb will be moved to the head of the ring.  Adding
@@ -59,9 +59,14 @@
 ;;; Code:
 
 (require 'ring)
-(require 'eieio)
+(require 'cl-lib)
 (require 'pulse)
 
+(defgroup breadcrumbs ()
+  "Track buffer positions."
+  :group 'convenience
+  :prefix "breadcrumbs-")
+
 (defvar breadcrumbs-ring nil
   "All dropped breadcrumbs, up to `breadcrumb-ring-max' breadcrumbs.")
 
@@ -81,44 +86,47 @@
 (defvar breadcrumbs-list-buffer nil
   "The \"*Breadcrumbs List*\" buffer.")
 
-(defclass breadcrumbs--breadcrumb ()
-  ((buffer-name
-    :initform (buffer-name)
-    :documentation "Name of the buffer.")
-   (buffer-position
-    :initform (point)
-    :documentation "Position in buffer."
-    )
-   (buffer-file-name
-    :initform (buffer-file-name)
-    :documentation "The full file path of this breadcrumb."))
-  "The breadcrumb definition.")
+(cl-defstruct breadcrumbs
+  "The breadcrumb definition."
+  (buffer-name
+   (buffer-name)
+   :documentation "Name of the buffer.")
+  (buffer-position
+   (point)
+   :documentation "Position in buffer.")
+  (buffer-file-name
+   (buffer-file-name)
+   :documentation "The full file path of this breadcrumb."))
 
 (defun breadcrumbs--setup ()
   "Set up the state required to start using breadcrumbs."
   (if (not breadcrumbs-ring)
       (setq breadcrumbs-ring (make-ring breadcrumb-ring-max)))
   (mapcar (lambda (fn)
-            (advice-add fn :around #'breadcrumbs--drop-around)) breadcrumbs-drop-around-fn-list))
+            (advice-add fn :around #'breadcrumbs--drop-around))
+	  breadcrumbs-drop-around-fn-list))
 
 (defun breadcrumbs--teardown ()
   "Tear down the state required for breadcrumbs."
   (setq breadcrumbs-ring nil)
   (setq breadcrumbs--neighbor nil)
   (mapcar (lambda (fn)
-            (advice-remove fn #'breadcrumbs--drop-around)) breadcrumbs-drop-around-fn-list))
+            (advice-remove fn #'breadcrumbs--drop-around))
+	  breadcrumbs-drop-around-fn-list))
 
 (defun breadcrumbs--jump (breadcrumb)
   "Jump to the specified breadcrumb."
   (setq breadcrumbs--neighbor breadcrumb)
-  (with-slots (buffer-name buffer-position buffer-file-name) breadcrumb
+  (let ((buffer-name (breadcrumbs-buffer-name breadcrumb))
+	(buffer-position (breadcrumbs-buffer-position breadcrumb))
+	(buffer-file-name (breadcrumbs-buffer-file-name breadcrumb)))
     (if (get-buffer buffer-name)
         (progn
           (switch-to-buffer buffer-name)
           (goto-char buffer-position))
       (find-file buffer-file-name)
       (goto-char buffer-position)
-      (setf buffer-name (buffer-name)))
+      (rename-buffer (buffer-name)))
     (pulse-momentary-highlight-one-line)))
 
 (defun breadcrumbs--drop ()
@@ -138,16 +146,18 @@
     (breadcrumbs--drop)
     result))
 
-(defun breadcrumbs--find-and-jump (&key direction)
+(defun breadcrumbs--find-and-jump (&rest args)
   "Find some candidate breadcrumb and jump to the next or previous, based on `direction'."
   (let ((jump-target
          (let ((candidate (make-instance 'breadcrumbs--breadcrumb)))
-           (if (ring-member breadcrumbs-ring candidate)
-               candidate))))
+           (and (ring-member breadcrumbs-ring candidate) candidate)))
+	(direction (plist-get args :type)))
     (cond (jump-target
            (breadcrumbs--jump
-            (cond ((eq direction 'next) (ring-next breadcrumbs-ring jump-target))
-                  ((eq direction 'previous) (ring-next breadcrumbs-ring jump-target)))))
+	    (ring-next
+	     (cond ((eq direction 'next) breadcrumbs-ring)
+                  ((eq direction 'previous) breadcrumbs-ring))
+	     jump-target)))
           (breadcrumbs--neighbor
            (if (eq direction 'previous)
                (breadcrumbs--jump breadcrumbs--neighbor))))))
@@ -156,14 +166,12 @@
 (define-minor-mode breadcrumbs-mode
   "Track positions in buffers and files using breadcrumbs."
   :global t
-  :init-value t
   (if breadcrumbs-mode
       (breadcrumbs--setup)
     (breadcrumbs--teardown)))
 
 (defvar breadcrumbs-list-mode-map
   (let ((map (make-sparse-keymap)))
-    (set-keymap-parent map tabulated-list-mode-map)
     (define-key map (kbd "j") #'breadcrumbs-list-jump)
     (define-key map (kbd "<RET>") #'breadcrumbs-list-jump)
     (define-key map (kbd "k") #'breadcrumbs-list-delete)
@@ -172,13 +180,14 @@
 
 (defun breadcrumbs--format-slot (slot len)
   "Formats a breadcrumbs slot."
-  (let ((format-string (format "%%-%ss" len)))
-    (format format-string
-            (if slot slot ""))))
+  (let ((format-string (format "%%-%ds" len)))
+    (format format-string (or slot ""))))
 
 (defun breadcrumbs--format-breadcrumb (breadcrumb)
   "Return a formatted breadcrumb as a vector of formatted slots."
-  (with-slots (buffer-name buffer-position buffer-file-name) breadcrumb
+  (let ((buffer-name (breadcrumbs-buffer-name breadcrumb))
+	(buffer-position (breadcrumbs-buffer-position breadcrumb))
+	(buffer-file-name (breadcrumbs-buffer-file-name breadcrumb)))
     (vector
      (breadcrumbs--format-slot buffer-name 16)
      (breadcrumbs--format-slot buffer-position 16)
@@ -199,13 +208,10 @@
 
 (define-derived-mode breadcrumbs-list-mode tabulated-list-mode
   "Tabular list mode displaying tracked breadcrumbs."
-  (setq-local tabulated-list-format
+  (setq-local tabulated-list-entries #'breadcrumbs-list--entries
+	      tabulated-list-format
               `[("Buffer" 16) ("Position" 16) ("File" 32)])
-  (add-hook 'tabulated-list-revert-hook
-            (lambda ()
-              (setf tabulated-list-entries (breadcrumbs-list--entries))))
-  (tabulated-list-init-header)
-  (breadcrumbs-list--revert))
+  (tabulated-list-init-header))
 
 (defun breadcrumbs-list-jump ()
   "Jump to breadcrumb from \"*Breadcrumbs List*\"."
@@ -217,8 +223,8 @@
   (interactive)
   (ring-remove breadcrumbs-ring
                (ring-member breadcrumbs-ring (tabulated-list-get-id)))
-  (if (ring-empty-p breadcrumbs-ring)
-      (setq breadcrumbs--neighbor nil))
+  (when (ring-empty-p breadcrumbs-ring)
+    (setq breadcrumbs--neighbor nil))
   (breadcrumbs-list--revert))
 
 ;;;###autoload

Diff finished.  Wed Jun  7 17:52:43 2023