bug#56323: 29.0.50; [v2] Add new customisable phonetic Tamil input method

[Eli Zaretskii <eliz@gnu.org>]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 56323@debbugs.gnu.org
> Date: Sun, 10 Jul 2022 09:26:39 +0530
> 
> > I will post an updated patch later when I clean up the comments, and
> > docstrings.  Thanks.
> 
> Here's an updated patch.

Thanks.

> +---
> +*** New default phonetic input method for the Tamil language environment.
> +The default input method for the Tamil language environment is now
> +"tamil" which is a customizable phonetic input method.  To change the
> +input method's translation rules, customize the user option
> +'tamil-translation-rules'.
> +
> 
>  * Changes in Specialized Modes and Packages in Emacs 29.1
> 
> diff --git a/lisp/language/indian.el b/lisp/language/indian.el
> index 2887d410ad..91ad818533 100644
> --- a/lisp/language/indian.el
> +++ b/lisp/language/indian.el
> @@ -109,7 +109,7 @@ 'devanagari
>   "Tamil" '((charset unicode)
>  	   (coding-system utf-8)
>  	   (coding-priority utf-8)
> -	   (input-method . "tamil-itrans")
> +	   (input-method . "tamil")
>             (sample-text . "Tamil (தமிழ்)	வணக்கம்")
>  	   (documentation . "\

Please name the new input method "tamil-phonetic", not just "tamil",
so that users who type "C-u C-\ tamil TAB" could have some means of
making the decision which one to choose.

> +;; This is needed since the Unicode codepoint order does not reflect
> +;; the actual order in the Tamil language.
> +(defvar quail-tamil-itrans--consonant-order
> +  '(("க" . 0) ("ங" . 1) ("ச" . 2) ("ஞ" . 3) ("ட" . 4) ("ண" . 5)
> +    ("த" . 6) ("ந" . 7) ("ப" . 8) ("ம" . 9) ("ய" . 10) ("ர" . 11)
> +    ("ல" . 12) ("வ" . 13) ("ழ" . 14) ("ள" . 15) ("ற" . 16) ("ன" . 17)
> +    ("ஜ" . 18) ("ஸ" . 19) ("ஷ" . 20) ("ஹ" . 21) ("க்ஷ" . 22)
> +    ("க்‌ஷ" . 23) ("ஶ" . 24)))

Since the characters are ordered in the correct order, I wonder why we
need the explicit ordinal numbers here: they are determined by the
index of the character in the list.

> +(defun quail-tamil-itrans-compute-syllable-table (vowels consonants)
> +  "Return the syllable table for the input method as a string.
> +VOWELS is a list of (VOWEL SIGN TRANS) where VOWEL is a string or
> +character representing the Tamil vowel character, SIGN is the
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
What does it mean "character representing ... character"?  Can you
clarify this confusing part of the doc string?

> +vowel sign corresponding to VOWEL or nil for none,

Likewise here: "vowel corresponding to VOWEL"?

>                                                    and TRANS is
> +the input sequence to insert VOWEL.

The input sequence is generally a sequence of ASCII characters, is
that right?  If so, I think telling that would make the documentation
more clear.  Also, TRANS is a peculiar name for something described as
"input sequence", so maybe rename it to INPUT-SEQ?

> +CONSONANTS is a list of (CONSONANT TRANS...) where CONSONANT is
> +the Tamil consonant character, and TRANS is one or more strings
> +that describe how to insert CONSONANT."

Same here regarding TRANS and its description.

> +  (setq vowels (sort vowels (lambda (x y) (string-lessp (car x) (car y))))
> +        consonants (sort consonants
> +                         (lambda (x y)
> +                           (< (or (assoc-default (car x) quail-tamil-itrans--consonant-order) 10000)
> +                              (or (assoc-default (car y) quail-tamil-itrans--consonant-order) 10000)))))

Can you wrap these long lines, so that they would be easier to read?

> +  (let ((digits "௦௧௨௩௪௫௬௭௮௯")
>  	(width 6) clm)
>      (with-temp-buffer
> -      (insert "\n" (make-string 18 ?-) "+")
> -      (when digitp (insert (make-string 60 ?-)))
> +      (insert "\n" (make-string 18 ?-))
> +      (when digitp
> +        (insert "+" (make-string 60 ?-)))
>        (insert "\n")
>        (insert
>         (propertize "\t" 'display '(space :align-to 5)) "various"
> -       (propertize "\t" 'display '(space :align-to 18)) "|")
> +       (propertize "\t" 'display '(space :align-to 18)))
>        (when digitp
>          (insert
> -         (propertize "\t" 'display '(space :align-to 45)) "digits"))
> -      (insert "\n" (make-string 18 ?-) "+")
> +          "|" (propertize "\t" 'display '(space :align-to 45)) "digits"))
> +      (insert "\n" (make-string 18 ?-))

Did you test those :align-to specs when display-line-numbers is in
use?

> +;;;
> +;;; Tamil phonetic input method
> +;;;
> +
> +;; Define the input method straightaway.
> +(quail-define-package "tamil" "Tamil" "ழ" t
> + "Customisable Tamil phonetic input method.

See above regarding the name of the input method.

> +    ;; Consonants.
> +    ("க்" "k" "g") ("ங்" "ng") ("ச்" "ch" "s") ("ஞ்" "nj") ("ட்" "t" "d")
> +    ("ண்" "N") ("த்" "th" "dh") ("ந்" "nh") ("ப்" "p" "b") ("ம்" "m")
> +    ("ய்" "y") ("ர்" "r") ("ல்" "l") ("வ்" "v") ("ழ்" "z" "zh")
> +    ("ள்" "L") ("ற்" "rh") ("ன்" "n")
> +    ;; Sanskrit.
> +    ("ஜ்" "j") ("ஸ்" "S") ("ஷ்" "sh") ("ஹ்" "h")
> +    ("க்‌ஷ்" "ksh") ("க்ஷ்" "ksH") ("ஶ்" "Z")
> +
> +    ;; Misc.  ஃ is neither a consonant nor a vowel.
> +    ("ஃ" "F" "q")
> +    ("ௐ" "OM"))
> +  "List of input sequences to translate to Tamil characters.
> +Each element should be (CHARACTER . TRANSLATIONS) where CHARACTER

The (CHARACTER . TRANSLATIONS) form seems to imply the elements are
cons cells, but the value itself uses lists.  Suggest to say instead

  Each element should be (CHARACTER TRANSLATIONS...)

> +is the Tamil character, and TRANSLATIONS is a list of input
> +sequences to translate to that character.
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"sequences which produce that character" is better.  And I suggest to
use INPUT-SEQUENCES here, not TRANSLATIONS, for the reason explained
above.

> +CHARACTER is considered as a consonant (மெய் எழுத்து) if it ends
> +with a pulli.

What is a "pulli"?  It is not a character name AFAICT.

> +CHARACTER is that is neither a vowel nor a consonant are
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Typo and/or redundant words here.

> +considered as \"miscellaneous\" characters and are inserted as
> +is.

Not sure what this wants to say: the fact that characters are inserted
in some way seems to be unrelated to the description of the value.
What is this about?

> +The input sequence for consonant+vowel pairs (உயிர்மெய் எழுத்துக்கள்)
> +is the input sequence for the consonant followed by the
> +corresponding vowel."

Isn't that obvious?  If not, the non-obvious part(s) should be
mentioned explicitly.

> +  :group 'tamil-input
> +  :type '(alist :key-type string :value-type (repeat string))
> +  :set #'tamil--setter
> +  :options

This defcustom lacks the :version tag.

> [ Also, I don't see the customization group until I load
>   lisp/leim/quail/indian.el?  But AFAICT, that's not the case for other
>   custom groups.  ]

There are no defcustoms in leim/quail/ files.  How about moving the
defcustom to lisp/language/indian.el?