Re: question regarding my emacs package

[ram <chat@rj95.be>]

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