From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] New package: cursor-undo Date: Sun, 21 Jul 2024 10:57:53 +0000 Message-ID: <87a5ibm2q6.fsf@posteo.net> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="17649"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Emacs developers To: =?utf-8?B?6Lev5a6i?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Jul 21 12:58:36 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sVUGx-0004Q8-HE for ged-emacs-devel@m.gmane-mx.org; Sun, 21 Jul 2024 12:58:35 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sVUGU-0005vM-0r; Sun, 21 Jul 2024 06:58:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sVUGS-0005uk-Ig for emacs-devel@gnu.org; Sun, 21 Jul 2024 06:58:04 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sVUGP-0005AH-9a for emacs-devel@gnu.org; Sun, 21 Jul 2024 06:58:03 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 2404A240029 for ; Sun, 21 Jul 2024 12:57:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1721559475; bh=P2NUSdfnNG+vpDzeiGxhdPsENrVXCRjix33ESWr7oc0=; h=From:To:Cc:Subject:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=CLvke/vuDTlMZp+R8eUDw4+b4VV2B4kL++Grv8LflTCFVv8uONLs7SQQByDl34W57 6Zizc5oPtqlRADnyLFg/tV+oUYxHE4+QeGlJ2TDosZ3ogKR1oGbrwliNVzzDTKbFTv 6JRAmABkjQ2fgys37W9E3+D1Xz3OspuPoUO8sDT+rks7EIXTAtspsdEdAHZ57eXq1G DTfo8kjQKV8+C+1rKXAFH85kK16DvMYBC9luWdXDbctxOACCLAIGPBo5mZSs62nCZe Iya7NLl3/P/TAHZeGZ0qSZDX6C9oAzroeqGaMdSY5EJ5AM9IuGkksMZCeeiXlxVd6p tco7E5xFKRnHQ== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4WRgNy2zcbz6txf; Sun, 21 Jul 2024 12:57:54 +0200 (CEST) In-Reply-To: (=?utf-8?B?Iui3r+WuoiIncw==?= message of "Sat, 20 Jul 2024 23:03:43 +0800") OpenPGP: id=7126E1DE2F0CE35C770BED01F2C3CC513DB89F66; url="https://keys.openpgp.org/vks/v1/by-fingerprint/7126E1DE2F0CE35C770BED01F2C3CC513DB89F66"; preference=signencrypt Received-SPF: pass client-ip=185.67.36.65; envelope-from=philipk@posteo.net; helo=mout01.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:321866 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable =E8=B7=AF=E5=AE=A2 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 =E2=80=98/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el=E2=80= =99 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 =E2=80=98/home/phi/Source/emacs/lisp/emacs-lisp/pcase.el=E2=80= =99 newer than byte-compiled file; using older file =3D=3D=3D=3D=3D=3D=3D=3D Building tarball archive-devel/cursor-undo-1.0.0.2= 0240720.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.t= ar FAILED!! =3D=3D=3D=3D=3D=3D=3D=3D 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 fo= rmat: 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: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 @@ =20 ;;; Code: =20 +;; 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) =20 ;; 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 l= eft ;; 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))))) =20 @@ -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 curs= or -relative screen position (screen-pos=3DNIL) nor `point' position (no-move= =3Dt).")) +"Error: No undo information will be stored if we're neither recording curs= or \ +relative screen position (screen-pos=3DNIL) nor `point' position (no-move= =3Dt)")) (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=3DNIL) nor `point'= position (no-move=3Dt).")) ;; 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=3DNIL) nor `point= ' position (no-move=3Dt).")) (numberp (cadr buffer-undo-list)) (=3D 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 + ;; =E2=80=98prev-screen-start=E2=80=99' byte-compiler w= arning + ;; below. '((push `(apply cundo-restore-win (,@prev-screen-start)) buffer-undo-list))) ,@(unless no-move @@ -598,3 +606,7 @@ relative screen position (screen-pos=3DNIL) nor `point'= position (no-move=3Dt).")) (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 + --=-=-= Content-Type: text/plain 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 --=-=-=--