[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: question regarding my emacs package
From: |
ram |
Subject: |
Re: question regarding my emacs package |
Date: |
Thu, 08 Jun 2023 18:50:28 +0000 |
-----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-----
- question regarding my emacs package, ram, 2023/06/07
- Re: question regarding my emacs package, Philip Kaludercic, 2023/06/07
- Re: question regarding my emacs package, Philip Kaludercic, 2023/06/07
- Re: question regarding my emacs package, ram, 2023/06/08
- Re: question regarding my emacs package, Philip Kaludercic, 2023/06/07
- Re: question regarding my emacs package, ram, 2023/06/08
- Re: question regarding my emacs package, ram, 2023/06/08
- Re: question regarding my emacs package, ram, 2023/06/08
- Re: question regarding my emacs package, Philip Kaludercic, 2023/06/08
- Re: question regarding my emacs package,
ram <=
- Re: question regarding my emacs package, Madhu, 2023/06/11
- Re: question regarding my emacs package, Madhu, 2023/06/12
- Re: question regarding my emacs package, Andrea Corallo, 2023/06/12
- Re: question regarding my emacs package, Madhu, 2023/06/13
- Re: question regarding my emacs package, Andrea Corallo, 2023/06/13
- Re: question regarding my emacs package, Madhu, 2023/06/13
- Re: question regarding my emacs package, Michael Heerdegen, 2023/06/13
- Re: question regarding my emacs package, Mattias EngdegÄrd, 2023/06/14
- Re: question regarding my emacs package, Michael Heerdegen, 2023/06/15
- Re: question regarding my emacs package, Madhu, 2023/06/16