emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [NonGNU ELPA] New package: devil


From: Philip Kaludercic
Subject: Re: [NonGNU ELPA] New package: devil
Date: Tue, 09 May 2023 08:42:20 +0000

Susam Pal <susam.pal@gmail.com> writes:

> Hello!
>
> I am the author and maintainer of a new package named Devil. This package
> intercepts keystrokes entered by the user and applies translation rules to
> translate the keystrokes into Emacs key sequences. It supports three types
> of rules: special keys that map to custom commands that are invoked
> immediately prior to any translations, translation rules to translate Devil
> key sequences to regular Emacs key sequences, and repeatable keys to allow
> a Devil key sequence to be repeated by typing the last keystroke over and
> over again using a transient map.
>
> See the README at https://github.com/susam/devil for more details.

Looks interesting, here is a diff with a few comments:

diff -u /home/philip/devil.el.1 /home/philip/devil.el
--- /home/philip/devil.el.1     2023-05-09 10:37:55.243222106 +0200
+++ /home/philip/devil.el       2023-05-09 10:39:50.110922443 +0200
@@ -36,34 +36,42 @@
 ;; key sequences without using modifier keys.
 
 ;;; Code:
+
+(defgroup devil '()
+  "Minor mode for Devil-like command entering." ;is there a clearer 
description?
+  :prefix "devil-"
+  :group 'editing)
+
 (defconst devil-version "0.2.0"
   "Devil version number.")
 
-(defun devil-show-version ()
+(defun devil-show-version ()           ;is this really needed?
   "Show Devil version number in the echo area."
   (interactive)
   (message "Devil %s" devil-version))
 
-(defvar devil-key ","
+(defcustom devil-key ","
   "The key sequence that begins Devil input.
 
 The key sequence must be specified in the format returned by `C-h
-k' (`describe-key'). This variable should be set before enabling
-Devil mode for it to take effect.")
-
-(defvar devil-lighter " Devil"
-  "String displayed on the mode line when Devil mode is enabled.")
+k' (`describe-key').  This variable should be set before enabling
+Devil mode for it to take effect."
+  :type 'key-sequence)
+
+(defcustom devil-lighter " Devil"
+  "String displayed on the mode line when Devil mode is enabled."
+  :type 'string)
 
 (defvar devil-mode-map
   (let ((map (make-sparse-keymap)))
-    (define-key map (kbd devil-key) #'devil)
+    (define-key map devil-key #'devil)
     map)
   "Keymap to wake up Devil when `devil-key' is typed.
 
 By default, only `devil-key' is added to this keymap so that
-Devil can be activated using it. To support multiple activation
+Devil can be activated using it.  To support multiple activation
 keys, this variable may be modified to a new keymap that defines
-multiple different keys to activate Devil. This variable should
+multiple different keys to activate Devil.  This variable should
 be modified before loading Devil for it to take effect.")
 
 ;;;###autoload
@@ -74,15 +82,16 @@
 
 ;;;###autoload
 (define-globalized-minor-mode
-  global-devil-mode devil-mode devil--on :group 'devil
+  global-devil-mode devil-mode devil--on ;; :group 'devil (not needed with the 
defgroup above)
   (if global-devil-mode (devil-add-extra-keys) (devil-remove-extra-keys)))
 
 (defun devil--on ()
   "Turn Devil mode on."
   (devil-mode 1))
 
-(defvar devil-logging nil
-  "Non-nil if and only if Devil should print log messages.")
+(defcustom devil-logging nil
+  "Non-nil if and only if Devil should print log messages."
+  :type 'boolean)
 
 (defvar devil-special-keys
   (list (cons "%k %k" (lambda () (interactive) (devil-run-key "%k")))
@@ -91,12 +100,12 @@
   "Special Devil keys that are executed as soon as they are typed.
 
 The value of this variable is an alist where each key represents
-a Devil key sequence. If a Devil key sequence matches any key in
+a Devil key sequence.  If a Devil key sequence matches any key in
 this alist, the function or lambda in the corresponding value is
-invoked. The format control specifier `%k' may be used to
+invoked.  The format control specifier `%k' may be used to
 represent `devil-key' in the keys.")
 
-(defvar devil-translations
+(defcustom devil-translations
   (list (cons "%k z" "C-")
         (cons "%k %k" "%k")
         (cons "%k m m" "M-")
@@ -108,12 +117,13 @@
 a translation rule that is applied on the Devil key sequence read
 from the user to obtain the Emacs key sequence to be executed.
 The translation rules are applied in the sequence they occur in
-the alist. For each rule, if the key occurs anywhere in the Devil
+the alist.  For each rule, if the key occurs anywhere in the Devil
 key sequence, it is replaced with the corresponding value in the
-translation rule. The format control specifier `%k' may be used
-to represent `devil-key' in the keys.")
+translation rule.  The format control specifier `%k' may be used
+to represent `devil-key' in the keys."
+  :type '(alist :key-type string :value-type string))
 
-(defvar devil-repeatable-keys
+(defcustom devil-repeatable-keys
   (list "%k p"
         "%k n"
         "%k f"
@@ -128,8 +138,9 @@
 
 The value of this variable is a list where each item represents a
 key sequence that may be repeated merely by typing the last
-character in the key sequence. The format control specified `%k'
-may be used to represent `devil-key' in the keys.")
+character in the key sequence.  The format control specified `%k'
+may be used to represent `devil-key' in the keys."
+  :type '(repeat string))
 
 (defun devil-run-key (key)
   "Execute the given key sequence KEY.
@@ -140,7 +151,7 @@
   (dolist (key (split-string key))
     (if (string= key "%k") (insert devil-key) (execute-kbd-macro (kbd key)))))
 
-(defvar devil--saved-keys
+(defvar devil--saved-keys nil          ;otherwise `devil--saved-keys' is 
assigned a string
   "Original key bindings saved by Devil.")
 
 (defun devil-add-extra-keys ()
@@ -160,8 +171,9 @@
 
 (defun devil--original-keys-to-be-saved ()
   "Return an alist of keys that will be modified by Devil."
-  (list (cons 'isearch-comma (lookup-key isearch-mode-map (kbd devil-key)))
-        (cons 'universal-u (lookup-key universal-argument-map (kbd "u")))))
+  ;; Weak suggestions
+  `((isearch-comma . ,(lookup-key isearch-mode-map (kbd devil-key)))
+    universal-u . ,(lookup-key universal-argument-map (kbd "u"))))
 
 (defun devil ()
   "Wake up Devil to read and translate Devil key sequences."
@@ -178,27 +190,30 @@
 after translation to Emacs key sequence.
 
 The argument KEY is a vector that represents the key sequence
-read so far. This function reads a new key from the user, appends
+read so far.  This function reads a new key from the user, appends
 it to KEY, and then checks if the result is a valid key sequence
-or an undefined key sequence. If the result is a valid key
+or an undefined key sequence.  If the result is a valid key
 sequence for a special key command or an Emacs command, then the
-command is executed. Otherwise, this function calls itself
+command is executed.  Otherwise, this function calls itself
 recursively to read yet another key from the user."
   (setq key (vconcat key (vector (read-key (devil--make-prompt key)))))
   (unless (devil--run-command key)
     (devil--read-key key)))
 
-(defvar devil-prompt "Devil: %t"
+(defcustom devil-prompt "Devil: %t"
   "A format control string that determines the Devil prompt.
 
 The following format control sequences are supported:
 
 %k - Devil key sequence read by Devil so far.
 %t - Emacs key sequence translated from Devil key sequence read so far.
-%% - The percent sign.")
+%% - The percent sign."
+  :type 'string)
 
 (defun devil--make-prompt (key)
   "Create Devil prompt based on the given KEY."
+  ;; If you are interested in adding Compat as a dependency, you can
+  ;; make use of `format-spec' without raining the minimum version.
   (let ((result devil-prompt)
         (controls (list (cons "%k" (key-description key))
                         (cons "%t" (devil-translate key))
@@ -210,16 +225,16 @@
 (defun devil--run-command (key)
   "Try running the command bound to the key sequence in KEY.
 
-KEY is a vector that represents a sequence of keystrokes. If KEY
+KEY is a vector that represents a sequence of keystrokes.  If KEY
 is found to be a special key in `devil-special-keys', the
 corresponding special command is executed immediately and t is
 returned.
 
 Otherwise, it is translated to an Emacs key sequence using
-`devil-translations'. If the resulting Emacs key sequence is
+`devil-translations'.  If the resulting Emacs key sequence is
 found to be a complete key sequence, the command it is bound to
-is executed interactively and t is returned. If it is found to be
-an undefined key sequence, then t is returned. If the resulting
+is executed interactively and t is returned.  If it is found to be
+an undefined key sequence, then t is returned.  If the resulting
 Emacs key sequence is found to be an incomplete key sequence,
 then nil is returned."
   (devil--log "Trying to execute key: %s" (key-description key))
@@ -231,7 +246,7 @@
 
 If the given key sequence KEY is found to be a special key in
 `devil-special-keys', the corresponding special command is
-executed, and t is returned. Otherwise nil is returned."
+executed, and t is returned.  Otherwise nil is returned."
   (catch 'break
     (dolist (entry devil-special-keys)
       (when (string= (key-description key) (devil-format (car entry)))
@@ -245,14 +260,14 @@
 
 After translating KEY to an Emacs key sequence, if the resulting
 key sequence turns out to be an incomplete key, then nil is
-returned. If it turns out to be a complete key sequence, the
-corresponding Emacs command is executed, and t is returned. If it
-turns out to be an undefined key sequence, t is returned. The
+returned.  If it turns out to be a complete key sequence, the
+corresponding Emacs command is executed, and t is returned.  If it
+turns out to be an undefined key sequence, t is returned.  The
 return value t indicates to the caller that no more Devil key
 sequences should be read from the user."
   (let* ((described-key (key-description key))
          (translated-key (devil-translate key))
-         (parsed-key (condition-case nil (kbd translated-key) (error nil)))
+         (parsed-key (condition-case nil (kbd translated-key) (error nil))) 
;or `ignore-errors'
          (binding (when parsed-key (key-binding parsed-key))))
     (cond ((string-match "[ACHMSs]-$" translated-key)
            (devil--log "Ignoring incomplete key: %s => %s"
@@ -307,27 +322,27 @@
   "Update variables that maintain command loop information.
 
 The given KEY and BINDING is used to update variables that
-maintain command loop information. This allows the commands that
+maintain command loop information.  This allows the commands that
 depend on them behave as if they were being invoked directly with
 the original Emacs key sequence."
   ;;
   ;; Set `last-command-event' so that `digit-argument' can determine
-  ;; the correct digit for key sequences like , 5 (C-5). See M-x
+  ;; the correct digit for key sequences like , 5 (C-5).  See M-x
   ;; find-function RET digit-argument RET for details.
   (setq last-command-event (aref key (- (length key) 1)))
   ;;
   ;; Set `this-command' to make several commands like , z SPC , z SPC
-  ;; (C-SPC C-SPC) and , p (C-p) work correctly. Emacs copies
-  ;; `this-command' to `last-command'. Both variables are used by
+  ;; (C-SPC C-SPC) and , p (C-p) work correctly.  Emacs copies
+  ;; `this-command' to `last-command'.  Both variables are used by
   ;; `set-mark-command' to decide whether to activate/deactivate the
-  ;; current mark. The first variable is used by vertical motion
-  ;; commands to keep the cursor at the `temporary-goal-column'. There
+  ;; current mark.  The first variable is used by vertical motion
+  ;; commands to keep the cursor at the `temporary-goal-column'.  There
   ;; may be other commands too that depend on this variable.
   (setq this-command binding)
   ;;
   ;; Set `real-this-command' to make , x z (C-x z) work correctly.
   ;; Emacs copies it to `last-repeatable-command' which is then used
-  ;; by repeat. See the following for more details:
+  ;; by repeat.  See the following for more details:
   ;;
   ;;   - M-x find-function RET repeat RET
   ;;   - C-h v last-repeatable-command RET
@@ -337,11 +352,12 @@
 (defun devil--log-command-loop-info ()
   "Log command loop information for debugging purpose."
   (devil--log
-   (concat "Found "
-           (format "current-prefix-arg: %s; " current-prefix-arg)
-           (format "this-command: %s; " this-command)
-           (format "last-command: %s; " last-command)
-           (format "last-repeatable-command: %s" last-repeatable-command))))
+   (format "Found current-prefix-arg: %s; \
+this-command: %s; last-command: %s; last-repeatable-command: %s"
+          current-prefix-arg
+          this-command
+          last-command
+          last-repeatable-command)))
 
 (defun devil--repeatable-key-p (described-key)
   "Return t iff DESCRIBED-KEY belongs to `devil-repeatable-keys'."
@@ -379,28 +395,25 @@
   (when devil-logging
     (apply #'message (concat "Devil: " format-string) args)))
 
-(defmacro devil--assert (form)
-  "Evaluate FORM and cause error if the result is nil."
-  `(unless ,form
-     (error "Assertion failed: %s" ',form)))
-
-(defun devil--tests ()
-  "Test Devil functions assuming Devil has not been customized."
-  (devil--assert (devil--invalid-key-p ""))
-  (devil--assert (devil--invalid-key-p "C-x-C-f"))
-  (devil--assert (devil--invalid-key-p "C-x CC-f"))
-  (devil--assert (not (devil--invalid-key-p "C-x C-f")))
-  (devil--assert (not (devil--invalid-key-p "C-M-x")))
-  (devil--assert (string= (devil-translate (vconcat ",")) "C-"))
-  (devil--assert (string= (devil-translate (vconcat ",x")) "C-x"))
-  (devil--assert (string= (devil-translate (vconcat ",x,")) "C-x C-"))
-  (devil--assert (string= (devil-translate (vconcat ",x,f")) "C-x C-f"))
-  (devil--assert (string= (devil-translate (vconcat ",,")) ","))
-  (devil--assert (string= (devil-translate (vconcat ",,,,")) ", ,"))
-  (devil--assert (string= (devil-translate (vconcat ",mx")) "C-M-x"))
-  (devil--assert (string= (devil-translate (vconcat ",mmx")) "M-x"))
-  (devil--assert (string= (devil-translate (vconcat ",mmm")) "M-m"))
-  (message "Done"))
+(ert-deftest devil-invalid-key-p ()
+  "Test if `devil--invalid-key-p' words as expected."
+  (should (devil--invalid-key-p ""))
+  (should (devil--invalid-key-p "C-x-C-f"))
+  (should (devil--invalid-key-p "C-x CC-f"))
+  (should (not (devil--invalid-key-p "C-x C-f")))
+  (should (not (devil--invalid-key-p "C-M-x"))))
+
+(ert-deftest devil-translate ()
+  "Test if `devil-translate' works as expected."
+  (should (string= (devil-translate (vconcat ",")) "C-"))
+  (should (string= (devil-translate (vconcat ",x")) "C-x"))
+  (should (string= (devil-translate (vconcat ",x,")) "C-x C-"))
+  (should (string= (devil-translate (vconcat ",x,f")) "C-x C-f"))
+  (should (string= (devil-translate (vconcat ",,")) ","))
+  (should (string= (devil-translate (vconcat ",,,,")) ", ,"))
+  (should (string= (devil-translate (vconcat ",mx")) "C-M-x"))
+  (should (string= (devil-translate (vconcat ",mmx")) "M-x"))
+  (should (string= (devil-translate (vconcat ",mmm")) "M-m")))
 
 (provide 'devil)
 

Diff finished.  Tue May  9 10:39:57 2023
> Also, see https://www.reddit.com/r/emacs/comments/13aj99j/ for some
> discussion on this new package.
>
> Please let me know if this package can be added to NonGNU ELPA. If there
> are any questions or feedback about this, please let me know.

I see no reason why not.  Thanks for contributing.

One point I have already mentioned on IRC is that a separate manual and
a shorter README would be nice.  We can generate a TeXinfo manual from
either .texi or .org files, would you be interested in setting that up?

> Regards,
> Susam
> From 3499d93502b130b1dcb8a648e73e7a614cd24abd Mon Sep 17 00:00:00 2001
> From: Susam Pal <susam@susam.net>
> Date: Tue, 9 May 2023 02:36:08 +0100
> Subject: [PATCH] * elpa-packages (devil): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index c333cc8bb3..f6fd68edaf 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -112,6 +112,8 @@
>  
>   (devhelp               :url "https://codeberg.org/akib/emacs-devhelp";)
>  
> + (devil                 :url "https://github.com/susam/devil";)

As you have a CHANGES.md file, we can add a :news entry here as well.

> +
>   (diff-ansi          :url "https://codeberg.org/ideasman42/emacs-diff-ansi";
>    :ignored-files ("LICENSE"))

reply via email to

[Prev in Thread] Current Thread [Next in Thread]