unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* defconst in lao.el
@ 2004-11-14 18:56 Luc Teirlinck
  2004-11-14 21:06 ` Stefan Monnier
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-14 18:56 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1596 bytes --]

I believe that all defconst's in lao.el should be defvar's.  Several
are changed in the file itself and produce compiler warnings.  I
propose the patch below.  I am not sure what to do about the
Copyright:

;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
;; Licensed to the Free Software Foundation.

===File ~/lao.el-diff=======================================
*** lao.el	01 Sep 2003 16:08:19 -0500	1.8
--- lao.el	14 Nov 2004 12:41:09 -0600	
***************
*** 42,48 ****
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defconst lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
--- 42,48 ----
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defvar lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
***************
*** 148,159 ****
      ("\\9" . "໙")
      ))
  
! (defconst lao-consonant-key-alist nil)
! (defconst lao-semivowel-key-alist nil)
! (defconst lao-vowel-key-alist nil)
! (defconst lao-voweltone-key-alist nil)
! (defconst lao-tone-key-alist nil)
! (defconst lao-other-key-alist nil)
  
  (let ((tail lao-key-alist)
        elt phonetic-type)
--- 148,159 ----
      ("\\9" . "໙")
      ))
  
! (defvar lao-consonant-key-alist nil)
! (defvar lao-semivowel-key-alist nil)
! (defvar lao-vowel-key-alist nil)
! (defvar lao-voweltone-key-alist nil)
! (defvar lao-tone-key-alist nil)
! (defvar lao-other-key-alist nil)
  
  (let ((tail lao-key-alist)
        elt phonetic-type)
============================================================

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-14 18:56 defconst in lao.el Luc Teirlinck
@ 2004-11-14 21:06 ` Stefan Monnier
  2004-11-14 21:47   ` Luc Teirlinck
  2004-11-15 14:00 ` Richard Stallman
  2004-11-17  1:41 ` Kenichi Handa
  2 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2004-11-14 21:06 UTC (permalink / raw)
  Cc: emacs-devel


> @@ -42,7 +42,7 @@
>  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
>    control-flag)
>  
> -(defconst lao-key-alist
> +(defvar lao-key-alist
>    '(("!" . "1")
>      ("\"" . "=")
>      ("#" . "3")

This one also looks wrong: this is treated as a constant as far as
I can tell.


        Stefan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-14 21:06 ` Stefan Monnier
@ 2004-11-14 21:47   ` Luc Teirlinck
  0 siblings, 0 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-14 21:47 UTC (permalink / raw)
  Cc: emacs-devel

Stefan Monnier wrote:

   > @@ -42,7 +42,7 @@
   >  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
   >    control-flag)
   >  
   > -(defconst lao-key-alist
   > +(defvar lao-key-alist
   >    '(("!" . "1")
   >      ("\"" . "=")
   >      ("#" . "3")

   This one also looks wrong: this is treated as a constant as far as
   I can tell.

This is again difficult to say, since for all related variables the
defconst was clearly wrong.  It indeed is less clear for this one and
it produces no compiler warnings.  Again, I will _keep_ the defconst,
to err on the consevative side in terms of making changes to code that
I am not really terribly familiar with.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-14 18:56 defconst in lao.el Luc Teirlinck
  2004-11-14 21:06 ` Stefan Monnier
@ 2004-11-15 14:00 ` Richard Stallman
  2004-11-17  1:41 ` Kenichi Handa
  2 siblings, 0 replies; 16+ messages in thread
From: Richard Stallman @ 2004-11-15 14:00 UTC (permalink / raw)
  Cc: emacs-devel

      I am not sure what to do about the
    Copyright:

    ;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
    ;; Licensed to the Free Software Foundation.

It is correct; don't change it.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-14 18:56 defconst in lao.el Luc Teirlinck
  2004-11-14 21:06 ` Stefan Monnier
  2004-11-15 14:00 ` Richard Stallman
@ 2004-11-17  1:41 ` Kenichi Handa
  2004-11-17  3:01   ` Luc Teirlinck
                     ` (3 more replies)
  2 siblings, 4 replies; 16+ messages in thread
From: Kenichi Handa @ 2004-11-17  1:41 UTC (permalink / raw)
  Cc: emacs-devel

In article <200411141856.iAEIuEM24284@raven.dms.auburn.edu>, Luc Teirlinck <teirllm@dms.auburn.edu> writes:
> I believe that all defconst's in lao.el should be
> defvar's.  Several are changed in the file itself and
> produce compiler warnings.  I propose the patch below.  I
> am not sure what to do about the Copyright:

> ;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
> ;; Licensed to the Free Software Foundation.

Please keep it as is and add a new Copyright line below it
(see leim/quail/latin-post.el) if you modify this file.

But...

> ! (defconat lao-key-alist
...
> ! (defvar lao-key-alist

The original defconst doesn't yield a warning, and this
variable is surely not modified later.

> ! (defvar lao-consonant-key-alist nil)
> ! (defvar lao-semivowel-key-alist nil)
> ! (defvar lao-vowel-key-alist nil)
> ! (defvar lao-voweltone-key-alist nil)
> ! (defvar lao-tone-key-alist nil)
> ! (defvar lao-other-key-alist nil)
  
I used "defconst" for them because they have to be set back
to nil when lao.el is re-loaded.  They are setup in the
following (let ...) form to correct values.  If we change
them to "defvar", they are anyway set to nil in that (let
...) form.  And, conceptually, they are "constant".  So I
think "defconst" is better.

Then, how to avoid the compiler warnings?  The problem is
that their initial values are calculated at the same time
(not one by one) in the "let" form.

---
Ken'ichi HANDA
handa@m17n.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  1:41 ` Kenichi Handa
@ 2004-11-17  3:01   ` Luc Teirlinck
  2004-11-17  6:39     ` Stephan Stahl
  2004-11-17  3:07   ` Luc Teirlinck
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17  3:01 UTC (permalink / raw)
  Cc: emacs-devel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]

Kenichi Handa wrote:

   I used "defconst" for them because they have to be set back
   to nil when lao.el is re-loaded.  They are setup in the
   following (let ...) form to correct values.  If we change
   them to "defvar", they are anyway set to nil in that (let
   ...) form.  And, conceptually, they are "constant".  So I
   think "defconst" is better.

Yes, but I believe that the defconst is misleading.  When I see:

(defconst var value)

I get the impression that VAR is set for ever after to VALUE.

   It's possible that a user provide different initial value
   for this variable in his .emacs.  In that sense, defvar may
   be better.  On the other hand, modifying this value after
   lao.el is loaded doesn't work.  In that sense, it seems that
   defconst is better.

Yes, but the code can be changed so that modifying the value after
lao.el is loaded _does_ work.

What about the following patch?  It avoids the misleading defconst's,
the compiler warnings, and allows the sophisticated user to set
`lao-key-alist' to a different value in his .emacs, even if lao.el is
already loaded.

===File ~/lao-diff==========================================
*** lao.el	01 Sep 2003 16:08:19 -0500	1.8
--- lao.el	16 Nov 2004 20:40:48 -0600	
***************
*** 2,7 ****
--- 2,8 ----
  
  ;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
  ;; Licensed to the Free Software Foundation.
+ ;; Copyright (C) 2004 Free Software Foundation.
  
  ;; Keywords: multilingual, input method, Lao
  
***************
*** 42,48 ****
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defconst lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
--- 43,52 ----
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! ;; If you change the value of this variable, be sure to do
! ;; `(lao-initialize-alists)' afterwards.  Otherwise, the related
! ;; alists will not be properly updated.
! (defvar lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
***************
*** 148,183 ****
      ("\\9" . "໙")
      ))
  
! (defconst lao-consonant-key-alist nil)
! (defconst lao-semivowel-key-alist nil)
! (defconst lao-vowel-key-alist nil)
! (defconst lao-voweltone-key-alist nil)
! (defconst lao-tone-key-alist nil)
! (defconst lao-other-key-alist nil)
! 
! (let ((tail lao-key-alist)
!       elt phonetic-type)
!   (while tail
!     (setq elt (car tail) tail (cdr tail))
!     (if (stringp (cdr elt))
! 	(setq phonetic-type (get-char-code-property (aref (cdr elt) 0)
  						    'phonetic-type))
!       (setq phonetic-type (get-char-code-property (aref (aref (cdr elt) 0) 0)
! 						  'phonetic-type))
!       (aset (cdr elt) 0 (compose-string (aref (cdr elt) 0))))
!     (cond ((eq phonetic-type 'consonant)
! 	   (setq lao-consonant-key-alist (cons elt lao-consonant-key-alist)))
! 	  ((memq phonetic-type '(vowel-upper vowel-lower))
! 	   (if (stringp (cdr elt))
! 	       (setq lao-vowel-key-alist (cons elt lao-vowel-key-alist))
! 	     (setq lao-voweltone-key-alist
! 		   (cons elt lao-voweltone-key-alist))))
! 	  ((eq  phonetic-type 'tone)
! 	   (setq lao-tone-key-alist (cons elt lao-tone-key-alist)))
! 	  ((eq phonetic-type 'semivowel-lower)
! 	   (setq lao-semivowel-key-alist (cons elt lao-semivowel-key-alist)))
! 	  (t
! 	   (setq lao-other-key-alist (cons elt lao-other-key-alist))))))
  
  (quail-define-package
   "lao" "Lao" "ລ" t
--- 152,200 ----
      ("\\9" . "໙")
      ))
  
! (defvar lao-consonant-key-alist nil)
! (defvar lao-semivowel-key-alist nil)
! (defvar lao-vowel-key-alist nil)
! (defvar lao-voweltone-key-alist nil)
! (defvar lao-tone-key-alist nil)
! (defvar lao-other-key-alist nil)
! 
! ;;; autoload
! (defun lao-initialize-alists ()
!   "Properly initialize the lao alists.
! If you change `lao-key-alist', you have to call this function to
! make sure that all related lao alists are properly updated."
!   (setq lao-consonant-key-alist nil
! 	lao-semivowel-key-alist nil
! 	lao-vowel-key-alist nil
! 	lao-voweltone-key-alist nil
! 	lao-tone-key-alist nil
! 	lao-other-key-alist nil)
!   (let ((tail lao-key-alist)
! 	elt phonetic-type)
!     (while tail
!       (setq elt (car tail) tail (cdr tail))
!       (if (stringp (cdr elt))
! 	  (setq phonetic-type (get-char-code-property (aref (cdr elt) 0)
! 						      'phonetic-type))
! 	(setq phonetic-type (get-char-code-property (aref (aref (cdr elt) 0) 0)
  						    'phonetic-type))
! 	(aset (cdr elt) 0 (compose-string (aref (cdr elt) 0))))
!       (cond ((eq phonetic-type 'consonant)
! 	     (setq lao-consonant-key-alist (cons elt lao-consonant-key-alist)))
! 	    ((memq phonetic-type '(vowel-upper vowel-lower))
! 	     (if (stringp (cdr elt))
! 		 (setq lao-vowel-key-alist (cons elt lao-vowel-key-alist))
! 	       (setq lao-voweltone-key-alist
! 		     (cons elt lao-voweltone-key-alist))))
! 	    ((eq  phonetic-type 'tone)
! 	     (setq lao-tone-key-alist (cons elt lao-tone-key-alist)))
! 	    ((eq phonetic-type 'semivowel-lower)
! 	     (setq lao-semivowel-key-alist (cons elt lao-semivowel-key-alist)))
! 	    (t
! 	     (setq lao-other-key-alist (cons elt lao-other-key-alist)))))))
! 
! (unless (featurep 'lao) (lao-initialize-alists))
  
  (quail-define-package
   "lao" "Lao" "ລ" t
***************
*** 197,201 ****
--- 214,220 ----
      (v-state (lao-vowel-key-alist . t-state))
      (t-state lao-tone-key-alist))))
  
+ (provide 'lao)
+ 
  ;;; arch-tag: 23863a30-a8bf-402c-b7ce-c517a7aa8570
  ;;; lao.el ends here
============================================================

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  1:41 ` Kenichi Handa
  2004-11-17  3:01   ` Luc Teirlinck
@ 2004-11-17  3:07   ` Luc Teirlinck
  2004-11-17  3:57   ` Luc Teirlinck
  2004-11-17 14:17   ` Stefan Monnier
  3 siblings, 0 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17  3:07 UTC (permalink / raw)
  Cc: emacs-devel

Maybe I did send my previous patch too quickly.  Does proper
re-initialization of the alists also require `quail-define-package'
and `quail-install-map' to be called again with the correct arguments?

Then an alternate patch would work.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  1:41 ` Kenichi Handa
  2004-11-17  3:01   ` Luc Teirlinck
  2004-11-17  3:07   ` Luc Teirlinck
@ 2004-11-17  3:57   ` Luc Teirlinck
  2004-11-17 14:17   ` Stefan Monnier
  3 siblings, 0 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17  3:57 UTC (permalink / raw)
  Cc: emacs-devel

>From my previous message:

   Maybe I did send my previous patch too quickly.  Does proper
   re-initialization of the alists also require `quail-define-package'
   and `quail-install-map' to be called again with the correct arguments?

   Then an alternate patch would work.

More concretely, put any of the two that are needed for proper
re-initialization inside the body of `lao-initialize-alists'.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  3:01   ` Luc Teirlinck
@ 2004-11-17  6:39     ` Stephan Stahl
  2004-11-17 14:26       ` Luc Teirlinck
  0 siblings, 1 reply; 16+ messages in thread
From: Stephan Stahl @ 2004-11-17  6:39 UTC (permalink / raw)
  Cc: emacs-devel, handa

Hi Luc.

Luc Teirlinck said:

> Yes, but the code can be changed so that modifying the value after
> lao.el is loaded _does_ work.
[....]
> ! ;; If you change the value of this variable, be sure to do
> ! ;; `(lao-initialize-alists)' afterwards.  Otherwise, the related
> ! ;; alists will not be properly updated.
> ! (defvar lao-key-alist
>     '(("!" . "1")
>       ("\"" . "=")
>       ("#" . "3")
[....]

Wouldn't it be better to include this comment as lao-key-alist's doc-string or
maybe even make it a defcustom with an :set that calls lao-initialize-alists ?
I think this would make it harder to miss the fact that lao-initialize-alists
has to be called.

Stephan
-- 
Stephan Stahl

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  1:41 ` Kenichi Handa
                     ` (2 preceding siblings ...)
  2004-11-17  3:57   ` Luc Teirlinck
@ 2004-11-17 14:17   ` Stefan Monnier
  2004-11-17 18:35     ` Luc Teirlinck
  3 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2004-11-17 14:17 UTC (permalink / raw)
  Cc: teirllm, emacs-devel

> I used "defconst" for them because they have to be set back
> to nil when lao.el is re-loaded.  They are setup in the
> following (let ...) form to correct values.  If we change
> them to "defvar", they are anyway set to nil in that (let
> ...) form.  And, conceptually, they are "constant".  So I
> think "defconst" is better.

> Then, how to avoid the compiler warnings?  The problem is
> that their initial values are calculated at the same time
> (not one by one) in the "let" form.

The "right" answer is to put the defconst in the body of the let.
Of course if you do that, you get complaints from the byte-compiler because
it doesn't see those defconst as being at the top-level and thus doesn't
relize that they are *always* defined.

We should fix this in the byte-compiler but nobody's done it yet.
But you can work around those warnings by adding a couple (defvar foo)
before the let.  Ugly, tho.


        Stefan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17  6:39     ` Stephan Stahl
@ 2004-11-17 14:26       ` Luc Teirlinck
  2004-11-17 22:05         ` Luc Teirlinck
  0 siblings, 1 reply; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17 14:26 UTC (permalink / raw)
  Cc: handa, emacs-devel

Stephan Stahl wrote:

   Wouldn't it be better to include this comment as lao-key-alist's
   doc-string or maybe even make it a defcustom with an :set that
   calls lao-initialize-alists ?  I think this would make it harder to
   miss the fact that lao-initialize-alists has to be called.

If I understand correctly, the probability that somebody might want to
customize `lao-key-alist' is just big enough not to remove the
possibility of doing so by using defconst.  On the other hand, it
might not be worth a defcustom.  It might not be worth an autoload
either, as my original patch does.  Somebody wanting to change
`lao-key-alists' would probably have to read the source code anyway as
it stands.  Maybe somebody more familiar with lao.el than me might
want to write a proper docstring including the comment.

Also, with my original patch, there is the (small) problem that
`lao-initialize-alists' needlessly gets called twice if lao.el is
already loaded.  Maybe the following smaller patch is actually better.
Asking people to re-load a file instead of calling a function is
somewhat unusual, but the situation may occur seldom enough that this
does not really matter too much in this case.

===File ~/lao-newdiff=======================================
diff -c /home/teirllm/emacscvsdir/emacs/leim/quail/lao.el.\~1.8.\~ /home/teirllm/emacscvsdir/emacs/leim/quail/lao-alt.el
*** /home/teirllm/emacscvsdir/emacs/leim/quail/lao.el.~1.8.~	Mon Sep  1 16:08:19 2003
--- /home/teirllm/emacscvsdir/emacs/leim/quail/lao-alt.el	Wed Nov 17 07:57:56 2004
***************
*** 2,7 ****
--- 2,8 ----
  
  ;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
  ;; Licensed to the Free Software Foundation.
+ ;; Copyright (C) 2004 Free Software Foundation.
  
  ;; Keywords: multilingual, input method, Lao
  
***************
*** 42,48 ****
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defconst lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
--- 43,53 ----
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! ;; If you change the value of this variable, be sure to execute
! ;; `(when (featurep 'lao) (load "lao"))' afterward.  If lao is already
! ;; loaded, it needs to be re-loaded to properly re-initialize related
! ;; alists.
! (defvar lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
***************
*** 148,159 ****
      ("\\9" . "^[(1y^[(B")
      ))
  
! (defconst lao-consonant-key-alist nil)
! (defconst lao-semivowel-key-alist nil)
! (defconst lao-vowel-key-alist nil)
! (defconst lao-voweltone-key-alist nil)
! (defconst lao-tone-key-alist nil)
! (defconst lao-other-key-alist nil)
  
  (let ((tail lao-key-alist)
        elt phonetic-type)
--- 153,172 ----
      ("\\9" . "^[(1y^[(B")
      ))
  
! (defvar lao-consonant-key-alist nil)
! (defvar lao-semivowel-key-alist nil)
! (defvar lao-vowel-key-alist nil)
! (defvar lao-voweltone-key-alist nil)
! (defvar lao-tone-key-alist nil)
! (defvar lao-other-key-alist nil)
! 
! ;; These need to be re-initialized if lao is re-loaded.
! (setq lao-consonant-key-alist nil
!       lao-semivowel-key-alist nil
!       lao-vowel-key-alist nil
!       lao-voweltone-key-alist nil
!       lao-tone-key-alist nil
!       lao-other-key-alist nil)
  
  (let ((tail lao-key-alist)
        elt phonetic-type)
***************
*** 197,201 ****
--- 210,216 ----
      (v-state (lao-vowel-key-alist . t-state))
      (t-state lao-tone-key-alist))))
  
+ (provide 'lao)
+ 
  ;;; arch-tag: 23863a30-a8bf-402c-b7ce-c517a7aa8570
  ;;; lao.el ends here

Diff finished.  Wed Nov 17 07:58:47 2004
============================================================

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17 14:17   ` Stefan Monnier
@ 2004-11-17 18:35     ` Luc Teirlinck
  2004-11-17 19:03       ` Stefan Monnier
  2004-12-04  2:49       ` Kenichi Handa
  0 siblings, 2 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17 18:35 UTC (permalink / raw)
  Cc: emacs-devel, handa

Stefan Monnier wrote:

   The "right" answer is to put the defconst in the body of the let.

I do believe that it is better to put a defconst at top level.
Putting it elsewhere is probably going to give worse problems than
just compiler warnings, anyway.

One can always do that as follows (without compiler warnings):

(defvar temp-my-const temp-value)
;; we need to re-initialize temp-my-const when this file is re-loaded.
(setq temp-my-const temp-value)

... initializing code....

(defconst my-const temp-my-const)

Note that the defvar-setq pair is necessary because, unlike CL, Elisp
has no defparameter.

In the case of lao.el however, I believe that using no defconst's at
all is better, because Handa has indicated that it actually might make
sense for a user to change `lao-key-alist' in his .emacs, in which
case defconst could lead to trouble.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17 18:35     ` Luc Teirlinck
@ 2004-11-17 19:03       ` Stefan Monnier
  2004-12-04  2:49       ` Kenichi Handa
  1 sibling, 0 replies; 16+ messages in thread
From: Stefan Monnier @ 2004-11-17 19:03 UTC (permalink / raw)
  Cc: emacs-devel, handa

> I do believe that it is better to put a defconst at top level.
> Putting it elsewhere is probably going to give worse problems than
> just compiler warnings, anyway.

Not at all, `defconst' within a `let' works just fine.


        Stefan "what we want really is SML's `local'"

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17 14:26       ` Luc Teirlinck
@ 2004-11-17 22:05         ` Luc Teirlinck
  0 siblings, 0 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-11-17 22:05 UTC (permalink / raw)
  Cc: stahl, emacs-devel, handa

>From my previous message:

   ! ;; If you change the value of this variable, be sure to execute
   ! ;; `(when (featurep 'lao) (load "lao"))' afterward.  If lao is already
   ! ;; loaded, it needs to be re-loaded to properly re-initialize related
   ! ;; alists.
   ! (defvar lao-key-alist

On second thought, the concrete code to be executed depends on the
situation.  So I believe that the following comment is better:

;; If you change the value of this variable while lao is already loaded,
;; you need to re-load it to properly re-initialize related alists.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-11-17 18:35     ` Luc Teirlinck
  2004-11-17 19:03       ` Stefan Monnier
@ 2004-12-04  2:49       ` Kenichi Handa
  2004-12-04  3:43         ` Luc Teirlinck
  1 sibling, 1 reply; 16+ messages in thread
From: Kenichi Handa @ 2004-12-04  2:49 UTC (permalink / raw)
  Cc: monnier, emacs-devel

Sorry for not responding on this thread long.

In article <200411171835.iAHIZls25178@raven.dms.auburn.edu>, Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> In the case of lao.el however, I believe that using no defconst's at
> all is better, because Handa has indicated that it actually might make
> sense for a user to change `lao-key-alist' in his .emacs, in which
> case defconst could lead to trouble.

I agree.

> On second thought, the concrete code to be executed depends on the
> situation.  So I believe that the following comment is better:

> ;; If you change the value of this variable while lao is already loaded,
> ;; you need to re-load it to properly re-initialize related alists.

Ok, I put that sentence in the docstring of lao-key-alist.

On the other hand, lao-consonant-key-alist, and etc. should
be constant.  So, I've just installed the attached change.
I introduced a temporary variable lao-key-alist-vector to
initialize lao-consonant-key-alist, etc., and makunbound it
after the initialization (the same method that Dave used in
utf-16.el).

What do you think?

---
Ken'ichi HANDA
handa@m17n.org

2004-12-04  Kenichi Handa  <handa@m17n.org>

	* quail/lao.el (lao-key-alist): Declare it by defvar.
	(lao-key-alist-vector): New variable.
	(lao-consonant-key-alist, lao-semivowel-key-alist)
	(lao-vowel-key-alist, lao-voweltone-key-alist)
	(lao-tone-key-alist, lao-other-key-alist): Initialize them from
	lao-key-alist-vector.

Index: lao.el
===================================================================
RCS file: /cvsroot/emacs/emacs/leim/quail/lao.el,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -c -r1.8 -r1.9
cvs diff: conflicting specifications of output style
*** lao.el	1 Sep 2003 15:45:02 -0000	1.8
--- lao.el	4 Dec 2004 02:45:47 -0000	1.9
***************
*** 2,7 ****
--- 2,10 ----
  
  ;; Copyright (C) 1997 Electrotechnical Laboratory, JAPAN.
  ;; Licensed to the Free Software Foundation.
+ ;; Copyright (C) 2004
+ ;;   National Institute of Advanced Industrial Science and Technology (AIST)
+ ;;   Registration Number H14PRO021
  
  ;; Keywords: multilingual, input method, Lao
  
***************
*** 42,48 ****
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defconst lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
--- 45,51 ----
  	  (compose-string (quail-lookup-map-and-concat quail-current-key))))
    control-flag)
  
! (defvar lao-key-alist
    '(("!" . "1")
      ("\"" . "=")
      ("#" . "3")
***************
*** 146,183 ****
      ("\\7" . "໗")
      ("\\8" . "໘")
      ("\\9" . "໙")
!     ))
! 
! (defconst lao-consonant-key-alist nil)
! (defconst lao-semivowel-key-alist nil)
! (defconst lao-vowel-key-alist nil)
! (defconst lao-voweltone-key-alist nil)
! (defconst lao-tone-key-alist nil)
! (defconst lao-other-key-alist nil)
! 
! (let ((tail lao-key-alist)
!       elt phonetic-type)
!   (while tail
!     (setq elt (car tail) tail (cdr tail))
!     (if (stringp (cdr elt))
! 	(setq phonetic-type (get-char-code-property (aref (cdr elt) 0)
  						    'phonetic-type))
!       (setq phonetic-type (get-char-code-property (aref (aref (cdr elt) 0) 0)
! 						  'phonetic-type))
!       (aset (cdr elt) 0 (compose-string (aref (cdr elt) 0))))
!     (cond ((eq phonetic-type 'consonant)
! 	   (setq lao-consonant-key-alist (cons elt lao-consonant-key-alist)))
! 	  ((memq phonetic-type '(vowel-upper vowel-lower))
! 	   (if (stringp (cdr elt))
! 	       (setq lao-vowel-key-alist (cons elt lao-vowel-key-alist))
! 	     (setq lao-voweltone-key-alist
! 		   (cons elt lao-voweltone-key-alist))))
! 	  ((eq  phonetic-type 'tone)
! 	   (setq lao-tone-key-alist (cons elt lao-tone-key-alist)))
! 	  ((eq phonetic-type 'semivowel-lower)
! 	   (setq lao-semivowel-key-alist (cons elt lao-semivowel-key-alist)))
! 	  (t
! 	   (setq lao-other-key-alist (cons elt lao-other-key-alist))))))
  
  (quail-define-package
   "lao" "Lao" "ລ" t
--- 149,198 ----
      ("\\7" . "໗")
      ("\\8" . "໘")
      ("\\9" . "໙")
!     )
!   "Alist of key sequences vs the corresponding Lao string to input.
! This variable is for the input method \"lao\".
! If you change the value of this variable while quail/lao is already loaded,
! you need to re-load it to properly re-initialize related alists.")
! 
! ;; Temporary variable to initialize lao-consonant-key-alist, etc.
! (defconst lao-key-alist-vector
!   (let ((tail lao-key-alist)
! 	consonant-key-alist semivowel-key-alist vowel-key-alist 
! 	voweltone-key-alist tone-key-alist other-key-alist
! 	elt phonetic-type)
!     (while tail
!       (setq elt (car tail) tail (cdr tail))
!       (if (stringp (cdr elt))
! 	  (setq phonetic-type (get-char-code-property (aref (cdr elt) 0)
! 						      'phonetic-type))
! 	(setq phonetic-type (get-char-code-property (aref (aref (cdr elt) 0) 0)
  						    'phonetic-type))
! 	(aset (cdr elt) 0 (compose-string (aref (cdr elt) 0))))
!       (cond ((eq phonetic-type 'consonant)
! 	     (setq consonant-key-alist (cons elt consonant-key-alist)))
! 	    ((memq phonetic-type '(vowel-upper vowel-lower))
! 	     (if (stringp (cdr elt))
! 		 (setq vowel-key-alist (cons elt vowel-key-alist))
! 	       (setq voweltone-key-alist (cons elt voweltone-key-alist))))
! 	    ((eq  phonetic-type 'tone)
! 	     (setq tone-key-alist (cons elt tone-key-alist)))
! 	    ((eq phonetic-type 'semivowel-lower)
! 	     (setq semivowel-key-alist (cons elt semivowel-key-alist)))
! 	    (t
! 	     (setq other-key-alist (cons elt other-key-alist)))))
!     (vector consonant-key-alist semivowel-key-alist vowel-key-alist 
! 	    voweltone-key-alist tone-key-alist other-key-alist)))
! 
! (defconst lao-consonant-key-alist (aref lao-key-alist-vector 0))
! (defconst lao-semivowel-key-alist (aref lao-key-alist-vector 1))
! (defconst lao-vowel-key-alist (aref lao-key-alist-vector 2))
! (defconst lao-voweltone-key-alist (aref lao-key-alist-vector 3))
! (defconst lao-tone-key-alist (aref lao-key-alist-vector 4))
! (defconst lao-other-key-alist (aref lao-key-alist-vector 5))
! 
! ;; Done with it.
! (makunbound 'lao-key-alist-vector)
  
  (quail-define-package
   "lao" "Lao" "ລ" t

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: defconst in lao.el
  2004-12-04  2:49       ` Kenichi Handa
@ 2004-12-04  3:43         ` Luc Teirlinck
  0 siblings, 0 replies; 16+ messages in thread
From: Luc Teirlinck @ 2004-12-04  3:43 UTC (permalink / raw)
  Cc: monnier, emacs-devel

Ken'ichi HANDA wrote:

   What do you think?

Looks OK to me.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2004-12-04  3:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-14 18:56 defconst in lao.el Luc Teirlinck
2004-11-14 21:06 ` Stefan Monnier
2004-11-14 21:47   ` Luc Teirlinck
2004-11-15 14:00 ` Richard Stallman
2004-11-17  1:41 ` Kenichi Handa
2004-11-17  3:01   ` Luc Teirlinck
2004-11-17  6:39     ` Stephan Stahl
2004-11-17 14:26       ` Luc Teirlinck
2004-11-17 22:05         ` Luc Teirlinck
2004-11-17  3:07   ` Luc Teirlinck
2004-11-17  3:57   ` Luc Teirlinck
2004-11-17 14:17   ` Stefan Monnier
2004-11-17 18:35     ` Luc Teirlinck
2004-11-17 19:03       ` Stefan Monnier
2004-12-04  2:49       ` Kenichi Handa
2004-12-04  3:43         ` Luc Teirlinck

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).