bug#63838: 28.2; 'wid-edit.el' problems with 'integer' (and 'sexp') widgets.

bug#63838: 28.2; 'wid-edit.el' problems with 'integer' (and 'sexp') widgets.

[Marco Antoniotti]

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

Hi

The issue is with the following code that tries to use the Widget
library.  The slots of the widget are actually not necessary to see the
problem.

 (widget-create 'integer
                :help-echo "Enter an integer..."
                :notify (lambda (w1 &rest ws)
                           (ignore ws)
                           (message "WIFT: IF: %s"
                                    (widget-value w1)
                           ))
                :size 4
  )

If you set up a widget buffer, the field will appear as expected with a 0
as initial value.  The problem starts if you try to clear the field and
then enter a new value, say by hitting C-k, and the problem is in the
:value-to-external functions in the in sexp and restricted-sexp widgets
(the number, float and natnum widgets must also be fixed).

The library :value-to-external functions eventually do a (read value) where
value is the widget value; alas, value can be "" if you edit the field in a
pretty standard way, as, as I said above, if you had tried to kill the
field line (C-k); this causes the traditional minor demons to fly out of
the nose, as a 'End of file during parsing' error gets signaled.  After
that, the buffer with the widgets is pretty much in a random state.

Specializing the widget with a hack like the function below fixes the
problem, but a
good solution may need some more work to get all the corner cases.  Note
that setting a "widget buffer" in overwrite mode goes a long way to
mitigate these problems: maybe having widget-setup switch to it would be
another good thing to do.

  :value-to-external
  (lambda (widget value)
    ;; Lifted, and fixed, from `:value-to-external' in
    ;; `restricted-sexp'.

    (unless (stringp value)
      (display-warning
       'widget-bad-default-value
       (format-message
        "\nA widget of type %S has a bad default value.
value: %S
match function: %S
match-alternatives: %S"
        (widget-type widget)
        value
        (widget-get widget :match)
        (widget-get widget :match-alternatives))
       :warning))
    (if (and (stringp value) (string-equal "" value))
        ;; Oooops, we cannot just `read'.
        (widget-get widget :default-value)
      (read value))
    )

Note: this goes back, at least, to 25.x.

Thanks for the attention

Marco Antoniotti





In GNU Emacs 28.2 (build 2, x86_64-w64-mingw32)
 of 2022-09-13 built on AVALON
Windowing system distributor 'Microsoft Corp.', version 10.0.22621
System Description: Microsoft Windows 10 Pro (v10.0.2009.22621.1778)

Configured using:
 'configure --with-modules --without-dbus --with-native-compilation
 --without-compress-install CFLAGS=-O2'

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
XPM ZLIB

(NATIVE_COMP present but libgccjit not available)

Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived gnus-util rmail rmail-loaddefs
text-property-search mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date wombat-theme finder-inf tex-site
epg rfc6068 epg-config gnu-elpa-keyring-update slime-autoloads rx info
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
dos-w32 ls-lisp disp-table term/w32-win w32-win w32-vars term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads w32notify
w32 lcms2 multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 246351 7879)
 (symbols 48 14548 1)
 (strings 32 84982 2407)
 (string-bytes 1 2218067)
 (vectors 16 24066)
 (vector-slots 8 414464 16368)
 (floats 8 28 222)
 (intervals 56 377 9)
 (buffers 992 12))


-- 
Marco Antoniotti
Somewhere over the Rainbow

[-- Attachment #2: Type: text/html, Size: 7327 bytes --]

bug#63838: 28.2; 'wid-edit.el' problems with 'integer' (and 'sexp') widgets.

[Mauro Aranda]

Marco Antoniotti <marcoxa@gmail.com> writes:

 > Hi

Thanks for the bug report!

 > The issue is with the following code that tries to use the Widget
 > library.  The slots of the widget are actually not necessary to see the
 > problem.
 >
 >  (widget-create 'integer
 >                 :help-echo "Enter an integer..."
 >                 :notify (lambda (w1 &rest ws)
 >                            (ignore ws)
 >                            (message "WIFT: IF: %s"
 >                                     (widget-value w1)
 >                            ))
 >                 :size 4
 >   )

The :notify value is kind of necessary, right? I mean, the bug doesn't
manifest as long as code doesn't try to access the value of a
restricted-sexp widget while its empty.

 > Specializing the widget with a hack like the function below fixes the
 > problem, but a
 > good solution may need some more work to get all the corner cases.
 > Note that setting a "widget buffer" in overwrite mode goes a long way to
 > mitigate these problems: maybe having widget-setup switch to it would
 > be another good thing to do.

I don't think we want to do that.

 >  :value-to-external
 >   (lambda (widget value)
 >     ;; Lifted, and fixed, from `:value-to-external' in
 >     ;; `restricted-sexp'.
 >
 >     (unless (stringp value)
 >       (display-warning
 >        'widget-bad-default-value
 >        (format-message
 >         "\nA widget of type %S has a bad default value.
 > value: %S
 > match function: %S
 > match-alternatives: %S"
 >         (widget-type widget)
 >         value
 >         (widget-get widget :match)
 >         (widget-get widget :match-alternatives))
 >        :warning))
 >     (if (and (stringp value) (string-equal "" value))
 >         ;; Oooops, we cannot just `read'.
 >         (widget-get widget :default-value)
 >       (read value))
 >     )

I don't think this is a good place to do this.  And there's no
:default-value property in the widget library, just :value.

I think it'd be much better if we specialize the :value-get function for
the restricted-sexp widget, defaulting to return :value when the widget
is empty.

bug#63838: 28.2; 'wid-edit.el' problems with 'integer' (and 'sexp') widgets.

[Mauro Aranda]

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

tags 63838 patch
quit


I attach a patch to handle an empty restricted sexp widget, together
with a test.

[-- Attachment #2: 0001-Avoid-errors-when-a-restricted-sexp-widget-is-empty.patch --]
[-- Type: text/x-patch, Size: 2048 bytes --]

From a43899c409703af4966e619a92bda123e80122e0 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 9 Sep 2023 21:59:30 +0800
Subject: [PATCH] Avoid errors when a restricted-sexp widget is empty

* lisp/wid-edit.el (restricted-sexp): Don't try to read
an empty string when converting the current value to the
external format.  (Bug#63838)

* test/lisp/wid-edit-tests.el (widget-test-restricted-sexp-empty-val):
New test.
---
 lisp/wid-edit.el            |  4 +++-
 test/lisp/wid-edit-tests.el | 11 +++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index d18d721f7ed..74412414113 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -3681,7 +3681,9 @@ 'restricted-sexp
                           :warning)
                          ;; Make sure we will `read' a string.
                          (setq value (prin1-to-string value)))
-                       (read value)))
+                       (if (string-empty-p value)
+                           value
+                       (read value))))
 
 (defun widget-restricted-sexp-match (widget value)
   (let ((alternatives (widget-get widget :match-alternatives))
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index ebfe729bc9a..66bff4ad2e3 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -380,4 +380,15 @@ widget-test-alist-default-value-4
                                     :value (("1" . 1) ("2" . 2))))))
       (should (equal '(("1" . 1) ("2" . 2)) (widget-default-get w))))))
 
+(ert-deftest widget-test-restricted-sexp-empty-val ()
+  "Test that we handle an empty restricted-sexp widget just fine."
+  (with-temp-buffer
+    (let ((w (widget-create '(restricted-sexp
+                              :value 3
+                              :match-alternatives (integerp)))))
+      (widget-setup)
+      (widget-backward 1)
+      (delete-char 1)
+      (should (string= (widget-value w) "")))))
+
 ;;; wid-edit-tests.el ends here
-- 
2.34.1

bug#63838: 28.2; 'wid-edit.el' problems with 'integer' (and 'sexp') widgets.

[Eli Zaretskii]

> Cc: Marco Antoniotti <marcoxa@gmail.com>
> Date: Sat, 9 Sep 2023 12:13:53 -0300
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> I attach a patch to handle an empty restricted sexp widget, together
> with a test.

Thanks, installed on the master branch, and closing the bug.