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

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

[Claudio Bley]

1. M-x customize-option RET whitespace-display-mappings RET
2. click INS to insert an arbitrary char into one of the vectors
3. click the State button and select "Set for current session"

You should see this error / backtrace:

Debugger entered--Lisp error: (error "This field should contain a single character")
  signal(error ("This field should contain a single character"))
  error("%s" "This field should contain a single character")
  custom-variable-set((custom-variable :documentation-shown t
  :custom-state modified :tag "Whitespace Display Mappings" :value
  whitespace-display-mappings :custom-form edit :custom-magic
  [...]
  call-interactively(widget-button-click nil nil)
  command-execute(widget-button-click)


In wid-edit.el the character widget is defined as

(define-widget 'character 'editable-field
  "A character."
  :tag "Character"
  :value 0
  :size 1
  :format "%{%t%}: %v\n"
  :valid-regexp "\\`.\\'"
  :error "This field should contain a single character"
  [...]

Note, that the regexp does not match a single newline character, which
happens to be the problem here, as the default value of
`whitespace-display-mappings' is

((space-mark 32 [183] [46])
 (space-mark 160 [164] [95])
 (newline-mark 10 [36 10])  ;; <- ?\n here
 (tab-mark 9 [187 9] [92 9]))

I think the regexp should be changed to "\\`\(.\|\n\)\\'" to allow a
single newline also.

Claudio
-- 
Claudio

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

[Glenn Morris]

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

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

[Claudio Bley]

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.

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?

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?

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?
-- 
Claudio

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

[Stefan Monnier]

> How about displaying those chars in the buffer in another format?
> E.g. use escape sequences like \n \r et cetera?

Agreed.  It could use the same syntax as used after the ? in the Elisp
source code.  I.e. the conversion would be (read-from-string (concat
"?" field)) in one direction and (substring (prin1-char char) 1) in the other.


        Stefan

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

[Mauro Aranda]

[-- 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

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

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

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

Thanks; applied to Emacs 28.

The bits that remains to be fixed here is the display of the TAB/newline
(etc.) characters?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> 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).
>
> Thanks; applied to Emacs 28.

Thank you.

> The bits that remains to be fixed here is the display of the TAB/newline
> (etc.) characters?

Yes.  I think there were two suggestions, which if I'm not mistaken can
be summarized as:
- Display newline as \n, tab as \t, etc.
- Display newline as C-j (or ?\\C-j, or \\C-j, etc), tab as C-i, etc.

To those suggestions, I add one of mine:
- Display newline as ^J, tab as ^I, etc.

That is what Isearch does, and I think it would not be much of a
trouble to implement that.  Furthermore, we already display other
non-printable characters like that, so my suggestion would just
change newline and tab, I think.

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

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

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Yes.  I think there were two suggestions, which if I'm not mistaken can
> be summarized as:
> - Display newline as \n, tab as \t, etc.
> - Display newline as C-j (or ?\\C-j, or \\C-j, etc), tab as C-i, etc.
>
> To those suggestions, I add one of mine:
> - Display newline as ^J, tab as ^I, etc.
>
> That is what Isearch does, and I think it would not be much of a
> trouble to implement that.  Furthermore, we already display other
> non-printable characters like that, so my suggestion would just
> change newline and tab, I think.

Isearch is a bit special here (being interactive)...

My guess is that more people understand that \n is newline than people
remember that ^J is newline.  (The same goes for \t/^I, but probably to
a lesser degree.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Yes.  I think there were two suggestions, which if I'm not mistaken can
>> be summarized as:
>> - Display newline as \n, tab as \t, etc.
>> - Display newline as C-j (or ?\\C-j, or \\C-j, etc), tab as C-i, etc.
>>
>> To those suggestions, I add one of mine:
>> - Display newline as ^J, tab as ^I, etc.
>>
>> That is what Isearch does, and I think it would not be much of a
>> trouble to implement that.  Furthermore, we already display other
>> non-printable characters like that, so my suggestion would just
>> change newline and tab, I think.
>
> Isearch is a bit special here (being interactive)...
>
> My guess is that more people understand that \n is newline than people
> remember that ^J is newline.  (The same goes for \t/^I, but probably to
> a lesser degree.)

Those are good points.  A couple of questions:
- Do we change the display for the space character as well?

We could display it as ?\s, or leave it as " ", which makes the
character widget appear empty, when it's not.

- What do we do with the other escape sequences, like ?\r and ?\f?

Right now, we display those as ^M and ^L respectively.  If we keep this
representation, maybe somebody will feel there is some inconsistency,
because some characters we display as ^M, while others as \n.  Perhaps
is not a big deal, though.

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

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

[Mauro Aranda]

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

Mauro Aranda <maurooaranda@gmail.com> writes:

> - Do we change the display for the space character as well?
>
> We could display it as ?\s, or leave it as " ", which makes the
> character widget appear empty, when it's not.

I said display it as ?\s, but I meant display it as \s.  Sorry.

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

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

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Those are good points.  A couple of questions:
> - Do we change the display for the space character as well?
>
> We could display it as ?\s, or leave it as " ", which makes the
> character widget appear empty, when it's not.
>
> - What do we do with the other escape sequences, like ?\r and ?\f?
>
> Right now, we display those as ^M and ^L respectively.  If we keep this
> representation, maybe somebody will feel there is some inconsistency,
> because some characters we display as ^M, while others as \n.  Perhaps
> is not a big deal, though.

We should definitely strive for consistency here...  \r and \f for ^M
and ^L is fine by me (although I guess more people are familiar with ^L
than \f).

As for space...  Hm.  It's unfortunate that it's displayed just as a
blank, so displaying it as \s would definitely make sense.  But it also
sounds a bit confusing.  :-/  Could we ... use a different background
colour on the space to indicate that it's there?  I guess that's not all
that easy to interpret, either...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 25 Sep 2020 12:00:07 +0200
> Cc: Glenn Morris <rgm@gnu.org>, 15925@debbugs.gnu.org,
>  claudio.bley@googlemail.com
> 
> > - What do we do with the other escape sequences, like ?\r and ?\f?
> >
> > Right now, we display those as ^M and ^L respectively.  If we keep this
> > representation, maybe somebody will feel there is some inconsistency,
> > because some characters we display as ^M, while others as \n.  Perhaps
> > is not a big deal, though.
> 
> We should definitely strive for consistency here...  \r and \f for ^M
> and ^L is fine by me (although I guess more people are familiar with ^L
> than \f).

I very much hope we leave the ^M and ^L display alone.  This is what
we did since day one (and yes, \n is a special case), and I'd rather
we didn't change that just in the name of consistency.

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

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Those are good points.  A couple of questions:
>> - Do we change the display for the space character as well?
>>
>> We could display it as ?\s, or leave it as " ", which makes the
>> character widget appear empty, when it's not.
>>
>> - What do we do with the other escape sequences, like ?\r and ?\f?
>>
>> Right now, we display those as ^M and ^L respectively.  If we keep this
>> representation, maybe somebody will feel there is some inconsistency,
>> because some characters we display as ^M, while others as \n.  Perhaps
>> is not a big deal, though.
>
> We should definitely strive for consistency here...  \r and \f for ^M
> and ^L is fine by me (although I guess more people are familiar with ^L
> than \f).
>
> As for space...  Hm.  It's unfortunate that it's displayed just as a
> blank, so displaying it as \s would definitely make sense.  But it also
> sounds a bit confusing.  :-/  Could we ... use a different background
> colour on the space to indicate that it's there?  I guess that's not all
> that easy to interpret, either...

I don't think that a different background colour will be much of a
gain.  It will just look like the widget has a different background
color...After editing the widget it might be easier to interpret what's
going on, but then again the same can be said with the current state.

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

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

[Mauro Aranda]

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

How does the attached patch look? I didn't include changes for the rest
of the escape sequences, because of Eli's objections.

The reason I used the notify function is because it is much simpler than
rebinding insertion commands and having to copy some of the
functionality.  And I don't think the change in the calls to the notify
function for editable-field widgets (and descendants) will be much of a
problem.  I looked for functions that assume EVENT is nil and found
none, nor in-tree or in third-party packages.

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

[-- Attachment #2: 0001-Display-some-character-widget-values-in-a-more-user-.patch --]
[-- Type: text/x-patch, Size: 4874 bytes --]

From e5ccdb868b71667797dbf7399e9247503d71b809 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 25 Sep 2020 11:16:14 -0300
Subject: [PATCH] Display some character widget values in a more user-friendly
 way (Bug#15925)

* lisp/wid-edit.el (widget-character--escape-sequences-alist): New
variable.
(widget-character--change-character-display): New function.  Use the new
variable.
(widget-character-notify): New function, to keep track of the changes
in the character widget, and display characters like tab,
newline and spaces better.
(character widget): Use widget-character-notify as the notify
function.  Use widget-character--change-character-display for the
internal representation of value.
---
 lisp/wid-edit.el | 66 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 8ad99f49aa..509c487bf2 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1370,7 +1370,8 @@ widget-before-change
 	     (signal 'text-read-only
 		     '("Attempt to change text outside editable field")))
 	    (widget-field-use-before-change
-	     (widget-apply from-field :notify from-field))))))
+	     (widget-apply from-field :notify
+                           from-field (list 'before-change from to)))))))
 
 (defun widget-add-change ()
   (remove-hook 'post-command-hook 'widget-add-change t)
@@ -1407,7 +1408,7 @@ widget-after-change
 				 (> (point) begin))
 		       (delete-char -1)))))))
 	(widget-specify-secret field))
-      (widget-apply field :notify field))))
+      (widget-apply field :notify field (list 'after-change from to)))))
 
 ;;; Widget Functions
 ;;
@@ -3533,13 +3534,70 @@ 'character
   :value-to-internal (lambda (_widget value)
 		       (if (stringp value)
 			   value
-			 (char-to-string value)))
+                         (let ((disp
+                                (widget-character--change-character-display
+                                 value)))
+                           (if disp
+                               (propertize (char-to-string value) 'display disp)
+                             (char-to-string value)))))
   :value-to-external (lambda (_widget value)
 		       (if (stringp value)
 			   (aref value 0)
 			 value))
   :match (lambda (_widget value)
-	   (characterp value)))
+	   (characterp value))
+  :notify #'widget-character-notify)
+
+;; Only some escape sequences, not all of them.  (Bug#15925)
+(defvar widget-character--escape-sequences-alist
+  '((?\t . ?t)
+    (?\n . ?n)
+    (?\s . ?s))
+  "Alist that associates escape sequences to a character.
+Each element has the form (ESCAPE-SEQUENCE . CHARACTER).
+
+The character widget uses this alist to display the
+non-printable character represented by ESCAPE-SEQUENCE as \\CHARACTER,
+since that makes it easier to see what's in the widget.")
+
+(defun widget-character--change-character-display (c)
+  "Return a string to represent the character C, or nil.
+
+The character widget represents some characters (e.g., the newline character
+or the tab character) specially, to make it easier for the user to see what's
+in it.  For those characters, return a string to display that character in a
+more user-friendly way.
+
+For the caller, nil should mean that it is good enough to use the return value
+of `char-to-string' for the representation of C."
+  (let ((char (alist-get c widget-character--escape-sequences-alist)))
+    (and char (propertize (format "\\%c" char) 'face 'escape-glyph))))
+
+(defun widget-character-notify (widget child &optional event)
+  "Notify function for the character widget.
+
+This function allows the widget character to better display some characters,
+like the newline character or the tab character."
+  (when (eq (car-safe event) 'after-change)
+    (let* ((start (nth 1 event))
+           (end (nth 2 event))
+           str)
+      (if (eql start end)
+          (when (char-equal (widget-value widget) ?\s)
+            ;; The character widget is not really empty:
+            ;; its value is a single space character.
+            ;; We need to propertize it again, if it became empty for a while.
+            (let ((ov (widget-get widget :field-overlay)))
+              (put-text-property
+               (overlay-start ov) (overlay-end ov)
+               'display (widget-character--change-character-display ?\s))))
+        (setq str (buffer-substring-no-properties start end))
+        ;; This assumes the user enters one character at a time,
+        ;; and does nothing crazy, like yanking a long string.
+        (let ((disp (widget-character--change-character-display (aref str 0))))
+          (when disp
+            (put-text-property start end 'display disp))))))
+  (widget-default-notify widget child event))
 
 (define-widget 'list 'group
   "A Lisp list."
-- 
2.28.0

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

[Drew Adams]

> I very much hope we leave the ^M and ^L display alone.  This is what
> we did since day one (and yes, \n is a special case), and I'd rather
> we didn't change that just in the name of consistency.

+1.  But why not for all control chars?

I haven't been following this thread - no
doubt I'm not seeing special cases etc.
___

Currently, `ctl-arrow' controls the display.
That works for me.

And any code that changes the display-table
entry of a char has its intended effect.
Also good.
___

(Maybe `ctl-arrow' could be extended, to allow
different non-nil values to specify different
representations, including \n and ?\n.  For
compatibility, nil would still specify octal.)

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

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> How does the attached patch look? I didn't include changes for the rest
> of the escape sequences, because of Eli's objections.

Looks good.  The only thing that's confusing now is that if you delete
the char, it displays as \s?

M-x customize-variable RET whitespace-display-mappings RET



-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

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

(Oops.  Sent the previous mail too early.)

Mauro Aranda <maurooaranda@gmail.com> writes:

> How does the attached patch look? I didn't include changes for the rest
> of the escape sequences, because of Eli's objections.

Looks good.  The only thing that's confusing now is that if you delete
the char, it displays as \s?

M-x customize-variable RET whitespace-display-mappings RET


[-- Attachment #2: Type: image/png, Size: 4437 bytes --]

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


Hit DEL:


[-- Attachment #4: Type: image/png, Size: 10792 bytes --]

[-- Attachment #5: Type: text/plain, Size: 199 bytes --]



So there's no difference in the display between a missing character and
a space character...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (Oops.  Sent the previous mail too early.)
>
> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> How does the attached patch look? I didn't include changes for the rest
>> of the escape sequences, because of Eli's objections.
>
> Looks good.  The only thing that's confusing now is that if you delete
> the char, it displays as \s?
>
> M-x customize-variable RET whitespace-display-mappings RET
>
>
>
>
> Hit DEL:
>
>
>
>
>
> So there's no difference in the display between a missing character and
> a space character...

I did that on purpose, and this comment tries to explain that:
;; The character widget is not really empty:
;; its value is a single space character.
;; We need to propertize it again, if it became empty for a while.

Try the following in current master (i.e., without my patch applied):

(defcustom foo-option ?. "..."
  :type 'character
  :group 'emacs)

Eval, and confirm that foo-option value is ?.

M-x customize-option RET foo-option RET

Delete the ".", and then click State and Set for Current Session.
You'll see that foo-option was set...but to what? You said there is a
missing character, but really there's not such thing: there is a space
there, and the Widget library will return ?\s for the value of that
widget.

If you do:
M-: foo-option RET
you can confirm foo-option was set to ?\s

That's why I thought it would be less confusing to show the \s at all
times, and say "No, you can't have an empty character widget".  Having a
truly empty character widget wouldn't be of much help, since the
:validate function will complain anyway because the widget doesn't have
a single character.

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

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

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> That's why I thought it would be less confusing to show the \s at all
> times, and say "No, you can't have an empty character widget".  Having a
> truly empty character widget wouldn't be of much help, since the
> :validate function will complain anyway because the widget doesn't have
> a single character.

Ah, I see.  Thanks for the explanation.  Then your patch makes perfect
sense here, and I've applied it to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no