bug#15925: 24.3.50; error when customizing whitespace-display-mappings

[Mauro Aranda <maurooaranda@gmail.com>]

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

Claudio Bley <claudio.bley@googlemail.com> writes:

> At Tue, 19 Nov 2013 03:24:02 -0500,
> Glenn Morris wrote:
>>
>> Claudio Bley wrote:
>>
>> > I think the regexp should be changed to "\\`\(.\|\n\)\\'" to allow a
>> > single newline also.
>>
>> We already tried that once; it caused more problems than it solved:
>> http://debbugs.gnu.org/2689
>
> I see. So lets get this solved.
>
> At first, changing the regexp is just the right thing to do and
> in itself, IMO, does not break anything.

Indeed.

> Alas, it does not solve the problem at hand because
> `widget-specify-field' behaves /badly/ when the field ends with a
> newline.
>
> So, actually, this hunk of your patch trying to work around
> this behaviour broke other things.

[...]

> How about using a special :value-create function for character fields?

I don't think that's needed.

> Furthermore, the presentation of the character values is bad,
> usability-wise, because for nonprintable chars you cannot see what
> value the field actually has.
>
> How about displaying those chars in the buffer in another format?
> E.g. use escape sequences like \n \r et cetera?

Or we could do what Isearch does, and display TAB as ^I, and so on.

> I (rather hackish) way to fix this would be to always insert a ?\n
> as part of the value of a character field:
>
> (defun widget-field-char-create (widget)
>   "Create an editable character field."
>   (let ((value (widget-get widget :value))
> (from (point))
> ;; This is changed to a real overlay in `widget-setup'.  We
> ;; need the end points to behave differently until
> ;; `widget-setup' is called.
> (overlay (cons (make-marker) (make-marker))))
>     (widget-put widget :field-overlay overlay)
>     (insert value)
>     (insert ?\n) ;; EEEK
>     (unless (memq widget widget-field-list)
>       (setq widget-field-new (cons widget widget-field-new)))
>     (move-marker (cdr overlay) (point))
>     (set-marker-insertion-type (cdr overlay) nil)
>     (insert ?\n)
>     (move-marker (car overlay) from)
>     (set-marker-insertion-type (car overlay) t)))
>
> What do you think?

I don't think this is needed.

I attach a patch that should fix this (but that does not change the
way the characters are displayed) and that doesn't introduce the
breakage of Bug#3136 (for which I added a test).

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

[-- Attachment #2: 0001-Allow-the-newline-character-in-the-character-widget-.patch --]
[-- Type: text/x-patch, Size: 3338 bytes --]

From fc2b1a4e65f0c525a5f78e3109cd3d2f697ac600 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 22 Sep 2020 11:10:15 -0300
Subject: [PATCH] Allow the newline character in the character widget
 (Bug#15925)

* lisp/wid-edit.el (widget-specify-field): Extend check for adding the
boundary overlay.  Plus, a minor comment indentation fix.
(character widget): Tweak the valid-regexp to allow the newline
character.

* test/lisp/wid-edit-tests.el (widget-test-character-widget-value)
(widget-test-editable-field-widget-value): New tests.
---
 lisp/wid-edit.el            | 11 +++++++----
 test/lisp/wid-edit-tests.el | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 13d850a57f..8ad99f49aa 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -303,12 +303,15 @@ widget-specify-field
 	 (or (not widget-field-add-space) (widget-get widget :size))))
     (if (functionp help-echo)
       (setq help-echo 'widget-mouse-help))
-    (when (= (char-before to) ?\n)
+    (when (and (or (> to (1+ from)) (null (widget-get widget :size)))
+               (= (char-before to) ?\n))
       ;; When the last character in the field is a newline, we want to
       ;; give it a `field' char-property of `boundary', which helps the
       ;; C-n/C-p act more naturally when entering/leaving the field.  We
-     ;; do this by making a small secondary overlay to contain just that
-      ;; one character.
+      ;; do this by making a small secondary overlay to contain just that
+      ;; one character.  BUT we only do this if there is more than one
+      ;; character (so we don't do this for the character widget),
+      ;; or if the size of the editable field isn't specified.
       (let ((overlay (make-overlay (1- to) to nil t nil)))
 	(overlay-put overlay 'field 'boundary)
         ;; We need the real field for tabbing.
@@ -3524,7 +3527,7 @@ 'character
   :value 0
   :size 1
   :format "%{%t%}: %v\n"
-  :valid-regexp "\\`.\\'"
+  :valid-regexp "\\`\\(.\\|\n\\)\\'"
   :error "This field should contain a single character"
   :value-get (lambda (w) (widget-field-value-get w t))
   :value-to-internal (lambda (_widget value)
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index 2ddb656fa9..df49ffc822 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -113,4 +113,20 @@ widget-test-newline-and-indent-same-widget
         (should (eq (current-column)
                     (widget-get grandchild :indent)))))))
 
+(ert-deftest widget-test-character-widget-value ()
+  "Check that we get the character widget's value correctly."
+  (with-temp-buffer
+    (let ((wid (widget-create '(character :value ?\n))))
+      (goto-char (widget-get wid :from))
+      (should (string= (widget-apply wid :value-get) "\n"))
+      (should (char-equal (widget-value wid) ?\n))
+      (should-not (widget-apply wid :validate)))))
+
+(ert-deftest widget-test-editable-field-widget-value ()
+  "Test that we get the editable field widget's value correctly."
+  (with-temp-buffer
+    (let ((wid (widget-create '(editable-field :value ""))))
+      (widget-insert "And some non-widget text.")
+      (should (string= (widget-apply wid :value-get) "")))))
+
 ;;; wid-edit-tests.el ends here
-- 
2.28.0