[ELPA] New package: cursor-undo

[ELPA] New package: cursor-undo

[路客]

Hi all,
I've just pushed my new package into ELPA.

Cursor-undo allows you to undo cursor movement commands using the Emacs
standard `undo' command.

For frequent cursor movements such as up/down/left/right, it
combines the movements of the same direction into a single undo entry.
This prevents the undo command from reversing each individual
character movement separately.  For example, if you move the cursor 20
characters to the right and then 10 lines up, the first undo will go
down 10 lines back, and the next undo move back 20 characters left.
On the other hand, for search commands that often jump across multiple
pages, each search command has its own undo entry, allowing you to
undo them one at a time rather than as a combined operation.

This cursor-undo functionality has existed in my local Emacs init file
for over 11+ years, since version 0 on 2013-06-26.  It was originally
intended to support my Brief Editor Mode only, but I later found it
would be more useful if implemented in a more generalized way.  For
years I have hoped for an official implementation of this feature,
which is commonly seen among various editors.  Considering my
implementation using advice functions a bit inelegant so I have always
hesitated to release it till recently.

Until there is official support for the cursor undo feature, this
package serves most common daily needs.  The core design is to align
with Emacs's native `undo' function by recording cursor positions
and screen-relative position undo entries in the `buffer-undo-list'
in accordance with its documentation.

As this package primarily consists of advice functions to wrap cursor
movement commands, each cursor movement command needs to be manually
wrapped with `def-cursor-undo'.  For interactive functions that
heavily invoke advised cursor movement commands, you may even need to
advise them with `disable-cursor-tracking' to prevent generating
numerous distinct cursor undo entries from a single command.  For user
convenience, we have prepared ready `def-cursor-undo' advice sets for
standard Emacs cursor movement commands, Brief Editor mode, Viper
mode, and EVIL mode.

Usage:

  In theory, once this package is installed, you should already have
  cursor-undo autoloaded and enabled.  If not, or if you downloaded this
  package as source, put "cursor-undo.el" file in a `load-path' and add
  the following line into your Emacs init file .emacs or init.el:

    (require 'cursor-undo)

Notes for EVIL mode user:

  If you choose to use default Emacs `undo' system, you should be able
  to use `evil-undo' to undo cursor movements.  If your choice is
  tree-undo or another undo system, you might need to use Emacs default
  `undo' (C-_, C-/ or C-x u ...) to undo cursor movements.

Notes for Viper mode user:

  The default `viper-undo' is advised to allow cursor-undo.  If you
  find the advised function not working properly, consider comment out
  the following source code `(define-advice viper-undo ...' to restore
  the original `viper-undo' function and use Emacs default `undo'
  (C-_, C-/ or C-x u ...) to undo cursor movements.


-- 
Best regards,
Luke Lee

Re: [ELPA] New package: cursor-undo

[Philip Kaludercic]

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

路客 <luke.yx.lee@gmail.com> writes:

> Hi all,
> I've just pushed my new package into ELPA.

Before pushing a package directly to elpa.git, you should try to build
it locally.  Currently it fails because the :doc specification links to
a .txt file, which cannot be converted to a Info manual:

--8<---------------cut here---------------start------------->8---
$ make build/cursor-undo
emacs --batch -Q -l admin/elpa-admin.el \
         -f elpaa-batch-pkg-spec-make-dependencies .pkg-descs.mk
Source file ‘/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el’ newer than byte-compiled file; using older file
emacs --batch -l /home/phi/Source/elpa/admin/elpa-admin.el	\
         -f elpaa-batch-make-one-package cursor-undo
Source file ‘/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el’ newer than byte-compiled file; using older file
======== Building tarball archive-devel/cursor-undo-1.0.0.20240720.150723.tar...
Build error for archive-devel/cursor-undo-1.0.0.20240720.150723.tar: (error "Not a supported doc format: README.txt")
######## Build of package archive-devel/cursor-undo-1.0.0.20240720.150723.tar FAILED!!
======== Building tarball archive/cursor-undo-1.0.tar...
Warning (package): Package lacks a terminating comment
Build error for archive/cursor-undo-1.0.tar: (error "Not a supported doc format: README.txt")
######## Build of package archive/cursor-undo-1.0.tar FAILED!!
--8<---------------cut here---------------end--------------->8---

I can fix that myself by replacing the :doc with a :readme -- though
since your commentary section has the same contents as the README file,
we could also extract the documentation from there, without having to
duplicate it.

It would also be nice to announce a package and wait for some comments,
before pushing it to elpa.git (though I'll admit that I have done the
same in the past as well).  I therefore have a few comments that I think
you should consider:


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

diff --git a/cursor-undo.el b/cursor-undo.el
index 3af2dc8d23..717ad10bc0 100644
--- a/cursor-undo.el
+++ b/cursor-undo.el
@@ -83,7 +83,7 @@
 ;;   find the advised function not working properly, consider comment out
 ;;   the following source code `(define-advice viper-undo ...' to restore
 ;;   the original `viper-undo' function and use Emacs default `undo'
-;;   (C-_, C-/ or C-x u ...) to undo cursor movements.
+;;   (\\[undo]) to undo cursor movements.
 ;;
 ;; Future TODO thoughts: a more desired implementation is to integrate it
 ;; into Emacs source by extending the `interactive' spec to add code
@@ -93,11 +93,11 @@
 
 ;;; Code:
 
+;; shouldn't this be a major mode?
 (defvar cundo-enable-cursor-tracking t) ; global enable flag
 ;; Local disable flag, use reverse logic as NIL is still a list and we can
 ;; pop it again and again
-(defvar cundo-disable-local-cursor-tracking nil)
-(make-variable-buffer-local 'cundo-disable-local-cursor-tracking)
+(defvar-local cundo-disable-local-cursor-tracking nil)
 
 ;; Clear duplicated `(apply cdr nil) nil' pairs in `buffer-undo-list' done by
 ;; `primitive-undo'.
@@ -108,9 +108,9 @@
   (if cundo-clear-usless-undoinfo
       ;; This tries to remove as much garbage as possible but still some left
       ;; in the `buffer-undo-list'.  TODO: add a idle task to remove them
-      (while (and (null (first buffer-undo-list)) ;; nil
-                  (equal (second buffer-undo-list) '(apply cdr nil))
-                  (null (third buffer-undo-list)))
+      (while (and (null (nth 0 buffer-undo-list)) ;; nil
+                  (equal (nth 1 buffer-undo-list) '(apply cdr nil))
+                  (null (nth 2 buffer-undo-list)))
         ;; (nil (apply cdr nil) nil a b...) -> (nil a b...)
         (setq buffer-undo-list (cddr buffer-undo-list)))))
 
@@ -143,8 +143,8 @@ Parameters:
   (if (and (not screen-pos)
            no-move)
       (error
-"Error: No undo information will be stored if we're neither recording cursor
-relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
+"Error: No undo information will be stored if we're neither recording cursor \
+relative screen position (screen-pos=NIL) nor `point' position (no-move=t)"))
   (let* ((func-sym-str (symbol-name func-sym))
          (advice-sym-str (concat func-sym-str "-cursor-undo"))
          (advice-sym (make-symbol advice-sym-str))
@@ -161,7 +161,7 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
                 ;; prevent nested calls for complicated compound commands
                 (cundo-enable-cursor-tracking nil)
                 (prev-point (point))
-                prev-screen-start)
+                (prev-screen-start))
            ,@(when screen-pos
                '((when cursor-tracking
                    (setq prev-screen-start (window-start)))))
@@ -206,6 +206,14 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
                         (numberp (cadr buffer-undo-list))
                         (= prev-point (cadr buffer-undo-list))))
              ,@(if screen-pos
+                   ;; unless I am misreading this, `prev-screen-start'
+                   ;; is being evaluated in the context of the macro,
+                   ;; /not/ the macro-expansion.  Yet
+                   ;; `prev-screen-start' is bound and set inside the
+                   ;; context of the macro-expansion.  This seems to
+                   ;; trigger the 'Unused lexical variable
+                   ;; ‘prev-screen-start’' byte-compiler warning
+                   ;; below.
                   '((push `(apply cundo-restore-win (,@prev-screen-start))
                           buffer-undo-list)))
              ,@(unless no-move
@@ -598,3 +606,7 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
      (def-cursor-undo viper-search-forward                   t       t)
      (def-cursor-undo viper-beginning-of-line)
      (def-cursor-undo viper-repeat-find                      t       t)))
+
+(provide 'cursor-undo)
+;;; cursor-undo.el ends here
+

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


A general comment also regarding the `eval-after-load' blocks.  If I
were you, I'll pull all this extra support into additional files that
require cursor-undo and the packages you wish to support.  And instead
of the `eval-after-load' blocks, you could just scan the current
directory for all "cursor-undo-[...].el" files and call
`eval-after-load', requiring "cursor-undo-[...]".  That should clear a
number of the byte-compiler warnings.  It would also be nice if you
could tackle the checkdoc warnings as well.

> Cursor-undo allows you to undo cursor movement commands using the Emacs
> standard `undo' command.
>
> For frequent cursor movements such as up/down/left/right, it
> combines the movements of the same direction into a single undo entry.
> This prevents the undo command from reversing each individual
> character movement separately.  For example, if you move the cursor 20
> characters to the right and then 10 lines up, the first undo will go
> down 10 lines back, and the next undo move back 20 characters left.
> On the other hand, for search commands that often jump across multiple
> pages, each search command has its own undo entry, allowing you to
> undo them one at a time rather than as a combined operation.
>
> This cursor-undo functionality has existed in my local Emacs init file
> for over 11+ years, since version 0 on 2013-06-26.  It was originally
> intended to support my Brief Editor Mode only, but I later found it
> would be more useful if implemented in a more generalized way.  For
> years I have hoped for an official implementation of this feature,
> which is commonly seen among various editors.  Considering my
> implementation using advice functions a bit inelegant so I have always
> hesitated to release it till recently.
>
> Until there is official support for the cursor undo feature, this
> package serves most common daily needs.  The core design is to align
> with Emacs's native `undo' function by recording cursor positions
> and screen-relative position undo entries in the `buffer-undo-list'
> in accordance with its documentation.
>
> As this package primarily consists of advice functions to wrap cursor
> movement commands, each cursor movement command needs to be manually
> wrapped with `def-cursor-undo'.  For interactive functions that
> heavily invoke advised cursor movement commands, you may even need to
> advise them with `disable-cursor-tracking' to prevent generating
> numerous distinct cursor undo entries from a single command.  For user
> convenience, we have prepared ready `def-cursor-undo' advice sets for
> standard Emacs cursor movement commands, Brief Editor mode, Viper
> mode, and EVIL mode.
>
> Usage:
>
>   In theory, once this package is installed, you should already have
>   cursor-undo autoloaded and enabled.  If not, or if you downloaded this
>   package as source, put "cursor-undo.el" file in a `load-path' and add
>   the following line into your Emacs init file .emacs or init.el:
>
>     (require 'cursor-undo)
>
> Notes for EVIL mode user:
>
>   If you choose to use default Emacs `undo' system, you should be able
>   to use `evil-undo' to undo cursor movements.  If your choice is
>   tree-undo or another undo system, you might need to use Emacs default
>   `undo' (C-_, C-/ or C-x u ...) to undo cursor movements.
>
> Notes for Viper mode user:
>
>   The default `viper-undo' is advised to allow cursor-undo.  If you
>   find the advised function not working properly, consider comment out
>   the following source code `(define-advice viper-undo ...' to restore
>   the original `viper-undo' function and use Emacs default `undo'
>   (C-_, C-/ or C-x u ...) to undo cursor movements.

-- 
	Philip Kaludercic on peregrine

Re: [ELPA] New package: cursor-undo

[路客]

>  Before pushing a package directly to elpa.git, you should try to build
>  it locally.  Currently it fails because the :doc specification links to
>  a .txt file, which cannot be converted to a Info manual:

I followed the ELPA steps but there is no info about how to build a new
package locally before pushing into the main git branch external/<package>
. A "make build/cursor-undo" command sometimes cause a pull from external
git.  Even in the ELPA `elpa-admin.el' the first TODO is:

  ;; - bug#45345: [elpa-archive] "make build/<package>" should not pull
  ;;   unconditionally

and I found this is /sometimes/ true.  Sometimes I modified my source code
and do "make build/cursor-undo" again it will be overwritten back to the
old one in git external/cursor-undo.  Therefore, the only way is to push
it back into git and wait a while then rebuild.  However, sometimes it
won't so it's confusing what the correct steps to perform a pure local
build really are.  So, let me know if there is a way to build a new
package locally without push into the external/<package> first.

But as a hindsight I now learned I could first modify "elpa-packages"
locally and build but still need to push into external/<package> first.
Next time I will make sure I do that.

After I managed to build my package I found the ELPA README did not mention
that the ";;; <package> ends here" is required.

Some of your comments are followed while some not:

> +;; shouldn't this be a major mode?
> (defvar cundo-enable-cursor-tracking t) ; global enable flag

Actually this package is not a mode, but it seems it's preferred as Stefan
also mentioned this to me so I now made it a minor mode.

>                   ;; unless I am misreading this, `prev-screen-start'
>                   ;; is being evaluated in the context of the macro,
>                   ;; /not/ the macro-expansion.  Yet
>                   ;; `prev-screen-start' is bound and set inside the
>                   ;; context of the macro-expansion.  This seems to
>                   ;; trigger the 'Unused lexical variable
>                   ;; ‘prev-screen-start’' byte-compiler warning
>                   ;; below.

Yes it was meant to be evaluated instead of being expanded since the
beginning many years ago, this part of code has been tested for many
years, I reviewed the generated code again and it's still valid.  I think
the byte compiler must have mis-interpreted my intention and gave the
incorrect warnings.  Anyway, before the root cause is found a quick
mitigation is to (defvar prev-screen-start).

>  +(provide 'cursor-undo)

This was done earlier in the code.

> +;;; cursor-undo.el ends here

This step is a must but the ELPA README did not mention that.  I found
the local build would fail without it, so it's now added.

The fixed version has been pushed into the git.  Thanks.

Best regards,
Luke Lee


On Sun, Jul 21, 2024 at 6:57 PM Philip Kaludercic <philipk@posteo.net> wrote:
>
> 路客 <luke.yx.lee@gmail.com> writes:
>
> > Hi all,
> > I've just pushed my new package into ELPA.
>
> Before pushing a package directly to elpa.git, you should try to build
> it locally.  Currently it fails because the :doc specification links to
> a .txt file, which cannot be converted to a Info manual:
>
> --8<---------------cut here---------------start------------->8---
> $ make build/cursor-undo
> emacs --batch -Q -l admin/elpa-admin.el \
>          -f elpaa-batch-pkg-spec-make-dependencies .pkg-descs.mk
> Source file ‘/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el’ newer than byte-compiled file; using older file
> emacs --batch -l /home/phi/Source/elpa/admin/elpa-admin.el      \
>          -f elpaa-batch-make-one-package cursor-undo
> Source file ‘/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el’ newer than byte-compiled file; using older file
> ======== Building tarball archive-devel/cursor-undo-1.0.0.20240720.150723.tar...
> Build error for archive-devel/cursor-undo-1.0.0.20240720.150723.tar: (error "Not a supported doc format: README.txt")
> ######## Build of package archive-devel/cursor-undo-1.0.0.20240720.150723.tar FAILED!!
> ======== Building tarball archive/cursor-undo-1.0.tar...
> Warning (package): Package lacks a terminating comment
> Build error for archive/cursor-undo-1.0.tar: (error "Not a supported doc format: README.txt")
> ######## Build of package archive/cursor-undo-1.0.tar FAILED!!
> --8<---------------cut here---------------end--------------->8---
>
> I can fix that myself by replacing the :doc with a :readme -- though
> since your commentary section has the same contents as the README file,
> we could also extract the documentation from there, without having to
> duplicate it.
>
> It would also be nice to announce a package and wait for some comments,
> before pushing it to elpa.git (though I'll admit that I have done the
> same in the past as well).  I therefore have a few comments that I think
> you should consider:
>
> diff --git a/cursor-undo.el b/cursor-undo.el
> index 3af2dc8d23..717ad10bc0 100644
> --- a/cursor-undo.el
> +++ b/cursor-undo.el
> @@ -83,7 +83,7 @@
>  ;;   find the advised function not working properly, consider comment out
>  ;;   the following source code `(define-advice viper-undo ...' to restore
>  ;;   the original `viper-undo' function and use Emacs default `undo'
> -;;   (C-_, C-/ or C-x u ...) to undo cursor movements.
> +;;   (\\[undo]) to undo cursor movements.
>  ;;
>  ;; Future TODO thoughts: a more desired implementation is to integrate it
>  ;; into Emacs source by extending the `interactive' spec to add code
> @@ -93,11 +93,11 @@
>
>  ;;; Code:
>
> +;; shouldn't this be a major mode?
>  (defvar cundo-enable-cursor-tracking t) ; global enable flag
>  ;; Local disable flag, use reverse logic as NIL is still a list and we can
>  ;; pop it again and again
> -(defvar cundo-disable-local-cursor-tracking nil)
> -(make-variable-buffer-local 'cundo-disable-local-cursor-tracking)
> +(defvar-local cundo-disable-local-cursor-tracking nil)
>
>  ;; Clear duplicated `(apply cdr nil) nil' pairs in `buffer-undo-list' done by
>  ;; `primitive-undo'.
> @@ -108,9 +108,9 @@
>    (if cundo-clear-usless-undoinfo
>        ;; This tries to remove as much garbage as possible but still some left
>        ;; in the `buffer-undo-list'.  TODO: add a idle task to remove them
> -      (while (and (null (first buffer-undo-list)) ;; nil
> -                  (equal (second buffer-undo-list) '(apply cdr nil))
> -                  (null (third buffer-undo-list)))
> +      (while (and (null (nth 0 buffer-undo-list)) ;; nil
> +                  (equal (nth 1 buffer-undo-list) '(apply cdr nil))
> +                  (null (nth 2 buffer-undo-list)))
>          ;; (nil (apply cdr nil) nil a b...) -> (nil a b...)
>          (setq buffer-undo-list (cddr buffer-undo-list)))))
>
> @@ -143,8 +143,8 @@ Parameters:
>    (if (and (not screen-pos)
>             no-move)
>        (error
> -"Error: No undo information will be stored if we're neither recording cursor
> -relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
> +"Error: No undo information will be stored if we're neither recording cursor \
> +relative screen position (screen-pos=NIL) nor `point' position (no-move=t)"))
>    (let* ((func-sym-str (symbol-name func-sym))
>           (advice-sym-str (concat func-sym-str "-cursor-undo"))
>           (advice-sym (make-symbol advice-sym-str))
> @@ -161,7 +161,7 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
>                  ;; prevent nested calls for complicated compound commands
>                  (cundo-enable-cursor-tracking nil)
>                  (prev-point (point))
> -                prev-screen-start)
> +                (prev-screen-start))
>             ,@(when screen-pos
>                 '((when cursor-tracking
>                     (setq prev-screen-start (window-start)))))
> @@ -206,6 +206,14 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
>                          (numberp (cadr buffer-undo-list))
>                          (= prev-point (cadr buffer-undo-list))))
>               ,@(if screen-pos
> +                   ;; unless I am misreading this, `prev-screen-start'
> +                   ;; is being evaluated in the context of the macro,
> +                   ;; /not/ the macro-expansion.  Yet
> +                   ;; `prev-screen-start' is bound and set inside the
> +                   ;; context of the macro-expansion.  This seems to
> +                   ;; trigger the 'Unused lexical variable
> +                   ;; ‘prev-screen-start’' byte-compiler warning
> +                   ;; below.
>                    '((push `(apply cundo-restore-win (,@prev-screen-start))
>                            buffer-undo-list)))
>               ,@(unless no-move
> @@ -598,3 +606,7 @@ relative screen position (screen-pos=NIL) nor `point' position (no-move=t)."))
>       (def-cursor-undo viper-search-forward                   t       t)
>       (def-cursor-undo viper-beginning-of-line)
>       (def-cursor-undo viper-repeat-find                      t       t)))
> +
> +(provide 'cursor-undo)
> +;;; cursor-undo.el ends here
> +
>
> A general comment also regarding the `eval-after-load' blocks.  If I
> were you, I'll pull all this extra support into additional files that
> require cursor-undo and the packages you wish to support.  And instead
> of the `eval-after-load' blocks, you could just scan the current
> directory for all "cursor-undo-[...].el" files and call
> `eval-after-load', requiring "cursor-undo-[...]".  That should clear a
> number of the byte-compiler warnings.  It would also be nice if you
> could tackle the checkdoc warnings as well.
>
> > Cursor-undo allows you to undo cursor movement commands using the Emacs
> > standard `undo' command.
> >
> > For frequent cursor movements such as up/down/left/right, it
> > combines the movements of the same direction into a single undo entry.
> > This prevents the undo command from reversing each individual
> > character movement separately.  For example, if you move the cursor 20
> > characters to the right and then 10 lines up, the first undo will go
> > down 10 lines back, and the next undo move back 20 characters left.
> > On the other hand, for search commands that often jump across multiple
> > pages, each search command has its own undo entry, allowing you to
> > undo them one at a time rather than as a combined operation.
> >
> > This cursor-undo functionality has existed in my local Emacs init file
> > for over 11+ years, since version 0 on 2013-06-26.  It was originally
> > intended to support my Brief Editor Mode only, but I later found it
> > would be more useful if implemented in a more generalized way.  For
> > years I have hoped for an official implementation of this feature,
> > which is commonly seen among various editors.  Considering my
> > implementation using advice functions a bit inelegant so I have always
> > hesitated to release it till recently.
> >
> > Until there is official support for the cursor undo feature, this
> > package serves most common daily needs.  The core design is to align
> > with Emacs's native `undo' function by recording cursor positions
> > and screen-relative position undo entries in the `buffer-undo-list'
> > in accordance with its documentation.
> >
> > As this package primarily consists of advice functions to wrap cursor
> > movement commands, each cursor movement command needs to be manually
> > wrapped with `def-cursor-undo'.  For interactive functions that
> > heavily invoke advised cursor movement commands, you may even need to
> > advise them with `disable-cursor-tracking' to prevent generating
> > numerous distinct cursor undo entries from a single command.  For user
> > convenience, we have prepared ready `def-cursor-undo' advice sets for
> > standard Emacs cursor movement commands, Brief Editor mode, Viper
> > mode, and EVIL mode.
> >
> > Usage:
> >
> >   In theory, once this package is installed, you should already have
> >   cursor-undo autoloaded and enabled.  If not, or if you downloaded this
> >   package as source, put "cursor-undo.el" file in a `load-path' and add
> >   the following line into your Emacs init file .emacs or init.el:
> >
> >     (require 'cursor-undo)
> >
> > Notes for EVIL mode user:
> >
> >   If you choose to use default Emacs `undo' system, you should be able
> >   to use `evil-undo' to undo cursor movements.  If your choice is
> >   tree-undo or another undo system, you might need to use Emacs default
> >   `undo' (C-_, C-/ or C-x u ...) to undo cursor movements.
> >
> > Notes for Viper mode user:
> >
> >   The default `viper-undo' is advised to allow cursor-undo.  If you
> >   find the advised function not working properly, consider comment out
> >   the following source code `(define-advice viper-undo ...' to restore
> >   the original `viper-undo' function and use Emacs default `undo'
> >   (C-_, C-/ or C-x u ...) to undo cursor movements.
>
> --
>         Philip Kaludercic on peregrine



--
Best regards,
Luke Lee

Re: [ELPA] New package: cursor-undo

[Philip Kaludercic]

路客 <luke.yx.lee@gmail.com> writes:

>>  Before pushing a package directly to elpa.git, you should try to build
>>  it locally.  Currently it fails because the :doc specification links to
>>  a .txt file, which cannot be converted to a Info manual:
>
> I followed the ELPA steps but there is no info about how to build a new
> package locally before pushing into the main git branch external/<package>
> . A "make build/cursor-undo" command sometimes cause a pull from external
> git.  Even in the ELPA `elpa-admin.el' the first TODO is:
>
>   ;; - bug#45345: [elpa-archive] "make build/<package>" should not pull
>   ;;   unconditionally
>
> and I found this is /sometimes/ true.  Sometimes I modified my source code
> and do "make build/cursor-undo" again it will be overwritten back to the
> old one in git external/cursor-undo.  Therefore, the only way is to push
> it back into git and wait a while then rebuild.  However, sometimes it
> won't so it's confusing what the correct steps to perform a pure local
> build really are.  So, let me know if there is a way to build a new
> package locally without push into the external/<package> first.

I have to admit that I am not sure about this, but I can look into it.
In the last few years the recommended workflow has been to use an
external repository that is mirrored into elpa.git, so I guess this
hasn't been an issue that a lot of people have encountered recently.

> But as a hindsight I now learned I could first modify "elpa-packages"
> locally and build but still need to push into external/<package> first.
> Next time I will make sure I do that.
>
> After I managed to build my package I found the ELPA README did not mention
> that the ";;; <package> ends here" is required.

That is documented in (elisp) Library Headers.  It is not a
ELPA-specific rule.

> Some of your comments are followed while some not:
>
>> +;; shouldn't this be a major mode?
>> (defvar cundo-enable-cursor-tracking t) ; global enable flag
>
> Actually this package is not a mode, but it seems it's preferred as Stefan
> also mentioned this to me so I now made it a minor mode.

1+

>>                   ;; unless I am misreading this, `prev-screen-start'
>>                   ;; is being evaluated in the context of the macro,
>>                   ;; /not/ the macro-expansion.  Yet
>>                   ;; `prev-screen-start' is bound and set inside the
>>                   ;; context of the macro-expansion.  This seems to
>>                   ;; trigger the 'Unused lexical variable
>>                   ;; ‘prev-screen-start’' byte-compiler warning
>>                   ;; below.
>
> Yes it was meant to be evaluated instead of being expanded since the
> beginning many years ago, this part of code has been tested for many
> years, I reviewed the generated code again and it's still valid.  I think
> the byte compiler must have mis-interpreted my intention and gave the
> incorrect warnings.  Anyway, before the root cause is found a quick
> mitigation is to (defvar prev-screen-start).

Let's hope so.

>>  +(provide 'cursor-undo)
>
> This was done earlier in the code.

Must have missed it then.  Is there a reason you didn't add it at the
end, as is conventional (and probably useful, if there is an error while
loading the file, that you don't mark it as required)?

>> +;;; cursor-undo.el ends here
>
> This step is a must but the ELPA README did not mention that.  I found
> the local build would fail without it, so it's now added.
>
> The fixed version has been pushed into the git.  Thanks.
>
> Best regards,
> Luke Lee

-- 
	Philip Kaludercic on peregrine

Re: [ELPA] New package: cursor-undo

[Luke Lee]

> >>  +(provide 'cursor-undo)
> >
> > This was done earlier in the code.
>
> Must have missed it then.  Is there a reason you didn't add it at the
> end, as is conventional (and probably useful, if there is an error while
> loading the file, that you don't mark it as required)?
>

I was trying to separate the implementation code and its use, but as
you give a great point about errors while loading that I didn't think of,
I'll move it down before the end marker in my next release.
Thanks!

Best regards,
Luke Lee