question regarding my emacs package

question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

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

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

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

best,
ram
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmR/9tMJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AACtMAQDBJcq/+hW8lFC/JkXKSAsIjD5/dFF/jcjYjfRh3o0TMwD9Eb4W
f0DKtY6/QCgyoPMLsMIBUf1cDZ1c53rQ9xHWCAI=
=5pUI
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[Philip Kaludercic]

[-- 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

Re: question regarding my emacs package

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

>> here is the repo: https://github.com/gitrj95/breadcrumbs.el
>
> Here are a few suggestions:

Oh, and can you address the issue raised by checkdoc?

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

changing the central type from defclass to cl-defstruct has appeared to cause bugs? i am not sure what to make of this, but find-file and its variants do not appear to work when changing only this. the file is not opened; it is not that the file is opened but the active buffer isn't switched




------- Original Message -------
On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic <philipk@posteo.net> wrote:


> 
> 
> Philip Kaludercic philipk@posteo.net writes:
> 
> > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
> > 
> > Here are a few suggestions:
> 
> 
> Oh, and can you address the issue raised by checkdoc?
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSA3l4JkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AACphAP950X8wqpTmrh9i++73rNXdcW0ZDL8q7W9n2Jy7HaOvGAEA+a96
jQwzmJeyUo9r5Ot7U0US48Ty3tR3WVYXhKbVBA4=
=l3rq
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[Philip Kaludercic]

ram <chat@rj95.be> writes:

> changing the central type from defclass to cl-defstruct has appeared
> to cause bugs? 

That might have been a typo on my end, I did not evaluate the code and
was is a hurry to send you the diff.  My point is that you don't need
defclass, unless I am missing something in your code (and I don't think
the minimal convenience of `with-slots' warrants classes here).

>                i am not sure what to make of this, but find-file and
> its variants do not appear to work when changing only this. the file
> is not opened; it is not that the file is opened but the active buffer
> isn't switched
>
>
>
>
> ------- Original Message -------
> On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic <philipk@posteo.net> wrote:
>
>
>> 
>> 
>> Philip Kaludercic philipk@posteo.net writes:
>> 
>> > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
>> > 
>> > Here are a few suggestions:
>> 
>> 
>> Oh, and can you address the issue raised by checkdoc?

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

yes i figured; i found other bugs as well, but i don't think this is a typo. i'll dig a bit more and see what i can find




------- Original Message -------
On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic <philipk@posteo.net> wrote:


> 
> 
> ram chat@rj95.be writes:
> 
> > changing the central type from defclass to cl-defstruct has appeared
> > to cause bugs?
> 
> 
> That might have been a typo on my end, I did not evaluate the code and
> was is a hurry to send you the diff. My point is that you don't need
> defclass, unless I am missing something in your code (and I don't think
> the minimal convenience of `with-slots' warrants classes here).
> 
> > i am not sure what to make of this, but find-file and
> > its variants do not appear to work when changing only this. the file
> > is not opened; it is not that the file is opened but the active buffer
> > isn't switched
> > 
> > ------- Original Message -------
> > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
> > 
> > > Philip Kaludercic philipk@posteo.net writes:
> > > 
> > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
> > > > 
> > > > Here are a few suggestions:
> > > 
> > > Oh, and can you address the issue raised by checkdoc?
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSA34MJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AAIp0AP4nHhxZ7nig6chOhVCeoRyQtB74N2HJcrU+rW6wCRMyfQEA2QjV
ujOgZia8Pf2Z7ceYxpgIYuk01VdpJiCfuEf7rQA=
=9NLc
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

just got back to this. calling find-file appears to not open the new file when called through the code. i'm not immediately sure what the problem is. any tips? i'll look more into this tonight




------- Original Message -------
On Wednesday, June 7th, 2023 at 3:50 PM, ram <chat@rj95.be> wrote:


> 
> 
> yes i figured; i found other bugs as well, but i don't think this is a typo. i'll dig a bit more and see what i can find
> 
> 
> 
> 
> ------- Original Message -------
> On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic philipk@posteo.net wrote:
> 
> 
> 
> > ram chat@rj95.be writes:
> > 
> > > changing the central type from defclass to cl-defstruct has appeared
> > > to cause bugs?
> > 
> > That might have been a typo on my end, I did not evaluate the code and
> > was is a hurry to send you the diff. My point is that you don't need
> > defclass, unless I am missing something in your code (and I don't think
> > the minimal convenience of `with-slots' warrants classes here).
> > 
> > > i am not sure what to make of this, but find-file and
> > > its variants do not appear to work when changing only this. the file
> > > is not opened; it is not that the file is opened but the active buffer
> > > isn't switched
> > > 
> > > ------- Original Message -------
> > > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
> > > 
> > > > Philip Kaludercic philipk@posteo.net writes:
> > > > 
> > > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
> > > > > 
> > > > > Here are a few suggestions:
> > > > 
> > > > Oh, and can you address the issue raised by checkdoc?
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSBBcgJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AANtWAQCd8Vm3sPsdMC8CpRsau20/7znzgExI97Ua1nQmT0y+VwEA96k+
rIlLZ/T1lf0cAd6l6mhKNoo8DOwY9ZmeCvFszA8=
=DrGv
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

i believe i've incorporated your suggestions, fixed the bugs, and satisfied check doc. let me know what else i need to do, eg fsf copyright notice, etc


------- Original Message -------
On Wednesday, June 7th, 2023 at 6:34 PM, ram <chat@rj95.be> wrote:


> 
> 
> just got back to this. calling find-file appears to not open the new file when called through the code. i'm not immediately sure what the problem is. any tips? i'll look more into this tonight
> 
> 
> 
> 
> ------- Original Message -------
> On Wednesday, June 7th, 2023 at 3:50 PM, ram chat@rj95.be wrote:
> 
> 
> 
> > yes i figured; i found other bugs as well, but i don't think this is a typo. i'll dig a bit more and see what i can find
> > 
> > ------- Original Message -------
> > On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic philipk@posteo.net wrote:
> > 
> > > ram chat@rj95.be writes:
> > > 
> > > > changing the central type from defclass to cl-defstruct has appeared
> > > > to cause bugs?
> > > 
> > > That might have been a typo on my end, I did not evaluate the code and
> > > was is a hurry to send you the diff. My point is that you don't need
> > > defclass, unless I am missing something in your code (and I don't think
> > > the minimal convenience of `with-slots' warrants classes here).
> > > 
> > > > i am not sure what to make of this, but find-file and
> > > > its variants do not appear to work when changing only this. the file
> > > > is not opened; it is not that the file is opened but the active buffer
> > > > isn't switched
> > > > 
> > > > ------- Original Message -------
> > > > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
> > > > 
> > > > > Philip Kaludercic philipk@posteo.net writes:
> > > > > 
> > > > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
> > > > > > 
> > > > > > Here are a few suggestions:
> > > > > 
> > > > > Oh, and can you address the issue raised by checkdoc?
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSBf4wJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AAPC/AQCH6eZ0EchnqBUeV2NFrw6aEUQ4si0H9efYCCtEGleTuwD9F12K
Q5BwPbelBJqZaU0WtYHltPZWi82pyaJzyhmyeQc=
=t1tP
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[Philip Kaludercic]

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

ram <chat@rj95.be> writes:

> i believe i've incorporated your suggestions, fixed the bugs, and satisfied check doc. let me know what else i need to do, eg fsf copyright notice, etc

Here are a few more things I noticed:


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

diff -u /tmp/breadcrumbs.el.1 /tmp/breadcrumbs.el
--- /tmp/breadcrumbs.el.1	2023-06-08 09:20:18.490560968 +0200
+++ /tmp/breadcrumbs.el	2023-06-08 09:23:13.217684032 +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
@@ -103,28 +103,26 @@
   "Set up the state required to start using breadcrumbs."
   (unless 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))
+  (dolist (fn breadcrumbs-drop-around-fn-list)
+    (advice-add fn :around #'breadcrumbs--drop-around)))
 
 (defun breadcrumbs--teardown ()
   "Tear down the state required for breadcrumbs."
   (setq breadcrumbs-ring nil
         breadcrumbs--neighbor nil)
-  (mapcar (lambda (fn)
-            (advice-remove fn #'breadcrumbs--drop-around))
-          breadcrumbs-drop-around-fn-list))
+  (dolist (fn breadcrumbs-drop-around-fn-list)
+    (advice-remove fn #'breadcrumbs--drop-around)))
 
 (defun breadcrumbs--switch-to-fileless-buffer (breadcrumb)
-  "Switch to BREADCRUMB's `fileless-buffer-name' if it is non-nil and optionally remove dead ones."
+  "Switch to BREADCRUMB's `fileless-buffer-name' if it is non-nil.
+Optionally remove dead ones."
   (if-let* ((buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb))
             (buffer (get-buffer buffer-name)))
       (switch-to-buffer buffer)
     (when (yes-or-no-p (format "%s has been killed.  Remove from all instances from `breadcrumbs-ring'? " buffer-name))
-      (mapcar (lambda (breadcrumb-to-remove)
-                (when (equal buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove))
-                  (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring breadcrumb-to-remove))))
-              (ring-elements breadcrumbs-ring))
+      (dolist (breadcrumb-to-remove (ring-elements breadcrumbs-ring))
+	(when (equal buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove))
+          (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring breadcrumb-to-remove))))
       (breadcrumbs-list--revert)
       nil)))
 
@@ -170,7 +168,7 @@
                   ((eq direction 'previous) (ring-next breadcrumbs-ring jump-target)))))
           (breadcrumbs--neighbor
            (when (eq direction 'previous)
-               (breadcrumbs--jump breadcrumbs--neighbor))))))
+             (breadcrumbs--jump breadcrumbs--neighbor))))))
 
 ;;;###autoload
 (define-minor-mode breadcrumbs-mode
@@ -195,9 +193,9 @@
 (defun breadcrumbs--format-breadcrumb (breadcrumb)
   "Return BREADCRUMB's formatted slots as a vector."
   (let* ((buffer-file-name (breadcrumbs--breadcrumb-buffer-file-name breadcrumb))
-	(buffer-position (breadcrumbs--breadcrumb-buffer-position breadcrumb))
-        (buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb))
-        (breadcrumb-name (or buffer-file-name buffer-name)))
+	 (buffer-position (breadcrumbs--breadcrumb-buffer-position breadcrumb))
+         (buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb))
+         (breadcrumb-name (or buffer-file-name buffer-name)))
     (vector
      (breadcrumbs--format-slot breadcrumb-name 64)
      (breadcrumbs--format-slot buffer-position 16))))
@@ -207,7 +205,8 @@
   (mapcar (lambda (breadcrumb)
             (list
              breadcrumb
-             (breadcrumbs--format-breadcrumb breadcrumb))) (ring-elements breadcrumbs-ring)))
+             (breadcrumbs--format-breadcrumb breadcrumb)))
+	  (ring-elements breadcrumbs-ring)))
 
 (defun breadcrumbs-list--revert ()
   "Reverts `breadcrumbs-list-buffer'."
@@ -232,7 +231,7 @@
   (ring-remove breadcrumbs-ring
                (ring-member breadcrumbs-ring (tabulated-list-get-id)))
   (when (ring-empty-p breadcrumbs-ring)
-      (setq breadcrumbs--neighbor nil))
+    (setq breadcrumbs--neighbor nil))
   (breadcrumbs-list--revert))
 
 ;;;###autoload

Diff finished.  Thu Jun  8 09:24:02 2023

[-- Attachment #3: Type: text/plain, Size: 2267 bytes --]


The main things were inconsistent indentation, using mapcar for
side-effects (if anything mapc should be used in that case, but I think
dolist is the best choice) and there still was one checkdoc complaint.
On the topic of docstrings, I think you should invest some more time to
make them understandable to someone who doesn't already know what the
code is about.

Have you signed the FSF copyright assignment?

> ------- Original Message -------
> On Wednesday, June 7th, 2023 at 6:34 PM, ram <chat@rj95.be> wrote:
>
>
>> 
>> 
>> just got back to this. calling find-file appears to not open the new
>> file when called through the code. i'm not immediately sure what the
>> problem is. any tips? i'll look more into this tonight
>> 
>> 
>> 
>> 
>> ------- Original Message -------
>> On Wednesday, June 7th, 2023 at 3:50 PM, ram chat@rj95.be wrote:
>> 
>> 
>> 
>> > yes i figured; i found other bugs as well, but i don't think this is a typo. i'll dig a bit more and see what i can find
>> > 
>> > ------- Original Message -------
>> > On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic philipk@posteo.net wrote:
>> > 
>> > > ram chat@rj95.be writes:
>> > > 
>> > > > changing the central type from defclass to cl-defstruct has appeared
>> > > > to cause bugs?
>> > > 
>> > > That might have been a typo on my end, I did not evaluate the code and
>> > > was is a hurry to send you the diff. My point is that you don't need
>> > > defclass, unless I am missing something in your code (and I don't think
>> > > the minimal convenience of `with-slots' warrants classes here).
>> > > 
>> > > > i am not sure what to make of this, but find-file and
>> > > > its variants do not appear to work when changing only this. the file
>> > > > is not opened; it is not that the file is opened but the active buffer
>> > > > isn't switched
>> > > > 
>> > > > ------- Original Message -------
>> > > > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
>> > > > 
>> > > > > Philip Kaludercic philipk@posteo.net writes:
>> > > > > 
>> > > > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
>> > > > > > 
>> > > > > > Here are a few suggestions:
>> > > > > 
>> > > > > Oh, and can you address the issue raised by checkdoc?

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

thanks for the feedback. i've incorporated your suggestions. i believe i cleared check doc locally; please let me know otherwise

i have not signed the fsf copyright assignment. what does this process entail?

------- Original Message -------
On Thursday, June 8th, 2023 at 3:26 AM, Philip Kaludercic <philipk@posteo.net> wrote:


> 
> 
> ram chat@rj95.be writes:
> 
> > i believe i've incorporated your suggestions, fixed the bugs, and satisfied check doc. let me know what else i need to do, eg fsf copyright notice, etc
> 
> 
> Here are a few more things I noticed:
> 
> diff -u /tmp/breadcrumbs.el.1 /tmp/breadcrumbs.el
> --- /tmp/breadcrumbs.el.1 2023-06-08 09:20:18.490560968 +0200
> +++ /tmp/breadcrumbs.el 2023-06-08 09:23:13.217684032 +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
> @@ -103,28 +103,26 @@
> "Set up the state required to start using breadcrumbs."
> (unless 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))
> + (dolist (fn breadcrumbs-drop-around-fn-list)
> + (advice-add fn :around #'breadcrumbs--drop-around)))
> 
> (defun breadcrumbs--teardown ()
> "Tear down the state required for breadcrumbs."
> (setq breadcrumbs-ring nil
> breadcrumbs--neighbor nil)
> - (mapcar (lambda (fn)
> - (advice-remove fn #'breadcrumbs--drop-around))
> - breadcrumbs-drop-around-fn-list))
> + (dolist (fn breadcrumbs-drop-around-fn-list)
> + (advice-remove fn #'breadcrumbs--drop-around)))
> 
> (defun breadcrumbs--switch-to-fileless-buffer (breadcrumb)
> - "Switch to BREADCRUMB's `fileless-buffer-name' if it is non-nil and optionally remove dead ones." + "Switch to BREADCRUMB's` fileless-buffer-name' if it is non-nil.
> +Optionally remove dead ones."
> (if-let* ((buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb))
> (buffer (get-buffer buffer-name)))
> (switch-to-buffer buffer)
> (when (yes-or-no-p (format "%s has been killed. Remove from all instances from `breadcrumbs-ring'? " buffer-name)) - (mapcar (lambda (breadcrumb-to-remove) - (when (equal buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove)) - (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring breadcrumb-to-remove)))) - (ring-elements breadcrumbs-ring)) + (dolist (breadcrumb-to-remove (ring-elements breadcrumbs-ring)) + (when (equal buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove)) + (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring breadcrumb-to-remove)))) (breadcrumbs-list--revert) nil))) @@ -170,7 +168,7 @@ ((eq direction 'previous) (ring-next breadcrumbs-ring jump-target))))) (breadcrumbs--neighbor (when (eq direction 'previous) - (breadcrumbs--jump breadcrumbs--neighbor)))))) + (breadcrumbs--jump breadcrumbs--neighbor)))))) ;;;###autoload (define-minor-mode breadcrumbs-mode @@ -195,9 +193,9 @@ (defun breadcrumbs--format-breadcrumb (breadcrumb) "Return BREADCRUMB's formatted slots as a vector." (let* ((buffer-file-name (breadcrumbs--breadcrumb-buffer-file-name breadcrumb)) - (buffer-position (breadcrumbs--breadcrumb-buffer-position breadcrumb)) - (buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb)) - (breadcrumb-name (or buffer-file-name buffer-name))) + (buffer-position (breadcrumbs--breadcrumb-buffer-position breadcrumb)) + (buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb)) + (breadcrumb-name (or buffer-file-name buffer-name))) (vector (breadcrumbs--format-slot breadcrumb-name 64) (breadcrumbs--format-slot buffer-position 16)))) @@ -207,7 +205,8 @@ (mapcar (lambda (breadcrumb) (list breadcrumb - (breadcrumbs--format-breadcrumb breadcrumb))) (ring-elements breadcrumbs-ring))) + (breadcrumbs--format-breadcrumb breadcrumb))) + (ring-elements breadcrumbs-ring))) (defun breadcrumbs-list--revert () "Reverts` breadcrumbs-list-buffer'."
> @@ -232,7 +231,7 @@
> (ring-remove breadcrumbs-ring
> (ring-member breadcrumbs-ring (tabulated-list-get-id)))
> (when (ring-empty-p breadcrumbs-ring)
> - (setq breadcrumbs--neighbor nil))
> + (setq breadcrumbs--neighbor nil))
> (breadcrumbs-list--revert))
> 
> ;;;###autoload
> 
> Diff finished. Thu Jun 8 09:24:02 2023
> 
> The main things were inconsistent indentation, using mapcar for
> side-effects (if anything mapc should be used in that case, but I think
> dolist is the best choice) and there still was one checkdoc complaint.
> On the topic of docstrings, I think you should invest some more time to
> make them understandable to someone who doesn't already know what the
> code is about.
> 
> Have you signed the FSF copyright assignment?
> 
> > ------- Original Message -------
> > On Wednesday, June 7th, 2023 at 6:34 PM, ram chat@rj95.be wrote:
> > 
> > > just got back to this. calling find-file appears to not open the new
> > > file when called through the code. i'm not immediately sure what the
> > > problem is. any tips? i'll look more into this tonight
> > > 
> > > ------- Original Message -------
> > > On Wednesday, June 7th, 2023 at 3:50 PM, ram chat@rj95.be wrote:
> > > 
> > > > yes i figured; i found other bugs as well, but i don't think this is a typo. i'll dig a bit more and see what i can find
> > > > 
> > > > ------- Original Message -------
> > > > On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic philipk@posteo.net wrote:
> > > > 
> > > > > ram chat@rj95.be writes:
> > > > > 
> > > > > > changing the central type from defclass to cl-defstruct has appeared
> > > > > > to cause bugs?
> > > > > 
> > > > > That might have been a typo on my end, I did not evaluate the code and
> > > > > was is a hurry to send you the diff. My point is that you don't need
> > > > > defclass, unless I am missing something in your code (and I don't think
> > > > > the minimal convenience of `with-slots' warrants classes here).
> > > > > 
> > > > > > i am not sure what to make of this, but find-file and
> > > > > > its variants do not appear to work when changing only this. the file
> > > > > > is not opened; it is not that the file is opened but the active buffer
> > > > > > isn't switched
> > > > > > 
> > > > > > ------- Original Message -------
> > > > > > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
> > > > > > 
> > > > > > > Philip Kaludercic philipk@posteo.net writes:
> > > > > > > 
> > > > > > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
> > > > > > > > 
> > > > > > > > Here are a few suggestions:
> > > > > > > 
> > > > > > > Oh, and can you address the issue raised by checkdoc?
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSCItwJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AALMUAP9Rpdg6E3becVvQmjcCV9ZkEyBMW+PCe6H0wbbHWbP58QEAwDsJ
/OZQGzUxPBHnA5X4kyxY+c9uEHxqylY31HF5MQA=
=kyeV
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[Philip Kaludercic]

ram <chat@rj95.be> writes:

> thanks for the feedback. i've incorporated your suggestions. i believe
> i cleared check doc locally; please let me know otherwise

There still appear to be a few minor issues.  Do you use Flymake?  If
not, try out M-x flymake-mode and it should highlight the byte-compiler
and checkdoc issues.  None of the issues appear to be blocking
inclusion though.

> i have not signed the fsf copyright assignment. what does this process entail?

You have to request the copyright assignment form here on emacs-devel (I
have CC'ed a maintainer) and then sign the form you will receive.

> ------- Original Message -------
> On Thursday, June 8th, 2023 at 3:26 AM, Philip Kaludercic <philipk@posteo.net> wrote:
>
>
>> 
>> 
>> ram chat@rj95.be writes:
>> 
>> > i believe i've incorporated your suggestions, fixed the bugs, and
>> > satisfied check doc. let me know what else i need to do, eg fsf
>> > copyright notice, etc
>> 
>> 
>> Here are a few more things I noticed:
>> 
>> diff -u /tmp/breadcrumbs.el.1 /tmp/breadcrumbs.el
>> --- /tmp/breadcrumbs.el.1 2023-06-08 09:20:18.490560968 +0200
>> +++ /tmp/breadcrumbs.el 2023-06-08 09:23:13.217684032 +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
>> @@ -103,28 +103,26 @@
>> "Set up the state required to start using breadcrumbs."
>> (unless 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))
>> + (dolist (fn breadcrumbs-drop-around-fn-list)
>> + (advice-add fn :around #'breadcrumbs--drop-around)))
>> 
>> (defun breadcrumbs--teardown ()
>> "Tear down the state required for breadcrumbs."
>> (setq breadcrumbs-ring nil
>> breadcrumbs--neighbor nil)
>> - (mapcar (lambda (fn)
>> - (advice-remove fn #'breadcrumbs--drop-around))
>> - breadcrumbs-drop-around-fn-list))
>> + (dolist (fn breadcrumbs-drop-around-fn-list)
>> + (advice-remove fn #'breadcrumbs--drop-around)))
>> 
>> (defun breadcrumbs--switch-to-fileless-buffer (breadcrumb)
>> - "Switch to BREADCRUMB's `fileless-buffer-name' if it is non-nil
>> and optionally remove dead ones." + "Switch to BREADCRUMB's`
>> fileless-buffer-name' if it is non-nil.
>> +Optionally remove dead ones."
>> (if-let* ((buffer-name (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb))
>> (buffer (get-buffer buffer-name)))
>> (switch-to-buffer buffer)
>> (when (yes-or-no-p (format "%s has been killed. Remove from all
>> instances from `breadcrumbs-ring'? " buffer-name)) - (mapcar (lambda
>> (breadcrumb-to-remove) - (when (equal buffer-name
>> (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove))
>> - (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring
>> breadcrumb-to-remove)))) - (ring-elements breadcrumbs-ring)) +
>> (dolist (breadcrumb-to-remove (ring-elements breadcrumbs-ring)) +
>> (when (equal buffer-name
>> (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb-to-remove))
>> + (ring-remove breadcrumbs-ring (ring-member breadcrumbs-ring
>> breadcrumb-to-remove)))) (breadcrumbs-list--revert) nil))) @@ -170,7
>> +168,7 @@ ((eq direction 'previous) (ring-next breadcrumbs-ring
>> jump-target))))) (breadcrumbs--neighbor (when (eq direction
>> 'previous) - (breadcrumbs--jump breadcrumbs--neighbor)))))) +
>> (breadcrumbs--jump breadcrumbs--neighbor)))))) ;;;###autoload
>> (define-minor-mode breadcrumbs-mode @@ -195,9 +193,9 @@ (defun
>> breadcrumbs--format-breadcrumb (breadcrumb) "Return BREADCRUMB's
>> formatted slots as a vector." (let* ((buffer-file-name
>> (breadcrumbs--breadcrumb-buffer-file-name breadcrumb)) -
>> (buffer-position (breadcrumbs--breadcrumb-buffer-position
>> breadcrumb)) - (buffer-name
>> (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb)) -
>> (breadcrumb-name (or buffer-file-name buffer-name))) +
>> (buffer-position (breadcrumbs--breadcrumb-buffer-position
>> breadcrumb)) + (buffer-name
>> (breadcrumbs--breadcrumb-fileless-buffer-name breadcrumb)) +
>> (breadcrumb-name (or buffer-file-name buffer-name))) (vector
>> (breadcrumbs--format-slot breadcrumb-name 64)
>> (breadcrumbs--format-slot buffer-position 16)))) @@ -207,7 +205,8 @@
>> (mapcar (lambda (breadcrumb) (list breadcrumb -
>> (breadcrumbs--format-breadcrumb breadcrumb))) (ring-elements
>> breadcrumbs-ring))) + (breadcrumbs--format-breadcrumb breadcrumb)))
>> + (ring-elements breadcrumbs-ring))) (defun breadcrumbs-list--revert
>> () "Reverts` breadcrumbs-list-buffer'."
>> @@ -232,7 +231,7 @@
>> (ring-remove breadcrumbs-ring
>> (ring-member breadcrumbs-ring (tabulated-list-get-id)))
>> (when (ring-empty-p breadcrumbs-ring)
>> - (setq breadcrumbs--neighbor nil))
>> + (setq breadcrumbs--neighbor nil))
>> (breadcrumbs-list--revert))
>> 
>> ;;;###autoload
>> 
>> Diff finished. Thu Jun 8 09:24:02 2023
>> 
>> The main things were inconsistent indentation, using mapcar for
>> side-effects (if anything mapc should be used in that case, but I think
>> dolist is the best choice) and there still was one checkdoc complaint.
>> On the topic of docstrings, I think you should invest some more time to
>> make them understandable to someone who doesn't already know what the
>> code is about.
>> 
>> Have you signed the FSF copyright assignment?
>> 
>> > ------- Original Message -------
>> > On Wednesday, June 7th, 2023 at 6:34 PM, ram chat@rj95.be wrote:
>> > 
>> > > just got back to this. calling find-file appears to not open the new
>> > > file when called through the code. i'm not immediately sure what the
>> > > problem is. any tips? i'll look more into this tonight
>> > > 
>> > > ------- Original Message -------
>> > > On Wednesday, June 7th, 2023 at 3:50 PM, ram chat@rj95.be wrote:
>> > > 
>> > > > yes i figured; i found other bugs as well, but i don't think
>> > > > this is a typo. i'll dig a bit more and see what i can find
>> > > > 
>> > > > ------- Original Message -------
>> > > > On Wednesday, June 7th, 2023 at 3:48 PM, Philip Kaludercic philipk@posteo.net wrote:
>> > > > 
>> > > > > ram chat@rj95.be writes:
>> > > > > 
>> > > > > > changing the central type from defclass to cl-defstruct has appeared
>> > > > > > to cause bugs?
>> > > > > 
>> > > > > That might have been a typo on my end, I did not evaluate the code and
>> > > > > was is a hurry to send you the diff. My point is that you don't need
>> > > > > defclass, unless I am missing something in your code (and I don't think
>> > > > > the minimal convenience of `with-slots' warrants classes here).
>> > > > > 
>> > > > > > i am not sure what to make of this, but find-file and
>> > > > > > its variants do not appear to work when changing only this. the file
>> > > > > > is not opened; it is not that the file is opened but the active buffer
>> > > > > > isn't switched
>> > > > > > 
>> > > > > > ------- Original Message -------
>> > > > > > On Wednesday, June 7th, 2023 at 2:04 PM, Philip Kaludercic philipk@posteo.net wrote:
>> > > > > > 
>> > > > > > > Philip Kaludercic philipk@posteo.net writes:
>> > > > > > > 
>> > > > > > > > > here is the repo: https://github.com/gitrj95/breadcrumbs.el
>> > > > > > > > 
>> > > > > > > > Here are a few suggestions:
>> > > > > > > 
>> > > > > > > Oh, and can you address the issue raised by checkdoc?
>

-- 
Philip Kaludercic

Re: question regarding my emacs package

[Madhu]

[Question about cl-defstruct]

* Philip Kaludercic <877csetnwy.fsf @posteo.net> :
Wrote on Thu, 08 Jun 2023 07:26:37 +0000:
> ram <chat @rj95.be> writes:
>
>> i believe i've incorporated your suggestions, fixed the bugs, and
>> satisfied check doc. let me know what else i need to do, eg fsf
>> copyright notice, etc
>
> Here are a few more things I noticed:
>  ;; `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

[snip]

I tried loading the file from github on emacs master and it fails when
trying to compile

```
(defun breadcrumbs--drop ()
  "Track the buffer position as a `breadcrumbs--breadcrumb'.

If this has already been tracked, move an existing one in `breadcrumbs-ring' to head."
  (let* ((breadcrumb (make-breadcrumbs--breadcrumb))
         (index (ring-member breadcrumbs-ring breadcrumb)))
```

with
```
Debugger entered--Lisp error: (wrong-type-argument stringp (buffer-file-name))
  make-breadcrumbs--breadcrumb--cmacro((make-breadcrumbs--breadcrumb))
  apply(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb) nil)
  macroexp--compiler-macro(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb))
```

The cl-defstruct slot has an initform "(buffer-file-name)" which seems
legit since it takes an optional argument.

However when I change the form to (buffer-file-name nil) as below, it
loads


```
(cl-defstruct breadcrumbs--breadcrumb
  (buffer-file-name
   (buffer-file-name nil)
   :documentation "The file backing the breadcrumb.")
```

Common Lisp has a problem with the interpretation of the lexical
scoping of the initforms. C-h f cl-defstruct doesn't impose any
conditions on the SDEFAULT form.  Are there any guarantees or
limitations on the forms that can go into SDEFAULT

Re: question regarding my emacs package

[Madhu]

* Madhu <m37csacjku.fsf@leonis4.robolove.meer.net> :
Wrote on Sun, 11 Jun 2023 17:11:21 +0530:

> [Question about cl-defstruct]
>
>
> I tried loading the file from github on emacs master and it fails when
> trying to compile
>
> ```
> (defun breadcrumbs--drop ()
>   "Track the buffer position as a `breadcrumbs--breadcrumb'.
>
> If this has already been tracked, move an existing one in `breadcrumbs-ring' to head."
>   (let* ((breadcrumb (make-breadcrumbs--breadcrumb))
>          (index (ring-member breadcrumbs-ring breadcrumb)))
> ```
>
>
> with
> ```
> Debugger entered--Lisp error: (wrong-type-argument stringp (buffer-file-name))
>   make-breadcrumbs--breadcrumb--cmacro((make-breadcrumbs--breadcrumb))
>   apply(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb) nil)
>   macroexp--compiler-macro(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb))
> ```
>
> The cl-defstruct slot has an initform "(buffer-file-name)" which seems
> legit since it takes an optional argument.
>
> However when I change the form to (buffer-file-name nil) as below, it
> loads
>
> ```
> (cl-defstruct breadcrumbs--breadcrumb
>   (buffer-file-name
>    (buffer-file-name nil)
>    :documentation "The file backing the breadcrumb.")
> ```


So with my emacs-30 nativecomp this just doesnt work. the
buffer-file-name slot of the cl-defstruct has to be changed to
buffer-file-name-1 (and the corresponding accessors to
breadcrumbs--breadcrumb-buffer-file-name-1) throughout the file.

Do others not see this limitation in the compiler?


> Common Lisp has a problem with the interpretation of the lexical
> scoping of the initforms. C-h f cl-defstruct doesn't impose any
> conditions on the SDEFAULT form.  Are there any guarantees or
> limitations on the forms that can go into SDEFAULT

Re: question regarding my emacs package

[Andrea Corallo]

Madhu <enometh@meer.net> writes:

> * Madhu <m37csacjku.fsf@leonis4.robolove.meer.net> :
> Wrote on Sun, 11 Jun 2023 17:11:21 +0530:
>
>> [Question about cl-defstruct]
>>
>>
>> I tried loading the file from github on emacs master and it fails when
>> trying to compile
>>
>> ```
>> (defun breadcrumbs--drop ()
>>   "Track the buffer position as a `breadcrumbs--breadcrumb'.
>>
>> If this has already been tracked, move an existing one in `breadcrumbs-ring' to head."
>>   (let* ((breadcrumb (make-breadcrumbs--breadcrumb))
>>          (index (ring-member breadcrumbs-ring breadcrumb)))
>> ```
>>
>>
>> with
>> ```
>> Debugger entered--Lisp error: (wrong-type-argument stringp (buffer-file-name))
>>   make-breadcrumbs--breadcrumb--cmacro((make-breadcrumbs--breadcrumb))
>>   apply(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb) nil)
>>   macroexp--compiler-macro(make-breadcrumbs--breadcrumb--cmacro (make-breadcrumbs--breadcrumb))
>> ```
>>
>> The cl-defstruct slot has an initform "(buffer-file-name)" which seems
>> legit since it takes an optional argument.
>>
>> However when I change the form to (buffer-file-name nil) as below, it
>> loads
>>
>> ```
>> (cl-defstruct breadcrumbs--breadcrumb
>>   (buffer-file-name
>>    (buffer-file-name nil)
>>    :documentation "The file backing the breadcrumb.")
>> ```
>
>
> So with my emacs-30 nativecomp this just doesnt work. the
> buffer-file-name slot of the cl-defstruct has to be changed to
> buffer-file-name-1 (and the corresponding accessors to
> breadcrumbs--breadcrumb-buffer-file-name-1) throughout the file.

Does it works on the same Emacs codebase without native compilation?

Thanks

  Andrea

Re: question regarding my emacs package

[Madhu]

*  Andrea Corallo <yp1mt148wh4.fsf @fencepost.gnu.org>
Wrote on Mon, 12 Jun 2023 12:40:55 -0400
> Madhu <enometh@meer.net> writes:
>>
>> So with my emacs-30 nativecomp this just doesnt work. the
>> buffer-file-name slot of the cl-defstruct has to be changed to
>> buffer-file-name-1 (and the corresponding accessors to
>> breadcrumbs--breadcrumb-buffer-file-name-1) throughout the file.
>
> Does it works on the same Emacs codebase without native compilation?

Yes, it looks like native-comp wont let you use buffer-file-name as a
cl-defstruct slot name if you also have an initializer (SDEFAULT)
form.  it gets confused and tries to use the elisp function
definition.

The following works on non-native comp but fails on native-comp.

```
(require 'cl-lib)
(defun xyz ())
(cl-defstruct barf
  (buffer-file-name (xyz)))

(defun barf-foo ()
  (let ((barf (make-barf)))))
```

Re: question regarding my emacs package

[Andrea Corallo]

Madhu <enometh@meer.net> writes:

> *  Andrea Corallo <yp1mt148wh4.fsf @fencepost.gnu.org>
> Wrote on Mon, 12 Jun 2023 12:40:55 -0400
>> Madhu <enometh@meer.net> writes:
>>>
>>> So with my emacs-30 nativecomp this just doesnt work. the
>>> buffer-file-name slot of the cl-defstruct has to be changed to
>>> buffer-file-name-1 (and the corresponding accessors to
>>> breadcrumbs--breadcrumb-buffer-file-name-1) throughout the file.
>>
>> Does it works on the same Emacs codebase without native compilation?
>
> Yes, it looks like native-comp wont let you use buffer-file-name as a
> cl-defstruct slot name if you also have an initializer (SDEFAULT)
> form.  it gets confused and tries to use the elisp function
> definition.

Okay

> The following works on non-native comp but fails on native-comp.
>
> ```
> (require 'cl-lib)
> (defun xyz ())
> (cl-defstruct barf
>   (buffer-file-name (xyz)))
>
> (defun barf-foo ()
>   (let ((barf (make-barf)))))
> ```

What's the definition of fail here?

Thanks

  Andrea

Re: question regarding my emacs package

[Madhu]

* Andrea Corallo <yp1ilbr91x6.fsf @fencepost.gnu.org> :
Wrote on Tue, 13 Jun 2023 04:55:33 -0400:
> Madhu <enometh@meer.net> writes:
>
>> The following works on non-native comp but fails on native-comp.
>>
>> ```
>> (require 'cl-lib)
>> (defun xyz ())
>> (cl-defstruct barf
>>   (buffer-file-name (xyz)))
>>
>> (defun barf-foo ()
>>   (let ((barf (make-barf)))))
>> ```
>
> What's the definition of fail here?

calling M-: (byte-compile-file <file> ) or M-: (native-compile <file>)
on a file with those forms puts me in the debugger backtrace buffer with
the sort of lines I posted upthread.

OK.  I see why others dont see a problem with this. If I call M-x
byte-compile-file or M-x emacs-lisp-native-compile-and-load I only see
this in *Messages* and nothing in *compile-log*

```
Compiling /home/madhu/elisp/debug/barf.el... (make-barf)
Warning: Optimization failure for make-barf: Handler: make-barf--cmacro
(wrong-type-argument stringp (xyz))
Compiling /home/madhu/elisp/debug/barf.el...done
Wrote /home/madhu/elisp/debug/barf.elc
```

So my "errors" were a result of debug-on-error being `t' when evaluating
lisp interactively. So i assume this is not a real problem.

Re: question regarding my emacs package

[Michael Heerdegen]

Mattias,

could you maybe please have a short look?

Madhu <enometh@meer.net> writes:

> >> (require 'cl-lib)
> >> (defun xyz ())
> >> (cl-defstruct barf
> >>   (buffer-file-name (xyz)))
> >>
> >> (defun barf-foo ()
> >>   (let ((barf (make-barf)))))

==> Optimization failure for make-barf: Handler: make-barf--cmacro
(wrong-type-argument stringp (xyz))

Madhu, seems you have hit a corner case bug.

The resulting code (i.e. the defined `make-barf--cmacro') uses
`buffer-file-name' as the name of a variable it let-binds, which results
in the above warning (or a similar error message) in the case of this
variable.

Is this a known limitation (Mattias)?


TIA,

Michael.

Re: question regarding my emacs package

[Mattias Engdegård]

14 juni 2023 kl. 01.55 skrev Michael Heerdegen <michael_heerdegen@web.de>:

> The resulting code (i.e. the defined `make-barf--cmacro') uses
> `buffer-file-name' as the name of a variable it let-binds, which results
> in the above warning (or a similar error message) in the case of this
> variable.
> 
> Is this a known limitation (Mattias)?

Not to me until now. I'm sure Stefan would delight in delving into the nuts and bolts of the matter.

Re: question regarding my emacs package

[Michael Heerdegen]

Mattias Engdegård <mattiase@acm.org> writes:

> > The resulting code (i.e. the defined `make-barf--cmacro') uses
> > `buffer-file-name' as the name of a variable it let-binds, which results
> > in the above warning (or a similar error message) in the case of this
> > variable.
> > 
> > Is this a known limitation (Mattias)?
>
> Not to me until now. I'm sure Stefan would delight in delving into the
> nuts and bolts of the matter.

I think this is more or less the same as Bug#47552 ("cl-defstruct field
names matching read-only variables -> bad code") - which was marked as
fixed, but isn't.  I have reopened that one and added the recipe from
here as another incarnation example.

Michael.

Re: question regarding my emacs package

[Madhu]

* Michael Heerdegen <87v8fonjv4.fsf @web.de> :
Wrote on Fri, 16 Jun 2023 05:53:19 +0200:
>
> I think this is more or less the same as Bug#47552 ("cl-defstruct field
> names matching read-only variables -> bad code") - which was marked as
> fixed, but isn't.  I have reopened that one and added the recipe from
> here as another incarnation example.

Thank you.

Re: question regarding my emacs package

[Michael Heerdegen]

Madhu <enometh@meer.net> writes:

> [Question about cl-defstruct]

> Debugger entered--Lisp error: (wrong-type-argument stringp
> (buffer-file-name))
>   make-breadcrumbs--breadcrumb--cmacro((make-breadcrumbs--breadcrumb))
>   apply(make-breadcrumbs--breadcrumb--cmacro
> (make-breadcrumbs--breadcrumb) nil)
>   macroexp--compiler-macro(make-breadcrumbs--breadcrumb--cmacro
> (make-breadcrumbs--breadcrumb))
> ```

@ram: AFAIU you need to use a different slot name.  This one will cause
problems due to a bug in Emacs.

Michael.

Re: question regarding my emacs package

[ram via Emacs development discussions.]

[-- Attachment #1.1: Type: text/html, Size: 1091 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: question regarding my emacs package

[Michael Heerdegen]

ram via "Emacs development discussions." <emacs-devel@gnu.org> writes:

> what versions does this impact? i’ve never run into this on >=27.1?
> can someone confirm?

No, sorry.  In master using this example:

#+begin_src emacs-lisp
  (require 'cl-lib)
  (defun xyz ())
  
  (cl-defstruct barf "Doc" (buffer-file-name (xyz)))
  
  (defun barf-foo ()
    (let ((barf (make-barf)))
      barf))
#+end_src

I get an error when trying to load.  It compiles and I can load the
result, but for example M-: (make-barf :buffer-file-name t) errors.

Maybe there are differences due to lexical vs. dynamic binding and
changes to the compiler since 27.1, but I think the basic problem
remains the same.  If I were you I would avoid using 'buffer-file-name'
as slot name.

Michael.

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

got around to this. new repo is here: https://github.com/gitrj95/trail.el

renamed from breadcrumbs to trail, as per joao's suggestion, and changed the semantics quite a bit. i had some questions:

-   i'm supporting "middle" truncation of the file path strings. i couldn't find this in emacs libs, but for this purpose, i found truncation at the suffix or prefix less useful than truncation in the middle. ie, i want to know the file name and extension almost always and the root directory (especially in a code context) almost always
-   i'm using markers to track relative positions instead of absolute positions. i found that, especially in the code context, i wanted the trail-mark to "know" that i've added code before it, but still point to the function prototype i marked before instead of sliding. markers don't seem to be persistable, so i had the absolute positions of the markers stored in the ring that can be persisted, but default to corresponding markers in an associated, private ring if those exist. on hook for buffer close, the absolute positions are updated with the marker details. this seems like a fairly common thing to want to do, so i'd hope that i didn't reinvent the wheel. again, i couldn't find anything

also:

-   checkdoc passes
-   tested on emacs 27, 28, and 29

let me know if you have any questions, and please send over the fsf copyright agreement when you have the chance

looking forward


------- Original Message -------
On Friday, June 16th, 2023 at 10:01 PM, ram <chat@rj95.be> wrote:


> what versions does this impact? i’ve never run into this on >=27.1? can someone confirm?
> 
> 
> 
> On Fri, Jun 16, 2023 at 10:28 AM, Michael Heerdegen <michael_heerdegen@web.de> wrote:
> 
> > Madhu <enometh@meer.net> writes:
> > 
> > > [Question about cl-defstruct]
> > 
> > > Debugger entered--Lisp error: (wrong-type-argument stringp
> > > (buffer-file-name))
> > > make-breadcrumbs--breadcrumb--cmacro((make-breadcrumbs--breadcrumb))
> > > apply(make-breadcrumbs--breadcrumb--cmacro
> > > (make-breadcrumbs--breadcrumb) nil)
> > > macroexp--compiler-macro(make-breadcrumbs--breadcrumb--cmacro
> > > (make-breadcrumbs--breadcrumb))
> > > ```
> > 
> > @ram: AFAIU you need to use a different slot name. This one will cause
> > problems due to a bug in Emacs.
> > 
> > Michael.
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSd21AJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AACXgAQCVd483+cyNE+92uk5e8s6vlsWwUuDw33DfFpHQs4/tcAEApG0i
1FxxxcaK3wgFA0jZcTCAmlaTSWwi377MSkxzzgg=
=OoeH
-----END PGP SIGNATURE-----

Re: question regarding my emacs package

[João Távora]

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

Just a FYI, I've recently (about a month ago) created a package called
breadcrumb.el, singular. Lives at https://github.com/joaotavora/breadcrumb
and seems to be fairly popular.

Not saying anyone should consider changing package names, just dropping
this note here as a FYI.

João


On Wed, Jun 7, 2023, 13:09 ram <chat@rj95.be> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> 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
>
> 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
>
> here is the repo: https://github.com/gitrj95/breadcrumbs.el
>
> best,
> ram
> -----BEGIN PGP SIGNATURE-----
> Version: ProtonMail
>
> wnUEARYKACcFgmR/9tMJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
> A80AACtMAQDBJcq/+hW8lFC/JkXKSAsIjD5/dFF/jcjYjfRh3o0TMwD9Eb4W
> f0DKtY6/QCgyoPMLsMIBUf1cDZ1c53rQ9xHWCAI=
> =5pUI
> -----END PGP SIGNATURE-----
>
>
>

[-- Attachment #2: Type: text/html, Size: 1968 bytes --]

Re: question regarding my emacs package

[ram via Emacs development discussions.]

[-- Attachment #1.1: Type: text/html, Size: 2465 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: question regarding my emacs package

[Philip Kaludercic]

ram via "Emacs development discussions." <emacs-devel@gnu.org> writes:

> thanks. did not know this existed. from a quick glance, it looks like
> there’s no crossover in prefixes? that seems ok

Yes, but it is pretty close and will probably cause confusion.  That
being said, João is not part of any ELPA so one could argue the name
hasn't been reserved...

> On Sun, Jun 11, 2023 at 2:49 PM, João Távora <joaotavora@gmail.com> wrote:
>
>  Just a FYI, I've recently (about a month ago) created a package called breadcrumb.el, singular. Lives at
>  https://github.com/joaotavora/breadcrumb and seems to be fairly popular.
>
>  Not saying anyone should consider changing package names, just dropping this note here as a FYI.
>
>  João
>
>  On Wed, Jun 7, 2023, 13:09 ram <chat@rj95.be> wrote:
>
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA512
>
>  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
>
>  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
>
>  here is the repo: https://github.com/gitrj95/breadcrumbs.el
>
>  best,
>  ram
>  -----BEGIN PGP SIGNATURE-----
>  Version: ProtonMail
>
>  wnUEARYKACcFgmR/9tMJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
>  A80AACtMAQDBJcq/+hW8lFC/JkXKSAsIjD5/dFF/jcjYjfRh3o0TMwD9Eb4W
>  f0DKtY6/QCgyoPMLsMIBUf1cDZ1c53rQ9xHWCAI=
>  =5pUI
>  -----END PGP SIGNATURE-----
>

-- 
Philip Kaludercic

Re: question regarding my emacs package

[João Távora]

On Mon, Jun 12, 2023 at 9:45 AM Philip Kaludercic <philipk@posteo.net> wrote:
>
> ram via "Emacs development discussions." <emacs-devel@gnu.org> writes:
>
> > thanks. did not know this existed. from a quick glance, it looks like
> > there’s no crossover in prefixes? that seems ok
>
> Yes, but it is pretty close and will probably cause confusion.  That
> being said, João is not part of any ELPA so one could argue the name
> hasn't been reserved...

Just some thoughts:

1. There is no name clash, as ram observed

2. I'm not aware of a name reservation/parking procedure.  As far as I can
tell, ram's breadcrumbs.el isn't a part of any ELPA yet.

3. If early manifestation of adding a library is somehow relevant
then I might point you to this ~8-month old thread where I propose we
add breadcrumb.el to Emacs [1].  That's where breadcrumb.el was born and
proposed for Emacs.  I'm still on the fence about whether to propose
it for a :core package or ELPA package.  It takes inspiration on a
similar-named lsp-mode feature and the "breadcrumb" concept is apparently
a common name given to the type of feature breadcrumb.el enables.

4. There is no name clash, I think this bears repeating.

5. Above, I wrote "if early manifestation... is relevant".  I don't think
it should be.  If avoiding confusion to users is a goal to pursue -- and
if this confusion is indeed real (see 4 and 1) -- then I think it's more
important to evaluate the potential consequences of a rename in existing
users of each package.  We don't know that number, but merely as data point,
breadcrumbs.el has 6 github stars and breadcrumb.el has 116.

6. Please let's avoid another "meaning of names/meaning of life" thread.
There is no name clash.

João

[1]: https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-10/msg01027.html

Re: question regarding my emacs package

[Philip Kaludercic]

João Távora <joaotavora@gmail.com> writes:

> On Mon, Jun 12, 2023 at 9:45 AM Philip Kaludercic <philipk@posteo.net> wrote:
>>
>> ram via "Emacs development discussions." <emacs-devel@gnu.org> writes:
>>
>> > thanks. did not know this existed. from a quick glance, it looks like
>> > there’s no crossover in prefixes? that seems ok
>>
>> Yes, but it is pretty close and will probably cause confusion.  That
>> being said, João is not part of any ELPA so one could argue the name
>> hasn't been reserved...
>
> Just some thoughts:
>
> 1. There is no name clash, as ram observed

Yes, there is only the potential for confusion.

> 2. I'm not aware of a name reservation/parking procedure.  As far as I can
> tell, ram's breadcrumbs.el isn't a part of any ELPA yet.
>
> 3. If early manifestation of adding a library is somehow relevant
> then I might point you to this ~8-month old thread where I propose we
> add breadcrumb.el to Emacs [1].  That's where breadcrumb.el was born and
> proposed for Emacs.  

Oh, I did not know that.  Intuitively, I would argue that you tried to
reserve the name in the "core emacs/ELPA namespace" first.

>                      I'm still on the fence about whether to propose
> it for a :core package or ELPA package.  It takes inspiration on a
> similar-named lsp-mode feature and the "breadcrumb" concept is apparently
> a common name given to the type of feature breadcrumb.el enables.

I think that both options would be fine, and I'd be happy to help with
either.

> 4. There is no name clash, I think this bears repeating.
>
> 5. Above, I wrote "if early manifestation... is relevant".  I don't think
> it should be.  If avoiding confusion to users is a goal to pursue -- and
> if this confusion is indeed real (see 4 and 1) -- then I think it's more
> important to evaluate the potential consequences of a rename in existing
> users of each package.  We don't know that number, but merely as data point,
> breadcrumbs.el has 6 github stars and breadcrumb.el has 116.

Arguing via stars is difficult, because on the one hand you are a well
known Elisp author and your packages are bound to attract more attention
over a longer period of time, but we cannot deduce anything from these
facts.  E.g. None of my packages (except for Compat, which is
co-maintained) are on GitHub, so this metric couldn't be used if ram
decided to use SourceHut as well.

> 6. Please let's avoid another "meaning of names/meaning of life" thread.
> There is no name clash.

Ok, in that case I won't argue that that this is one of the issue of
using intuitive names ;)

> João
>
> [1]: https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-10/msg01027.html

-- 
Philip Kaludercic

Re: question regarding my emacs package

[João Távora]

On Mon, Jun 12, 2023 at 11:54 AM Philip Kaludercic <philipk@posteo.net> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> > On Mon, Jun 12, 2023 at 9:45 AM Philip Kaludercic <philipk@posteo.net> wrote:

> > 1. There is no name clash, as ram observed
> Yes, there is only the potential for confusion.

If a name clash existed, you wouldn't just have confusion,
you'd have an integration problem in your hands.

> Oh, I did not know that.  Intuitively, I would argue that you tried to
> reserve the name in the "core emacs/ELPA namespace" first.
> ...
> Arguing via stars is difficult, because on the one hand you are a well
> known Elisp author and your packages are bound to attract more attention
> over a longer period of time, but we cannot deduce anything from these
> facts.

Of course we can.   We can infer from the amount of attention attracted
the number of users that have dispensed that attention.  The period
of time over which that amount of attention was gathered and notoriety
of the author is irrelevant: all other things being equal, you want to
know the number of users impacted at the time by some hypothetical
administrative decision.

What I'm trying to tell you is two things: 1. I do agree with you that
some confusion is bound to arise from the similarity in name, though
I'm not sure it warrants any action: that is up to you or others.  2. If
you wish to tackle the problem via some specific action, it's much smarter
to consider the number of users affected by that action rather than
considering who "reserved" the name first.

João

Re: question regarding my emacs package

[ram via Emacs development discussions.]

[-- Attachment #1.1: Type: text/html, Size: 2187 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: question regarding my emacs package

[João Távora]

On Tue, Jun 13, 2023 at 1:36 AM ram <chat@rj95.be> wrote:
>
> i agree. fwiw i’m not opposed to a name change if it comes to it

If you accept my suggestion, it would be trail.el which pleasantly
much shorter than breadcrumbs.el

João

Re: question regarding my emacs package

[ram]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

terser is always welcomed. is there a cute analog for dropping breadcrumbs (ie tracking a buffer position in my package's parlance) for a trail?



------- Original Message -------
On Tuesday, June 13th, 2023 at 1:13 PM, João Távora <joaotavora@gmail.com> wrote:


> 
> 
> On Tue, Jun 13, 2023 at 1:36 AM ram chat@rj95.be wrote:
> 
> > i agree. fwiw i’m not opposed to a name change if it comes to it
> 
> 
> If you accept my suggestion, it would be trail.el which pleasantly
> much shorter than breadcrumbs.el
> 
> João
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wnUEARYKACcFgmSI44sJkLWF/Pp8PQPNFiEExUjWugOWGzBgCQZvtYX8+nw9
A80AAM11AQDuFaHO7YAhVbX92wbOGT5iHWunRpSMGIEXVi2JL77zMwEAy7o8
fuyUHVZPnVMSHh6AnrwAjxKaFVhXXCs9NYxy4gI=
=FIIS
-----END PGP SIGNATURE-----