bug#20870: 24.5; Options presented in ispell *Choices* should be accessible via keyboard

bug#20870: 24.5; Options presented in ispell *Choices* should be accessible via keyboard

[Jürgen Hartmann]

When ispell offers more than 67 options in its buffer *Choices*, it uses keys
like ^? or \200 that are hard (or impossible) to enter via keyboard. Here is
an example of the *Choices* content in such a case, where Hunspell and a
German alternate dictionary was used to complete the word "Abf":

-----------------------------------------------------------------------------
(0) Abfälle  (1) Abfällen  (2) Abfahrt  (3) Abfahrten  (4) Abfahrtshänge
(5) Abfahrtshängen  (6) Abfahrtshang  (7) Abfahrtsläufer
(8) Abfahrtsläuferin  (9) Abfahrtsläuferinnen  (:) Abfahrtsläufern
(;) Abfahrtsrennen  (<) Abfahrtsstelle  (=) Abfahrtsstellen
(>) Abfahrtsstrecke  (@) Abfahrtsstrecken  (B) Abfall  (C) Abfalls
(D) Abfallaufbereitung  (E) Abfalleimer  (F) Abfalleimern
(G) Abfallgraben  (H) Abfallkurve  (I) Abfallkurven  (J) Abfallpapier
(K) Abfallpapiers  (L) Abfallprodukt  (M) Abfallprodukten
(N) Abfallprodukts  (O) Abfallproduktes  (P) Abfallstoffe
(Q) Abfallstoffen  (S) Abfangjäger  (T) Abfangjägern  (U) Abfangjägers
(V) Abfangrakete  (W) Abfangraketen  (Y) Abfassung  (Z) Abfassungen
([) Abfertigung  (\) Abfertigungen  (]) Abfertigungsschalter
(^) Abfertigungsschaltern  (_) Abfertigungsschalters
(`) Abfertigungsstelle  (b) Abfertigungsstellen  (c) Abfindung
(d) Abfindungen  (e) Abflachung  (f) Abflachungen  (g) Abflüge
(h) Abflügen  (j) Abflüsse  (k) Abflüssen  (n) Abflug  (o) Abflugs
(p) Abflugflughafen  (s) Abflugland  (t) Abfluß  (v) Abflusses
(w) Abfördermenge  (y) Abfördermengen  (z) Abfolge  ({) Abfolgen
(|) Abfragesprache  (}) Abfragesprachen  (~) Abführmittel
(^?) Abführmitteln  (\200) Abführmittels  (\201) Abführung
(\202) Abfüllanlage  (\203) Abfüllanlagen  (\204) Abfüllfiliale
(\205) Abfüllfilialen  (\206) Abfüllstation  (\207) Abfüllstationen
(\210) Abfuhr  (\211) Abfuhren
-----------------------------------------------------------------------------

It is not straight forward to reproduce this example, since it depends on
the chosen dictionary. But we can do it in laboratory conditions, if we
generate an artificial dictionary.

For that, just redirect the output of the following shell script to a file
that we call /tmp/dict:

-----------------------------------------------------------------------------
#!/bin/sh

letters="a b c d e f g h i"

for i in $letters; do
    for j in $letters; do
    echo "aaa$i$j"
    done
done
-----------------------------------------------------------------------------

With that we can go to Emacs:

* Open Emacs 24.5 via

     LC_ALL=C ./emacs -Q

* Load the ispell library: M-x load-library RET ispell RET

* Open customization buffer for variable ispell-alternate-dictionary:
  M-x customize-variable RET ispell-alternate-dictionary RET

* Enter /tmp/dict as the variable's value and accept it for the current
  session. (Remember: /tmp/dict is the dictionary that we just generated.)

* Make a new Buffer and call it "text": C-x b RET text RET

* Change major mode of this buffer to text-mode.

* Enter "aaa" (without quotes) in the first line of the buffer text.

* With point immediately after the third letter a, start ispell-complete-word
  by C-M-i.

This should bring you a *Choices* buffer with a contents like that:

------------------------------------------------------------
(0) aaaaa  (1) aaaab  (2) aaaac  (3) aaaad  (4) aaaae
(5) aaaaf  (6) aaaag  (7) aaaah  (8) aaaai  (9) aaaba
(:) aaabb  (;) aaabc  (<) aaabd  (=) aaabe  (>) aaabf
(@) aaabg  (B) aaabh  (C) aaabi  (D) aaaca  (E) aaacb
(F) aaacc  (G) aaacd  (H) aaace  (I) aaacf  (J) aaacg
(K) aaach  (L) aaaci  (M) aaada  (N) aaadb  (O) aaadc
(P) aaadd  (Q) aaade  (S) aaadf  (T) aaadg  (U) aaadh
(V) aaadi  (W) aaaea  (Y) aaaeb  (Z) aaaec  ([) aaaed
(\) aaaee  (]) aaaef  (^) aaaeg  (_) aaaeh  (`) aaaei
(b) aaafa  (c) aaafb  (d) aaafc  (e) aaafd  (f) aaafe
(g) aaaff  (h) aaafg  (j) aaafh  (k) aaafi  (n) aaaga
(o) aaagb  (p) aaagc  (s) aaagd  (t) aaage  (v) aaagf
(w) aaagg  (y) aaagh  (z) aaagi  ({) aaaha  (|) aaahb
(}) aaahc  (~) aaahd  (^?) aaahe  (\200) aaahf  (\201) aaahg
(\202) aaahh  (\203) aaahi  (\204) aaaia  (\205) aaaib
(\206) aaaic  (\207) aaaid  (\210) aaaie  (\211) aaaif
(\212) aaaig  (\213) aaaih  (\214) aaaii
------------------------------------------------------------

which shows the problem.

Juergen

 		 	   		  

bug#20870: 24.5; [PATCH] Options presented in ispell *Choices* should be accessible via keyboard

[Jürgen Hartmann]

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

The attached patch tries to offer a workaround for this bug and to provide a
feature that was asked for by Emanuel Berg in

   http://lists.gnu.org/archive/html/help-gnu-emacs/2015-06/msg00363.html

i.e. that Ispell uses certain keys in a certain order for the offers in its
*Choices* buffer.

Therefore, the idea is to use a customizable variable to hold a vector of key
specifications that are used by ispell-command-loop in the given order to
assemble its options.

The patch alters ispell-command-loop accordingly introducing
ispell-option-keys as that customizable variable. Its default value is chosen
such, that the first keys in the vector are digits and lowercase letters,
since these are directly accessible on most keyboards without using any
modifiers.

As a minor detail the patch defines the inline function ispell-insert-choices
to hold some code that is used repetitively in ispell-command-loop.

The patch applies to the version of ispell.el that was shipped with the Emacs
24.4 release. And that is also the Emacs version the patch was tested on.

Juergen

 		 	   		  

[-- Attachment #2: choices-patch.patch --]
[-- Type: application/octet-stream, Size: 12368 bytes --]

--- ispell.el_original	2014-03-21 06:34:40.000000000 +0100
+++ ispell.el_changed	2015-06-27 12:16:49.335740151 +0200
@@ -2196,6 +2196,62 @@
   (setq ispell-pdict-modified-p nil))
 
 
+(defcustom ispell-option-keys
+  ["0" "1" "2" "3" "4" "5" "6" "7" "8" "9" "b" "c" "d" "e" "f" "g" "h" "j"
+   "k" "n" "o" "p" "s" "t" "v" "w" "y" "z" "<" "^" "B" "C" "D" "E" "F" "G"
+   "H" "I" "J" "K" "L" "M" "N" "O" "P" "Q" "S" "T" "U" "V" "W" "Y" "Z" ":"
+   ";" "=" ">" "_" "`" "@" "[" "\\" "]" "{" "|" "}" "~" "C-0" "C-1" "C-2"
+   "C-3" "C-4" "C-5" "C-6" "C-7" "C-8" "C-9" "C-a" "C-b" "C-c" "C-d" "C-e"
+   "C-f" "C-i" "C-j" "C-k" "C-m" "C-n" "C-o" "C-p" "C-q" "C-s" "C-t" "C-u"
+   "C-v" "C-w" "C-x" "C-y" "C-<" "C-^" "C-A" "C-B" "C-C" "C-D" "C-E" "C-F"
+   "C-G" "C-H" "C-I" "C-J" "C-K" "C-L" "C-M" "C-N" "C-O" "C-P" "C-Q" "C-R"
+   "C-S" "C-T" "C-U" "C-V" "C-W" "C-X" "C-Y" "C-Z" "C-:" "C-;" "C-=" "C->"
+   "C-_" "C-`" "C-@" "C-[" "C-\\" "C-]" "C-{" "C-|" "C-}" "C-~"]
+  "Define the inventory of keys used in Ispell’s *Choices* buffer.
+
+Value is a vector of strings that describe keys when used as
+arguments in `kbd'. (Remember to escape backslashes: \"\\\\\" is
+as string with one backslash.) These keys are used in the given
+order to identify the options that Ispell offers in its *Choices*
+buffer.
+
+The number of keys defined here limits the number of options that
+Ispell will offer.
+
+It is in the responsibility of the user, not to include keys in
+this vector that Ispell’s command loop recognizes as commands.
+These are:
+
+   <SPC>, ?, A, R, X, a, i, l, m, q, r, u, x,
+   C-g, C-h, C-l, C-r, C-z
+
+and whatever key is associated with `help-char', which is in
+general equivalent to C-h. The documentation of `ispell-help'
+explains, what these command keys do."
+  :group 'ispell)
+
+
+(defsubst ispell-insert-choices ()
+  "Helper that inserts the options in Ispell's *Choices* buffer.
+
+The code of this inline function is used identically at several
+places in `ispell-command-loop'."
+  (while (and choices
+	      (< count nof-opt-keys)
+	      (< (if (> (+ 6 (current-column) (length (car choices))
+			   (length (setq key
+					 (elt ispell-option-keys count))))
+			(window-width))
+		     (progn
+		       (insert "\n")
+		       (setq line (1+ line)))
+		   line)
+		 max-lines))
+    (insert "(" key ") " (car choices) "  ")
+    (setq choices (cdr choices)
+	  count (1+ count))))
+
+
 (defun ispell-command-loop (miss guess word start end)
   "Display possible corrections from list MISS.
 GUESS lists possibly valid affix construction of WORD.
@@ -2210,17 +2266,19 @@
 indicates whether the dictionary has been modified when option `a'
 or `i' is used.
 Global `ispell-quit' set to start location to continue spell session."
-  (let ((count ?0)
+  (let ((count 0)
+	(nof-opt-keys (length ispell-option-keys))
 	(line ispell-choices-win-default-height)
 	;; ensure 4 context lines.
 	(max-lines (- (ispell-adjusted-window-height) 4))
 	(choices miss)
 	(window-min-height (min window-min-height
 				ispell-choices-win-default-height))
-	(command-characters '( ?  ?i ?a ?A ?r ?R ?? ?x ?X ?q ?l ?u ?m ))
+	(command-keys (list help-char
+			    ?  ?? ?A ?R ?X ?a ?i ?l ?m ?q ?r ?u ?x
+			    ?\C-g ?\C-h ?\C-l ?\C-r ?\C-z))
 	(dedicated (window-dedicated-p))
-	(skipped 0)
-	char num result textwin dedicated-win)
+	key num result textwin dedicated-win)
 
     ;; setup the *Choices* buffer with valid data.
     (with-current-buffer (get-buffer-create ispell-choices-buffer)
@@ -2250,24 +2308,7 @@
 	      (setq guess (cdr guess)))
 	    (insert "\nUse option `i' to accept this spelling and put it in your private dictionary.\n")
 	    (setq line (+ line (if choices 3 2)))))
-      (while (and choices
-		  (< (if (> (+ 7 (current-column) (length (car choices))
-			       (if (> count ?~) 3 0))
-			    (window-width))
-			 (progn
-			   (insert "\n")
-			   (setq line (1+ line)))
-		       line)
-		     max-lines))
-	;; not so good if there are over 20 or 30 options, but then, if
-	;; there are that many you don't want to scan them all anyway...
-	(while (memq count command-characters) ; skip command characters.
-	  (setq count (ispell-int-char (1+ count))
-		skipped (1+ skipped)))
-	(insert "(" count ") " (car choices) "  ")
-	(setq choices (cdr choices)
-	      count (ispell-int-char (1+ count))))
-      (setq count (ispell-int-char (- count ?0 skipped))))
+      (ispell-insert-choices))
 
     ;; ensure word is visible
     (if (not (pos-visible-in-window-p end))
@@ -2301,38 +2342,35 @@
 				     "unchanged, Character to replace word")))
 		  (let ((inhibit-quit t)
 			(input-valid t))
-		    (setq char nil skipped 0)
+		    (setq key nil)
 		    ;; If the user types C-g, or generates some other
-		    ;; non-character event (such as a frame switch
+		    ;; non-keyboard event (such as a frame switch
 		    ;; event), stop ispell.  As a special exception,
 		    ;; ignore mouse events occurring in the same frame.
-		    (while (and input-valid (not (characterp char)))
-		      (setq char (read-key))
+		    (while (and input-valid (not (natnump key)))
+		      (setq key (read-key))
 		      (setq input-valid
-			    (or (characterp char)
-				(and (mouse-event-p char)
+			    (or (natnump key)
+				(and (mouse-event-p key)
 				     (eq (selected-frame)
 					 (window-frame
-					  (posn-window (event-start char))))))))
-		    (when (or quit-flag (not input-valid) (= char ?\C-g))
-		      (setq char ?X quit-flag nil)))
-		  ;; Adjust num to array offset skipping command characters.
-		  (let ((com-chars command-characters))
-		    (while com-chars
-		      (if (and (> (car com-chars) ?0) (< (car com-chars) char))
-			  (setq skipped (1+ skipped)))
-		      (setq com-chars (cdr com-chars)))
-		    (setq num (- char ?0 skipped)))
+					  (posn-window (event-start key))))))))
+		    (when (or quit-flag (not input-valid) (= key ?\C-g))
+		      (setq key ?X quit-flag nil)))
+		  ;; We could look up key in ispell-option-keys right now.
+		  ;; But for performance reasons, we do this in the cond form
+		  ;; below and only after it has turned out, that key is none
+		  ;; of the command keys.
 
 		  (cond
-		   ((= char ? ) nil)	; accept word this time only
-		   ((= char ?i)		; accept and insert word into pers dict
+		   ((= key ? ) nil)	; accept word this time only
+		   ((= key ?i)		; accept and insert word into pers dict
 		    (ispell-send-string (concat "*" word "\n"))
 		    (setq ispell-pdict-modified-p '(t)) ; dictionary modified!
 		    (and (fboundp 'flyspell-unhighlight-at)
 			 (flyspell-unhighlight-at start))
 		    nil)
-		   ((or (= char ?a) (= char ?A)) ; accept word without insert
+		   ((or (= key ?a) (= key ?A)) ; accept word without insert
 		    (ispell-send-string (concat "@" word "\n"))
 		    (add-to-list 'ispell-buffer-session-localwords word)
 		    (and (fboundp 'flyspell-unhighlight-at)
@@ -2342,12 +2380,12 @@
 		    (if (null ispell-pdict-modified-p)
 			(setq ispell-pdict-modified-p
 			      (list ispell-pdict-modified-p)))
-		    (if (= char ?A) 0))	; return 0 for ispell-add buffer-local
-		   ((or (= char ?r) (= char ?R)) ; type in replacement
+		    (if (= key ?A) 0))	; return 0 for ispell-add buffer-local
+		   ((or (= key ?r) (= key ?R)) ; type in replacement
 		    (and (eq 'block ispell-highlight-p) ; refresh tty's
 			 (ispell-highlight-spelling-error start end nil t))
 		    (let ((result
-			   (if (or (= char ?R) ispell-query-replace-choices)
+			   (if (or (= key ?R) ispell-query-replace-choices)
 			       (list (read-string
 				      (format "Query-replacement for %s: "word)
 				      word)
@@ -2358,7 +2396,7 @@
 			   (ispell-highlight-spelling-error start end nil
 							    'block))
 		      result))
-		   ((or (= char ??) (= char help-char) (= char ?\C-h))
+		   ((or (= key ??) (= key help-char) (= key ?\C-h))
 		    (and (eq 'block ispell-highlight-p)
 			 (ispell-highlight-spelling-error start end nil t))
 		    (ispell-help)
@@ -2367,13 +2405,13 @@
 							  'block))
 		    t)
 		   ;; Quit and move point back.
-		   ((= char ?x)
+		   ((= key ?x)
 		    (ispell-pdict-save ispell-silently-savep)
 		    (message "Exited spell-checking")
 		    (setq ispell-quit t)
 		    nil)
 		   ;; Quit and preserve point.
-		   ((= char ?X)
+		   ((= key ?X)
 		    (ispell-pdict-save ispell-silently-savep)
 		    (message "%s"
 		     (substitute-command-keys
@@ -2381,7 +2419,7 @@
 			      " use C-u \\[ispell-word] to resume")))
 		    (setq ispell-quit start)
 		    nil)
-		   ((= char ?q)
+		   ((= key ?q)
 		    (if (y-or-n-p "Really kill Ispell process? ")
 			(progn
 			  (ispell-kill-ispell t) ; terminate process.
@@ -2389,7 +2427,7 @@
 						(point))
 				ispell-pdict-modified-p nil))
 		      t))		; continue if they don't quit.
-		   ((= char ?l)
+		   ((= key ?l)
 		    (and (eq 'block ispell-highlight-p) ; refresh tty displays
 			 (ispell-highlight-spelling-error start end nil t))
 		    (let ((new-word (read-string
@@ -2400,8 +2438,7 @@
 			    (with-current-buffer (get-buffer-create
                                                   ispell-choices-buffer)
 			      (erase-buffer)
-			      (setq count ?0
-				    skipped 0
+			      (setq count 0
 				    mode-line-format ;; setup the *Choices* buffer with valid data.
 				    (concat "--  %b  --  word: " new-word
 					    "  --  word-list: "
@@ -2410,46 +2447,25 @@
 				    miss (ispell-lookup-words new-word)
 				    choices miss
 				    line ispell-choices-win-default-height)
-			      (while (and choices ; adjust choices window.
-					  (< (if (> (+ 7 (current-column)
-						       (length (car choices))
-						       (if (> count ?~) 3 0))
-						    (window-width))
-						 (progn
-						   (insert "\n")
-						   (setq line (1+ line)))
-					       line)
-					     max-lines))
-				(while (memq count command-characters)
-				  (setq count (ispell-int-char (1+ count))
-					skipped (1+ skipped)))
-				(insert "(" count ") " (car choices) "  ")
-				(setq choices (cdr choices)
-				      count (ispell-int-char (1+ count))))
-			      (setq count (ispell-int-char
-					   (- count ?0 skipped))))
+			      (ispell-insert-choices))
 			    (ispell-show-choices line end)
 			    (select-window (next-window)))))
 		    (and (eq 'block ispell-highlight-p)
 			 (ispell-highlight-spelling-error start end nil
 							  'block))
 		    t)			; reselect from new choices
-		   ((= char ?u)		; insert lowercase into dictionary
+		   ((= key ?u)		; insert lowercase into dictionary
 		    (ispell-send-string (concat "*" (downcase word) "\n"))
 		    (setq ispell-pdict-modified-p '(t)) ; dictionary modified!
 		    nil)
-		   ((= char ?m)		; type in what to insert
+		   ((= key ?m)		; type in what to insert
 		    (ispell-send-string
 		     (concat "*" (read-string "Insert: " word) "\n"))
 		    (setq ispell-pdict-modified-p '(t))
 		    (cons word nil))
-		   ((and (>= num 0) (< num count))
-		    (if ispell-query-replace-choices ; Query replace flag
-			(list (nth num miss) 'query-replace)
-		      (nth num miss)))
-		   ((= char ?\C-l)
+		   ((= key ?\C-l)
 		    (redraw-display) t)
-		   ((= char ?\C-r)
+		   ((= key ?\C-r)
 		    ;; This may have alignment errors if current line is edited
 		    (if (marker-position ispell-recursive-edit-marker)
 			(progn
@@ -2480,10 +2496,27 @@
 			       "Cannot continue ispell from this buffer.")))
 			(set-marker ispell-recursive-edit-marker nil)))
 		    (list word nil))	; recheck starting at this word.
-		   ((= char ?\C-z)
+		   ((= key ?\C-z)
 		    (funcall (key-binding "\C-z"))
 		    t)
-		   (t (ding) t))))))
+		   (t
+		    ;; key is none of the command keys, but might be one of
+		    ;; those in ispell-option-keys: Look up.
+		    (setq num 0)
+		    (while (and (< num nof-opt-keys)
+				(not (equal
+				      (elt (kbd (elt ispell-option-keys num))
+					   0)
+				      key)))
+		      (setq num (1+ num)))
+		    ;; If key was found in ispell-option-keys, num equals its
+		    ;; index now. Otherwise num equals nof-opt-keys >= count.
+		    (if (< num count)
+			(if ispell-query-replace-choices ; Query replace flag
+			    (list (nth num miss) 'query-replace)
+			  (nth num miss))
+		      ;; else--key is not defined
+		      (ding) t)))))))
 	  result)
       ;; protected
       (and ispell-highlight-p		; unhighlight

bug#20870: 24.5; [PATCH] Options presented in ispell *Choices* should be accessible via keyboard

[Eli Zaretskii]

> From: Jürgen Hartmann <juergen_hartmann_@hotmail.com>
> Date: Sat, 27 Jun 2015 12:36:13 +0200
> 
> The attached patch tries to offer a workaround for this bug and to provide a
> feature that was asked for by Emanuel Berg in
> 
>    http://lists.gnu.org/archive/html/help-gnu-emacs/2015-06/msg00363.html
> 
> i.e. that Ispell uses certain keys in a certain order for the offers in its
> *Choices* buffer.
> 
> Therefore, the idea is to use a customizable variable to hold a vector of key
> specifications that are used by ispell-command-loop in the given order to
> assemble its options.

Thanks, but isn't it better to simply use numbers longer than one
digit?  E.g., if the number of choices is more than 10, use 00, 01,
etc.; if the number of choices is more than 100, use 000, 001,
etc., and so on and so forth?  Bonus points for being able to support
responses like 1 and 10 when the number of choices is more than 100
(but this is a nicety, not really required IMO).

I just cannot imagine anyone in the need for customizing this.

bug#20870: 24.5; [PATCH] Options presented in ispell *Choices* should be accessible via keyboard

[Stefan Monnier]

> Thanks, but isn't it better to simply use numbers longer than one
> digit?  E.g., if the number of choices is more than 10, use 00, 01,
> etc.; if the number of choices is more than 100, use 000, 001,
> etc., and so on and so forth?  Bonus points for being able to support
> responses like 1 and 10 when the number of choices is more than 100
> (but this is a nicety, not really required IMO).

That's right.  And we can even combine these numbers with other schemes
(e.g. ido/icomplete/avy style selection) if we want.


        Stefan

bug#20870: 24.5; [PATCH] Options presented in ispell *Choices* should be accessible via keyboard

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Therefore, the idea is to use a customizable variable to hold a vector of key
>> specifications that are used by ispell-command-loop in the given order to
>> assemble its options.
>
> Thanks, but isn't it better to simply use numbers longer than one
> digit?  E.g., if the number of choices is more than 10, use 00, 01,
> etc.; if the number of choices is more than 100, use 000, 001,
> etc., and so on and so forth?  Bonus points for being able to support
> responses like 1 and 10 when the number of choices is more than 100
> (but this is a nicety, not really required IMO).
>
> I just cannot imagine anyone in the need for customizing this.

I think that would be a better interface, too, so the patch isn't ideal,
and I'm closing this bug report.  If somebody wants to work on Eli's
suggestion, a new bug report would perhaps be appropriate.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no