bug#8717: 23.3; widget match functions should be passed internal representation?

[Mauro Aranda <maurooaranda@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2180 bytes --]

Dave Abrahams <dave@boostpro.com> writes:

> Please consider this widget definition:
>
>   (defun el-get-repeat-value-to-internal (widget list-or-element)
>     (if (listp list-or-element) list-or-element (list list-or-element)))
>
>   (defun el-get-repeat-match (widget value)
>     (widget-editable-list-match widget (el-get-repeat-value-to-internal
widget value)))
>
>   (define-widget 'el-get-repeat 'repeat
>     "A variable length list of non-lists that can also be represented as
a single element"
>     :value-to-internal 'el-get-repeat-value-to-internal
>     :match 'el-get-repeat-match)
>
> I found it surprising that the :match field was required (if you don't
> include it, a value of "foo" does not match an (el-get-repeat string)
> widget.

It took me more than one read to understand this bug report, because the
title asks one thing, but the code presented doesn't do that, and maybe
that goes to show how confusing this stuff can be.

The :match function should be passed a value in the external format.
That is, the value as seen by the rest of Emacs.  A value matches the
repeat widget if the value is a list, and all the members match the type
specified by the repeat widget.  So, "foo" can't match a repeat widget
or a widget that derives from the repeat widget, unless the derived
widget overrides the :match function.

el-get-repeat-value-to-internal is not converting a value to an
"internal format", it is just adapting the value to an "external format"
that the repeat widget can represent.  Of course, if for the widget that
way of representing a value is useful, it can represent it that way,
but that doesn't mean that the :match function takes the value in the
internal format.

> I'm not sure what the proper remedy is; the documentation does
> not distinguish between where the system will operate on the "internal"
> and "external" values.  So, at least clarification in the docs is needed
> regardless, IMO.

The manual doesn't even say what an "external value" and an
"internal value" is.  And surely it needs more work in this and other
concepts, but would be it OK to install something along the lines of the
attached patch, to clarify this?

[-- Attachment #1.2: Type: text/html, Size: 2612 bytes --]

[-- Attachment #2: 0001-Document-that-the-match-function-for-a-widget-takes-.patch --]
[-- Type: text/x-patch, Size: 2018 bytes --]

From 3d3b280b64e92da65bd97f8b3951ca8b4a8fd536 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Mon, 2 Nov 2020 09:33:55 -0300
Subject: [PATCH] Document that the :match function for a widget takes an
 external value

* doc/misc/widget.texi (Basic Types): Document what an external value
is.  Document that a :match function expects the value to be in the
external format.  (Bug#8717)
---
 doc/misc/widget.texi | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/doc/misc/widget.texi b/doc/misc/widget.texi
index 3ce27a12a0..83a6c4c8d2 100644
--- a/doc/misc/widget.texi
+++ b/doc/misc/widget.texi
@@ -484,9 +484,21 @@ Basic Types
 The following keyword arguments apply to all widgets:
 
 @table @code
+@cindex internal format
+@cindex external format
 @vindex value@r{ keyword}
 @item :value
-The initial value for widgets of this type.
+The initial value for widgets of this type.  Typically, a widget
+represents its value in two formats: external and internal.  The
+external format is the value as the rest of Emacs sees it, and the
+internal format is a representation that the widget defines and uses
+in a widget specific way.
+
+Both formats might be the same for certain widgets and might differ
+for others, and there is no guarantee about which format the value
+stored in the @code{:value} property has.  However, when creating a
+widget or defining a new one (@pxref{Defining New Widgets}), the
+@code{:value} should be in the external format.
 
 @vindex format@r{ keyword}
 @item :format
@@ -629,8 +641,9 @@ Basic Types
 
 @vindex match@r{ keyword}
 @item :match
-Should be a function called with two arguments, the widget and a value,
-and returning non-@code{nil} if the widget can represent the specified value.
+Should be a function called with two arguments, the widget and an
+external value, and should return non-@code{nil} if the widget can
+represent the specified value.
 
 @vindex validate@r{ keyword}
 @item :validate
-- 
2.29.0