unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* RE: custom type `color' is not enforced
       [not found] <DNEMKBNJBGPAOPIJOOICAELMEBAA.drew.adams@oracle.com>
@ 2007-12-18 23:00 ` Drew Adams
  2007-12-20  0:53   ` Richard Stallman
  0 siblings, 1 reply; 18+ messages in thread
From: Drew Adams @ 2007-12-18 23:00 UTC (permalink / raw)
  To: Emacs-Devel

> From: Drew Adams Sent: Sunday, December 02, 2007 1:00 AM
> > emacs -Q
> > (defcustom foo "White" "xxxxxxx" :type (quote color))
> > M-x customize-variable foo
> >
> > In Customize, change the value to "any-non-color-string". You are able
> > to set the new value and even save it. There is no check done that the
> > value is actually a color - any string will do. The value should be
> > checked to ensure that it matches either a defined color name or "#"
> > followed by a multiple of three hex digits (for RGB).
>
> > Similarly, a variable declared to be of type `regexp' should have
> > its value tested to ensure that it is a valid regexp. Currently,
> > any string is accepted.
> >
> > In GNU Emacs 22.1.1 (i386-mingw-nt5.1.2600) of 2007-06-02 on RELEASE
> > Windowing system distributor `Microsoft Corp.', version 5.1.2600
> > configured using `configure --with-gcc (3.4) --cflags
> > -Ic:/gnuwin32/include'

I sent that to bug-gnu-emacs a little while ago (no response yet). Copying
it to emacs-devel, with some additional motivating info.

The `color' type, in particular, should be easy to test (either the string
is in the list of color names or it satisfies `#' followed by three groups
of hex digits).

Type `regexp' might be more difficult to test, but that too should be
possible, since we issue invalid-regexp errors for strings that aren't valid
regexps.

I hope this bug (missing feature) will be fixed. Types are powerful and
useful. They let you categorize and filter options.

I am also interested in this missing feature because I think the fix would
likely provide some type-checking code that I could use in other places.

I have, for instance, a command `describe-option-of-type' (which I bind to
`C-h M-o'). It is similar to `describe-variable', but it lets you use
defcustom types to narrow the set of completion candidates. I found no
existing code that handles subtyping and type equivalence, so I just made it
handle an exact type match for now, by default. But with `C-u' it also
includes as candidates options whose current values are compatible with the
specified type.

`C-h M-o color RET TAB' correctly gives all options defined with :type
`color' (rigid equality test). However, since Emacs currently does not
distinguish type `color' from type `string' (= this bug), `C-u C-h M-o'
color RET TAB' gives all string-valued variables as candidates. It would be
nice if only legitimate color-valued candidates were available for
completion. Similarly, for type `regexp'.

FWIW, so you can see what I mean and in case Emacs is interested in it, this
is my definition of the command:

(defun describe-option-of-type (type variable)
  "Describe an Emacs user VARIABLE (option) of a given TYPE.
VARIABLE is defined with `defcustom' type TYPE.

With a prefix argument, either VARIABLE is defined with `defcustom'
type TYPE or its current value is compatible with TYPE.

If TYPE is nil (default value) then all `defcustom' variables are
potential candidates.  That is different from using `describe-option',
because `describe-option' includes user-variable candidates not
defined with `defcustom' (with `*'-prefixed doc strings)."
  (interactive
   (let* ((symb (and (symbolp (variable-at-point))
                     (variable-at-point)))
          (typ
           (car
            (condition-case err
                (read-from-string
                 (let ((types ()))
                   (mapatoms
                    (lambda (cand)
                      (when (custom-variable-p cand)
                        (push
                         (list
                          (format
                           "%s"
                           (format "%S" (get cand 'custom-type))))
                         types))))
                   (completing-read "Describe option of type: "
                                    (help-remove-duplicates types)
                                    nil nil nil nil "nil")))
              (end-of-file (error "No such custom type")))))
          (pref-arg current-prefix-arg))
     (list typ
           (intern
            (completing-read
             "Option: " obarray
             (lambda (var)
               (and (custom-variable-p var)
                    (or (not typ)       ; All vars if type = nil.
                        (if pref-arg
                            (help-var-is-of-type-p var (list typ))
                          (equal (get var 'custom-type) typ)))))
             t nil nil (and symb (symbol-name symb)) t)))))
  (describe-variable variable nil t))

(defun help-var-is-of-type-p (variable types)
  "Return non-nil if VARIABLE is of one of the custom types in TYPES.
Non-nil means either VARIABLE's custom type is a member of list TYPES
or VARIABLE is bound and its value satisfies a type in list TYPES."
  (or (memq (get variable 'custom-type) types)
      (and (boundp variable)
           (let ((val (symbol-value variable)))
             (catch 'help-type-matches
               (dolist (type types)
                 (setq type (widget-convert type))
                 (when (widget-apply type :match val)
                   (throw 'help-type-matches t)))
               nil)))))

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

* Re: custom type `color' is not enforced
  2007-12-18 23:00 ` custom type `color' is not enforced Drew Adams
@ 2007-12-20  0:53   ` Richard Stallman
  2007-12-20  1:27     ` Lennart Borgman (gmail)
  2007-12-20 19:11     ` Drew Adams
  0 siblings, 2 replies; 18+ messages in thread
From: Richard Stallman @ 2007-12-20  0:53 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

    The `color' type, in particular, should be easy to test (either the string
    is in the list of color names or it satisfies `#' followed by three groups
    of hex digits).

    Type `regexp' might be more difficult to test, but that too should be
    possible, since we issue invalid-regexp errors for strings that aren't valid
    regexps.

I agree that these changes would be improvements.  However, I don't
think we have any way to test the validity of a regexp without
matching it, and matching it (even against a null string) could
cause an infinite loop.

    I have, for instance, a command `describe-option-of-type' (which I bind to
    `C-h M-o'). It is similar to `describe-variable', but it lets you use
    defcustom types to narrow the set of completion candidates.

I have to say it doesn't sound tremendously useful.

    `C-h M-o color RET TAB' correctly gives all options defined with :type
    `color' (rigid equality test). However, since Emacs currently does not
    distinguish type `color' from type `string' (= this bug),

That statement surprises me.  The two types have different names, and
don't behave the same, so in what sense does Emacs not distinguish
them?

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

* Re: custom type `color' is not enforced
  2007-12-20  0:53   ` Richard Stallman
@ 2007-12-20  1:27     ` Lennart Borgman (gmail)
  2007-12-21  3:04       ` Richard Stallman
  2007-12-20 19:11     ` Drew Adams
  1 sibling, 1 reply; 18+ messages in thread
From: Lennart Borgman (gmail) @ 2007-12-20  1:27 UTC (permalink / raw)
  To: rms; +Cc: Drew Adams, emacs-devel

Richard Stallman wrote:
> I agree that these changes would be improvements.  However, I don't
> think we have any way to test the validity of a regexp without
> matching it, and matching it (even against a null string) could
> cause an infinite loop.

The widget library already validates

   :type 'regexp

with the function (defun widget-regexp-validate (widget) ... ) which 
does (string-match regexp "").

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

* RE: custom type `color' is not enforced
  2007-12-20  0:53   ` Richard Stallman
  2007-12-20  1:27     ` Lennart Borgman (gmail)
@ 2007-12-20 19:11     ` Drew Adams
  2007-12-20 19:27       ` Drew Adams
                         ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Drew Adams @ 2007-12-20 19:11 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

>     The `color' type, in particular, should be easy to test
>     (either the string is in the list of color names or it
>     satisfies `#' followed by three groups of hex digits).
>
>     Type `regexp' might be more difficult to test, but that too should be
>     possible, since we issue invalid-regexp errors for strings
>     that aren't valid regexps.
>
> I agree that these changes would be improvements.  However, I don't
> think we have any way to test the validity of a regexp without
> matching it, and matching it (even against a null string) could
> cause an infinite loop.

That could be, but `color' should be doable, at least. There might also be
other such type checks that could be done but are not.

>     I have, for instance, a command `describe-option-of-type'
>     (which I bind to `C-h M-o'). It is similar to `describe-variable',
>     but it lets you use defcustom types to narrow the set of
>     completion candidates.
>
> I have to say it doesn't sound tremendously useful.

I have to say "lack of imagination". ;-)

Don't assume that the user knows just which option s?he wants to describe.
`describe-variable' gives you only one way, by option name, to define a set
of candidate options, in order to pick one to describe. You can type a
prefix and hit TAB to see what the matching options are, but when there are
many with the same prefix that doesn't help much. You need to scan the list
looking for what might be the appropriate option, and some of the names in
the list might not be very suggestive. That can mean a lot of parsing and
thinking energy.

Obviously, with substring (not just prefix) matching, finding the right
option to describe is easier - you can match, say, `width' anywhere in the
option name, not just as a prefix. And if you can also match multiple
substrings, without regard to order, so much the better.

But you are still relying on only the option name, which (1) is not
necessarily efficient (there can still be many such matches) and (2) might
not work at all: you might not be thinking in terms of the same words that
are used in the option name.

Adding the ability to specify the option type increases your matching power
and thus your expressiveness and efficiency as a user.

For example, you might know that you want to examine or modify some
integer-valued option that controls how many chars are used for something.
You can try strings such as `char', `min', and `width' in the option name,
and you can also make it match the prefix `icicle-' to limit the search to
options in one package, but you will still be fishing in the dark to some
extent. Being able to also say that you know that the option is an integer
can narrow things down considerably.

Or suppose the option you are after is `Info-prefer-subnodes', and you can't
remember the name. You might be thinking in terms of `space', `menu',
`submenu', `depth-first', `navigate', and `traversal', but not in terms of
`subnode'. Matching against the `Info-' options gives you twice as many
candidates as matching against the `Info-' options that are boolean.

Same thing for other options where there are many with the same prefix and
the option name might not use terms you are thinking of - boolean options
`imenu-eager-completion-buffer' and `inverse-video', for example.

Similarly, if you want to customize an option, you can use
`customize-apropos-options', but you can often narrow things down better if
you use `customize-apropos-options-of-type' (another command I can offer).

>     `C-h M-o color RET TAB' correctly gives all options defined with :type
>     `color' (rigid equality test). However, since Emacs currently does not
>     distinguish type `color' from type `string' (= this bug),
>
> That statement surprises me.  The two types have different names, and
> don't behave the same, so in what sense does Emacs not distinguish
> them?

Please reread what I said: it _correctly_ gives all options defined with
`color', using a rigid equality test for the type. Where it breaks down is
"this bug": the fact that any string as the option value satisfies type
`color'.

The part you didn't quote explains this: Using `C-u' with `C-h M-o' tests
whether the current value of an option satisfies (is compatible with) type
`color' (in this case). And any string at all is deemed by Emacs to be of
type `color'. That's the missing feature or bug. If you don't use `C-u',
then you get a rigid equality test for the type, and that works OK, as far
as it goes.

`help-var-is-of-type-p' is the part of the code that `C-u' uses to test
whether the value is of the right type:

 (widget-apply (widget-convert type):match value)

And any string VALUE at all satisfies this test when TYPE is `color',
`string', or `regexp'.

The ability to look for options whose value is of the right type (i.e., via
`C-u' here) is all the more important in that there is not yet any treatment
of type comparison (beyond a rigid :type comparison with `equal') or of
subtyping (a `regexp' is a `string'; an `integer' is a `number').

With substring matching and using `C-u' with `C-h M-o', I can see candidate
options whose current value satisfies a `type' definition that includes
"string" - that means type definitions such as these:

(choice (string :tag "Show candidate plus a WYSIWYG swatch with text..."
                :value "MMMM")
        (const :tag "Show candidate itself using WYSIWYG" t)
        (const :tag "Show candidate as is, with no text properties" nil))

(repeat (list string (choice (const :tag "None" nil) (string :tag "Match
regexp")) (choice (const :tag "None" nil) (string :tag "No-match regexp"))
(choice (const :tag "None" nil) (function :tag "Predicate")) (choice (const
:tag "None" nil) (repeat (string :tag "Extra buffer"))) (choice (const :tag
"None" nil) (function :tag "Sort function"))))

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

* RE: custom type `color' is not enforced
  2007-12-20 19:11     ` Drew Adams
@ 2007-12-20 19:27       ` Drew Adams
  2007-12-21  3:59       ` Richard Stallman
  2007-12-21  3:59       ` Richard Stallman
  2 siblings, 0 replies; 18+ messages in thread
From: Drew Adams @ 2007-12-20 19:27 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

I wrote:

> With substring matching and using `C-u' with `C-h M-o', I can see
> candidate options whose current value satisfies a `type'
> definition that includes "string" - that means type definitions
> such as these:
>
> (choice (string :tag "Show candidate plus a WYSIWYG swatch with text..."
>                 :value "MMMM")
>         (const :tag "Show candidate itself using WYSIWYG" t)
>         (const :tag "Show candidate as is, with no text properties" nil))
>
> (repeat (list string (choice (const :tag "None" nil) (string :tag
> "Match regexp")) (choice (const :tag "None" nil) (string :tag
> "No-match regexp")) (choice (const :tag "None" nil) (function
> :tag "Predicate")) (choice (const :tag "None" nil) (repeat
> (string :tag "Extra buffer"))) (choice (const :tag "None" nil)
> (function :tag "Sort function"))))

Sorry; I mistakenly sent that before I was ready. My new Dell Latitude 620
has a glitch that it randomly flips into a mode where either Shift or
Control is automatically on, so, for instance, hitting the `s' key really
does `S' or `C-s'. Aggravating, and downright risky sometimes.

Anyway, I meant to format that last part:

(repeat
 (list string
       (choice (const :tag "None" nil)
               (string :tag "Match regexp"))
       (choice (const :tag "None" nil)
               (string :tag "No-match regexp"))
       (choice (const :tag "None" nil)
               (function :tag "Predicate"))
       (choice (const :tag "None" nil)
               (repeat (string :tag "Extra buffer")))
       (choice (const :tag "None" nil)
               (function :tag "Sort function"))))

And I meant to point out that a type def that simply includes "string"
somewhere is not necessarily a type def that uses type `string' as one of
its components ("string" might be in a :tag string, for instance). IOW, this
isn't a substitute for being able to check subtypes and aggregation type
components.

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

* Re: custom type `color' is not enforced
  2007-12-20  1:27     ` Lennart Borgman (gmail)
@ 2007-12-21  3:04       ` Richard Stallman
  0 siblings, 0 replies; 18+ messages in thread
From: Richard Stallman @ 2007-12-21  3:04 UTC (permalink / raw)
  To: Lennart Borgman (gmail); +Cc: drew.adams, emacs-devel

    with the function (defun widget-regexp-validate (widget) ... ) which 
    does (string-match regexp "").

Maybe that does work.  I just tried a regexp with nested loops,
"\\(\\(\\W?\\)*\\)*", and it returned immediately, which sure
surprised me.  Not only that, it returned immediately in re-search-forward
with a nonempty buffer, which surprised me even more.

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

* Re: custom type `color' is not enforced
  2007-12-20 19:11     ` Drew Adams
  2007-12-20 19:27       ` Drew Adams
@ 2007-12-21  3:59       ` Richard Stallman
  2007-12-21  6:47         ` Drew Adams
  2007-12-21  3:59       ` Richard Stallman
  2 siblings, 1 reply; 18+ messages in thread
From: Richard Stallman @ 2007-12-21  3:59 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

    Don't assume that the user knows just which option s?he wants to describe.
    `describe-variable' gives you only one way, by option name, to define a set
    of candidate options, in order to pick one to describe.

I'm not assuming that.  Rather, I don't think very many people
would take the trouble to remember that `describe-option-of-type'
exists.  Thus, practically speaking it would not be very useful.

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

* Re: custom type `color' is not enforced
  2007-12-20 19:11     ` Drew Adams
  2007-12-20 19:27       ` Drew Adams
  2007-12-21  3:59       ` Richard Stallman
@ 2007-12-21  3:59       ` Richard Stallman
  2007-12-21  6:48         ` Drew Adams
  2007-12-21  9:36         ` Lennart Borgman (gmail)
  2 siblings, 2 replies; 18+ messages in thread
From: Richard Stallman @ 2007-12-21  3:59 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

    >     `C-h M-o color RET TAB' correctly gives all options defined with :type
    >     `color' (rigid equality test). However, since Emacs currently does not
    >     distinguish type `color' from type `string' (= this bug),
    >
    > That statement surprises me.  The two types have different names, and
    > don't behave the same, so in what sense does Emacs not distinguish
    > them?

    Please reread what I said:

You said Emacs does not distinguish type `color' from type `string'.
That's mistaken; it does distinguish them.  They behave differently.

I agree it would be good to make `color' check for a valid color.
I hope someone will do that.

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

* RE: custom type `color' is not enforced
  2007-12-21  3:59       ` Richard Stallman
@ 2007-12-21  6:47         ` Drew Adams
  0 siblings, 0 replies; 18+ messages in thread
From: Drew Adams @ 2007-12-21  6:47 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

>     Don't assume that the user knows just which option s?he wants
>     to describe. `describe-variable' gives you only one way, by
>     option name, to define a set of candidate options, in order
>     to pick one to describe.
>
> I'm not assuming that.  Rather, I don't think very many people
> would take the trouble to remember that `describe-option-of-type'
> exists.  Thus, practically speaking it would not be very useful.

It needs a binding, yes. Not very many people would have learned or would
remember several of the help commands if they were not bound.

FWIW, in my code, I use these bindings in `help-map':

`o'   for `describe-option'  (= `describe-variable' with prefix arg)
`C-o' for `describe-option-of-type'
`c'   for `describe-command' (= `describe-function' with prefix arg)

(`describe-key-briefly' is `C-c'.)

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

* RE: custom type `color' is not enforced
  2007-12-21  3:59       ` Richard Stallman
@ 2007-12-21  6:48         ` Drew Adams
  2007-12-21  9:36         ` Lennart Borgman (gmail)
  1 sibling, 0 replies; 18+ messages in thread
From: Drew Adams @ 2007-12-21  6:48 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

>     >     `C-h M-o color RET TAB' correctly gives all options
>     >     defined with :type `color' (rigid equality test).
>     >     However, since Emacs currently does not
>     >     distinguish type `color' from type `string' (= this bug),
>     >
>     > That statement surprises me.  The two types have different
>     > names, and don't behave the same, so in what sense does
>     > Emacs not distinguish them?
>
>     Please reread what I said:
>
> You said Emacs does not distinguish type `color' from type `string'.
> That's mistaken; it does distinguish them.

You continue to omit the part where I explained what I meant by that. If you
want to argue about what I said, and you think that omitting the explanation
helps you win the argument, go ahead; you win.

That's like me quoting your current statement only partially: You said
"Emacs does not distinguish type". Yes, you said those words, but that's not
really what you were saying. Taking words out of context is not
constructive.

My point (from the beginning; = what I described as "this bug") was that
there is a sense in which those types are not distinguished. I explained
twice what that sense was, what I meant by the insufficient type
distinction. If you don't want to see my point, fine.

> They behave differently.

Yes, if you compare only the symbols `color' and `string', they do behave
differently. If `get-custom-type' is the only yardstick, then they are
totally different, unrelated types.

> I agree it would be good to make `color' check for a valid color.
> I hope someone will do that.

So do I.

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

* Re: custom type `color' is not enforced
  2007-12-21  3:59       ` Richard Stallman
  2007-12-21  6:48         ` Drew Adams
@ 2007-12-21  9:36         ` Lennart Borgman (gmail)
  2007-12-21 18:20           ` Drew Adams
  2007-12-22  6:29           ` Richard Stallman
  1 sibling, 2 replies; 18+ messages in thread
From: Lennart Borgman (gmail) @ 2007-12-21  9:36 UTC (permalink / raw)
  To: rms; +Cc: Drew Adams, emacs-devel

Richard Stallman wrote:
>     >     `C-h M-o color RET TAB' correctly gives all options defined with :type
>     >     `color' (rigid equality test). However, since Emacs currently does not
>     >     distinguish type `color' from type `string' (= this bug),
>     >
>     > That statement surprises me.  The two types have different names, and
>     > don't behave the same, so in what sense does Emacs not distinguish
>     > them?
> 
>     Please reread what I said:
> 
> You said Emacs does not distinguish type `color' from type `string'.
> That's mistaken; it does distinguish them.  They behave differently.
> 
> I agree it would be good to make `color' check for a valid color.
> I hope someone will do that.


Something like this?


(defun color-digits-p (color)
   (save-match-data
     (string-match (rx bos
                       "#"
                       (or (repeat 3 3 hex-digit)
                           (repeat 6 6 hex-digit))
                       eos)
                   color
                   )))

(defun widget-color-validate (widget)
   (let ((val (widget-value widget)))
     (unless (or
              ;;(color-defined-p val)
              (member val x-colors)
              (and (stringp val)
                   (color-digits-p val)))
       (widget-put widget :error (format "Invalid color: %S"
                                         (widget-value widget)))
       widget)))

(define-widget 'color 'editable-field
   "Choose a color name (with sample)."
   :format "%{%t%}: %v (%{sample%})\n"
   :size 10
   :tag "Color"
   :validate 'widget-color-validate
   :value "black"
   :complete 'widget-color-complete
   :sample-face-get 'widget-color-sample-face-get
   :notify 'widget-color-notify
   :action 'widget-color-action)

(defcustom test-color "black"
   "color test"
   :type 'color)

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

* RE: custom type `color' is not enforced
  2007-12-21  9:36         ` Lennart Borgman (gmail)
@ 2007-12-21 18:20           ` Drew Adams
  2007-12-21 22:03             ` Drew Adams
  2007-12-22  6:29           ` Richard Stallman
  1 sibling, 1 reply; 18+ messages in thread
From: Drew Adams @ 2007-12-21 18:20 UTC (permalink / raw)
  To: Lennart Borgman (gmail), rms; +Cc: emacs-devel

> > I agree it would be good to make `color' check for a valid color.
> > I hope someone will do that.
>
> Something like this?
> (defun color-digits-p (color)
>    (save-match-data
>      (string-match (rx bos "#"
>                        (or (repeat 3 3 hex-digit)
>                            (repeat 6 6 hex-digit))
>                        eos)
>                    color)))
> (defun widget-color-validate (widget)
>    (let ((val (widget-value widget)))
>      (unless (or (member val x-colors)
>                  (and (stringp val)
>                       (color-digits-p val)))
>        (widget-put widget :error (format "Invalid color: %S"
>                                          (widget-value widget)))
>        widget)))
> (define-widget 'color 'editable-field
>    "Choose a color name (with sample)."
>    :format "%{%t%}: %v (%{sample%})\n"
>    :size 10
>    :tag "Color"
>    :validate 'widget-color-validate
>    :value "black"
>    :complete 'widget-color-complete
>    :sample-face-get 'widget-color-sample-face-get
>    :notify 'widget-color-notify
>    :action 'widget-color-action)
>
> (defcustom test-color "black" "color test" :type 'color)

Thanks, Lennart!

However:

- (defined-colors) should perhaps be used in place of `x-colors'.

- `color-digits-p' doesn't work for some legitimate colors, such as
"#FFFF0000FFFF". Any number of hex digits is legitimate, as long as it is a
multiple of 3 (RGB). FWIW, I use this regexp to check for an RGB string:

 "^#\\([a-fA-F0-9][a-fA-F0-9][a-fA-F0-9]\\)+$"

- Perhaps :size should be a bit larger? Some color names are longer than 10
chars. But I see that you took that and the rest of the `color' widget
definition from wid-edit.el: 10 is used in the Emacs definition of the
`color' widget.


Trying your fix made me realize that my type test was insufficient. I don't
understand widgets very well.

Instead of testing (widget-apply type :match val), it looks like I need to
test (and var-widget (not (widget-apply var-widget :validate))), where
var-widget is (get var 'custom-variable).

Here is what I use now; it seems to work (but suggestions are welcome). When
combined with your fix, it correctly distinguishes options whose values are
colors from those with arbitrary string values.

(defun help-var-is-of-type-p (variable types)
  "Return non-nil if VARIABLE is of one of the custom types in TYPES.
Non-nil means either VARIABLE's custom type is a member of list TYPES
or VARIABLE is bound and its value satisfies a type in list TYPES."
  (or (memq (get variable 'custom-type) types)
      (and (boundp variable)
           (let ((val (symbol-value variable))
                 (var-widget (get variable 'custom-variable)))
             (catch 'help-type-matches
               (dolist (type types)
                 (setq type (widget-convert type))
                 (when (and var-widget
                       (not (widget-apply var-widget :validate)))
                   (throw 'help-type-matches t)))
               nil)))))

So `C-u C-h C-o' (my `describe-option-of-type') now shows, as completion
candidates, all options whose values are legitimate colors (and no others).

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

* RE: custom type `color' is not enforced
  2007-12-21 18:20           ` Drew Adams
@ 2007-12-21 22:03             ` Drew Adams
  0 siblings, 0 replies; 18+ messages in thread
From: Drew Adams @ 2007-12-21 22:03 UTC (permalink / raw)
  To: Lennart Borgman (gmail), rms; +Cc: emacs-devel

I wrote:

> Trying your fix made me realize that my type test was
> insufficient. I don't understand widgets very well.

That last part is certainly true!

> Instead of testing (widget-apply type :match val), it looks like I need to
> test (and var-widget (not (widget-apply var-widget :validate))), where
> var-widget is (get var 'custom-variable).

Not quite. The code I sent didn't work. This seems to work, however:

(defun help-var-is-of-type-p (variable types)
  "Return non-nil if VARIABLE is of one of the custom types in TYPES.
Non-nil means either VARIABLE's custom type is a member of list TYPES
or VARIABLE is bound and its value satisfies a type in list TYPES."
  (or (memq (get variable 'custom-type) types)
      (and (boundp variable)
           (let ((val (symbol-value variable))
                 (var-widget (widget-convert (get variable 'custom-type))))
             (catch 'help-type-matches
               (dolist (type types)
                 (setq type (widget-convert type))
                 (plist-put (cdr type) :value val)
                 (when (and var-widget
                            (not (widget-apply type :validate)))
                   (throw 'help-type-matches t)))
               nil)))))

[I also had to modify Lennart's `color-digits-p' to use a regexp such as
what I sent earlier: "^#\\([a-fA-F0-9][a-fA-F0-9][a-fA-F0-9]\\)+$".]

There may be a simpler way to do it, but this seems to work OK.

> So `C-u C-h C-o' (my `describe-option-of-type') now shows, as completion
> candidates, all options whose values are legitimate colors (and
> no others).

That was not true when I wrote it, but it seems to be true now, with the
above code.

With C-u, `describe-option-of-type' `color' includes non-`color' options
whose values are legitimate for type `color'. That means that
`help-var-is-of-type-p' must return non-nil when passed an option whose
defcustom type is `string' but whose current value is "white" (and when
passed '(color) as the TYPES arg). And the same thing, generalized for all
types. And `help-var-is-of-type-p' also returns non-nil when testing an
option of type `color' or `regexp' for type `string'.

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

* Re: custom type `color' is not enforced
  2007-12-21  9:36         ` Lennart Borgman (gmail)
  2007-12-21 18:20           ` Drew Adams
@ 2007-12-22  6:29           ` Richard Stallman
  2007-12-22 20:58             ` Lennart Borgman (gmail)
  1 sibling, 1 reply; 18+ messages in thread
From: Richard Stallman @ 2007-12-22  6:29 UTC (permalink / raw)
  To: Lennart Borgman (gmail); +Cc: drew.adams, emacs-devel

You have the right idea.

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

* Re: custom type `color' is not enforced
  2007-12-22  6:29           ` Richard Stallman
@ 2007-12-22 20:58             ` Lennart Borgman (gmail)
  2007-12-23 10:54               ` Per Abrahamsen
  0 siblings, 1 reply; 18+ messages in thread
From: Lennart Borgman (gmail) @ 2007-12-22 20:58 UTC (permalink / raw)
  To: rms; +Cc: Per Abrahamsen, drew.adams, emacs-devel

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

Richard Stallman wrote:
> You have the right idea.

I have tried to finish the code too now ;-)

Please see the attached files. Beside a more complete widget type for 
colors I have also included some basic functions for testing custom 
types. I believe these could be useful.

I have tried to make them as clean and simple as they can be at the 
moment. However I am unsure about how to call the :match and :validate 
functions. I might very well be missing something concerning the 
conversion from and to external values.

BTW when I have been looking at this I have had a hard time to 
understand why there are both :match and :validate functions.

I also do not understand the paramters they take. Why do the :match 
function have a widget parameter? Does it have something to do with 
external - internal conversion, or?


[-- Attachment #2: color-test.el --]
[-- Type: text/plain, Size: 4004 bytes --]


(defun color-digits-p (color)
  (save-match-data
    (string-match (rx bos
                      "#"
                      (1+ (repeat 3 3 hex-digit))
                      eos)
                  color)))

(defun widget-color-match (widget value)
  (or
   ;; I am not sure what colors to test. It might be relevant to check
   ;; all as I suggest here.
   ;;(color-defined-p val)
   (member value x-colors)
   (and (stringp value)
        (color-digits-p value))))

(defun widget-color-validate (widget)
  (let ((value (widget-value widget)))
    (unless (widget-color-match widget value)
      (widget-put widget :error (format "Invalid color: %S" value))
      widget)))

(define-widget 'color 'editable-field
  "Choose a color (with sample)."
  :format "%{%t%}: %v (%{sample%})\n"
  :size 25  ;; (length "light coldenrod yellow") = 22
  :tag "Color"
  :match 'widget-color-match
  :validate 'widget-color-validate
  :value "black"
  :complete 'widget-color-complete
  :sample-face-get 'widget-color-sample-face-get
  :notify 'widget-color-notify
  :action 'widget-color-action)

(defun custom-type-symbol-p (symbol custom-type)
  "Return t if value of symbol SYMBOL should fit CUSTOM-TYPE."
  (let ((found nil)
        (type (get symbol 'custom-type)))
    (while (and (not found) type)
      (setq found (eq type custom-type))
      (setq type (car (get type 'widget-type))))
    found))

(defun custom-type-value-p (value custom-type)
  "Return non-nil if value of VALUE fits CUSTOM-TYPE."
  (let ((widget (if (listp custom-type)
                    custom-type
                  (list custom-type))))
    (setq widget (widget-convert widget))
    ;; There are (unfortunately) two different ways to test the
    ;; values in a widget. Some widget types use both, some just one
    ;; of them. We check for both, but only use one of them here.
    (let ((match-fun (widget-get widget :match))
          (validate-fun (widget-get widget :validate)))
      ;;(setq match-fun nil)
      ;;(setq validate-fun nil)
      (widget-put widget :value value)
      ;; Fix-me: I am not sure whether widget-apply of funcall
      ;; should be used here, but I believe anyone of them can be
      ;; used. But please look into this. It might have something to
      ;; do with internal/external values for the widgets.
      (cond
       ;; Test the :match alternative first because this because this
       ;; seems most basic.
       (match-fun
        (when
            ;;(widget-apply widget :match value)
            (funcall match-fun widget value)
          t))
       (validate-fun
        (let (;;(val (widget-apply widget :validate))
              (val (funcall validate-fun widget)))
          ;; Check if :error was applied
          (when (not (widget-get val :error)) t)))
       (t
        (error
         "There is no way to check value against custom type %s"
         custom-type))))))

(defun custom-type-p (val-or-sym custom-type)
  "Return non-nil if VAL-OR-SYM fits CUSTOM-TYPE.
VAL-OR-SYM may be either a variable or a symbol. If it is a
variable then return non-nil if the value fits custom type
CUSTOM-TYPE.

If it is a symbol then return non-nil if the values this symbol's
variable can have fits CUSTOM-TYPE."
  (if (symbolp val-or-sym)
      (custom-type-symbol-p val-or-sym custom-type)
    (custom-type-value-p val-or-sym custom-type)))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Tests

;; (custom-type-p 'test-color 'color)
;; (custom-type-p 'test-color 'edit)
;; (custom-type-p 'test-color 'editable-field)
;; (custom-type-p test-color 'color)
;; (get 'test-color 'custom-type)
;; (setq test-color "bla")
;; (setq test-color "black")

(defcustom test-color "black"
  "color test"
  :type 'color)

(defun max-color-length()
  (let ((len 0)
        (longest ""))
    (mapc (lambda (color)
            (when (< len (length color))
              (setq len (length color))
              (setq longest color)))
          x-colors)
    (cons len longest)))
;; (max-color-length)

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

* Re: custom type `color' is not enforced
  2007-12-22 20:58             ` Lennart Borgman (gmail)
@ 2007-12-23 10:54               ` Per Abrahamsen
  2007-12-23 12:46                 ` Lennart Borgman (gmail)
  0 siblings, 1 reply; 18+ messages in thread
From: Per Abrahamsen @ 2007-12-23 10:54 UTC (permalink / raw)
  To: Lennart Borgman (gmail); +Cc: rms, drew.adams, emacs-devel

From my unreliable memory:

The :validate function exist to check that the value the user has
entered in the widget actually match the "type" of the widget.  If the
user enter "kurt" in the editable text for an integer widget, validate
will signal an error.

The :match function exist to choose when "branch" to take for a choice
widget, and a few similar situations:  E.g. if you instantiate a
(choice integer string) type with the initial value of "kurt", the
second branch of the choice should be activated.  The logic of the
:match widget can be quite complex with composite wdgets, in fact the
whole mechanism is regular expressions over the "alphabet" of sexps.
Or should have been, had it been done right, the actual implementation
is somewhat weaker.

The :match function takes the widget as an argument, because sometimes
whether or not it matches depends on the widget properties.  For
example, the choice widget above will match strings and integers, but
another choice widget with other arguments will match other values.

-- Per

On 12/22/07, Lennart Borgman (gmail) <lennart.borgman@gmail.com> wrote:
> Richard Stallman wrote:
> > You have the right idea.
>
> I have tried to finish the code too now ;-)
>
> Please see the attached files. Beside a more complete widget type for
> colors I have also included some basic functions for testing custom
> types. I believe these could be useful.
>
> I have tried to make them as clean and simple as they can be at the
> moment. However I am unsure about how to call the :match and :validate
> functions. I might very well be missing something concerning the
> conversion from and to external values.
>
> BTW when I have been looking at this I have had a hard time to
> understand why there are both :match and :validate functions.
>
> I also do not understand the paramters they take. Why do the :match
> function have a widget parameter? Does it have something to do with
> external - internal conversion, or?
>
>
>
> (defun color-digits-p (color)
>  (save-match-data
>    (string-match (rx bos
>                      "#"
>                      (1+ (repeat 3 3 hex-digit))
>                      eos)
>                  color)))
>
> (defun widget-color-match (widget value)
>  (or
>   ;; I am not sure what colors to test. It might be relevant to check
>   ;; all as I suggest here.
>   ;;(color-defined-p val)
>   (member value x-colors)
>   (and (stringp value)
>        (color-digits-p value))))
>
> (defun widget-color-validate (widget)
>  (let ((value (widget-value widget)))
>    (unless (widget-color-match widget value)
>      (widget-put widget :error (format "Invalid color: %S" value))
>      widget)))
>
> (define-widget 'color 'editable-field
>  "Choose a color (with sample)."
>  :format "%{%t%}: %v (%{sample%})\n"
>  :size 25  ;; (length "light coldenrod yellow") = 22
>  :tag "Color"
>  :match 'widget-color-match
>  :validate 'widget-color-validate
>  :value "black"
>  :complete 'widget-color-complete
>  :sample-face-get 'widget-color-sample-face-get
>  :notify 'widget-color-notify
>  :action 'widget-color-action)
>
> (defun custom-type-symbol-p (symbol custom-type)
>  "Return t if value of symbol SYMBOL should fit CUSTOM-TYPE."
>  (let ((found nil)
>        (type (get symbol 'custom-type)))
>    (while (and (not found) type)
>      (setq found (eq type custom-type))
>      (setq type (car (get type 'widget-type))))
>    found))
>
> (defun custom-type-value-p (value custom-type)
>  "Return non-nil if value of VALUE fits CUSTOM-TYPE."
>  (let ((widget (if (listp custom-type)
>                    custom-type
>                  (list custom-type))))
>    (setq widget (widget-convert widget))
>    ;; There are (unfortunately) two different ways to test the
>    ;; values in a widget. Some widget types use both, some just one
>    ;; of them. We check for both, but only use one of them here.
>    (let ((match-fun (widget-get widget :match))
>          (validate-fun (widget-get widget :validate)))
>      ;;(setq match-fun nil)
>      ;;(setq validate-fun nil)
>      (widget-put widget :value value)
>      ;; Fix-me: I am not sure whether widget-apply of funcall
>      ;; should be used here, but I believe anyone of them can be
>      ;; used. But please look into this. It might have something to
>      ;; do with internal/external values for the widgets.
>      (cond
>       ;; Test the :match alternative first because this because this
>       ;; seems most basic.
>       (match-fun
>        (when
>            ;;(widget-apply widget :match value)
>            (funcall match-fun widget value)
>          t))
>       (validate-fun
>        (let (;;(val (widget-apply widget :validate))
>              (val (funcall validate-fun widget)))
>          ;; Check if :error was applied
>          (when (not (widget-get val :error)) t)))
>       (t
>        (error
>         "There is no way to check value against custom type %s"
>         custom-type))))))
>
> (defun custom-type-p (val-or-sym custom-type)
>  "Return non-nil if VAL-OR-SYM fits CUSTOM-TYPE.
> VAL-OR-SYM may be either a variable or a symbol. If it is a
> variable then return non-nil if the value fits custom type
> CUSTOM-TYPE.
>
> If it is a symbol then return non-nil if the values this symbol's
> variable can have fits CUSTOM-TYPE."
>  (if (symbolp val-or-sym)
>      (custom-type-symbol-p val-or-sym custom-type)
>    (custom-type-value-p val-or-sym custom-type)))
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> ;;; Tests
>
> ;; (custom-type-p 'test-color 'color)
> ;; (custom-type-p 'test-color 'edit)
> ;; (custom-type-p 'test-color 'editable-field)
> ;; (custom-type-p test-color 'color)
> ;; (get 'test-color 'custom-type)
> ;; (setq test-color "bla")
> ;; (setq test-color "black")
>
> (defcustom test-color "black"
>  "color test"
>  :type 'color)
>
> (defun max-color-length()
>  (let ((len 0)
>        (longest ""))
>    (mapc (lambda (color)
>            (when (< len (length color))
>              (setq len (length color))
>              (setq longest color)))
>          x-colors)
>    (cons len longest)))
> ;; (max-color-length)
>
>

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

* Re: custom type `color' is not enforced
  2007-12-23 10:54               ` Per Abrahamsen
@ 2007-12-23 12:46                 ` Lennart Borgman (gmail)
  2007-12-25 13:52                   ` Per Abrahamsen
  0 siblings, 1 reply; 18+ messages in thread
From: Lennart Borgman (gmail) @ 2007-12-23 12:46 UTC (permalink / raw)
  To: Per Abrahamsen; +Cc: rms, drew.adams, emacs-devel

Per Abrahamsen wrote:
>>From my unreliable memory:
> 
> The :validate function exist to check that the value the user has
> entered in the widget actually match the "type" of the widget.  If the
..
> The :match function exist to choose when "branch" to take for a choice
> widget, and a few similar situations:  E.g. if you instantiate a
...

Thanks Per, maybe I understand a little bit now. Are you saying that 
simple types should not have a match function?

> On 12/22/07, Lennart Borgman (gmail) <lennart.borgman@gmail.com> wrote:
>> Richard Stallman wrote:
>>> You have the right idea.
>> I have tried to finish the code too now ;-)

And I failed again ;-)

>> (defun custom-type-symbol-p (symbol custom-type)

Drew kindly told me this only works for simple types. I have to rethink 
this.

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

* Re: custom type `color' is not enforced
  2007-12-23 12:46                 ` Lennart Borgman (gmail)
@ 2007-12-25 13:52                   ` Per Abrahamsen
  0 siblings, 0 replies; 18+ messages in thread
From: Per Abrahamsen @ 2007-12-25 13:52 UTC (permalink / raw)
  To: Lennart Borgman (gmail); +Cc: emacs-devel

On 12/23/07, Lennart Borgman (gmail) <lennart.borgman@gmail.com> wrote:
> Thanks Per, maybe I understand a little bit now. Are you saying that
> simple types should not have a match function?

The :match function for the choice type tries the :match function for
each option in turn, so atomic types definitely should have a :match
function.  It is what the composite types rely on.

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

end of thread, other threads:[~2007-12-25 13:52 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <DNEMKBNJBGPAOPIJOOICAELMEBAA.drew.adams@oracle.com>
2007-12-18 23:00 ` custom type `color' is not enforced Drew Adams
2007-12-20  0:53   ` Richard Stallman
2007-12-20  1:27     ` Lennart Borgman (gmail)
2007-12-21  3:04       ` Richard Stallman
2007-12-20 19:11     ` Drew Adams
2007-12-20 19:27       ` Drew Adams
2007-12-21  3:59       ` Richard Stallman
2007-12-21  6:47         ` Drew Adams
2007-12-21  3:59       ` Richard Stallman
2007-12-21  6:48         ` Drew Adams
2007-12-21  9:36         ` Lennart Borgman (gmail)
2007-12-21 18:20           ` Drew Adams
2007-12-21 22:03             ` Drew Adams
2007-12-22  6:29           ` Richard Stallman
2007-12-22 20:58             ` Lennart Borgman (gmail)
2007-12-23 10:54               ` Per Abrahamsen
2007-12-23 12:46                 ` Lennart Borgman (gmail)
2007-12-25 13:52                   ` Per Abrahamsen

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).