* bug#15925: 24.3.50; error when customizing whitespace-display-mappings @ 2013-11-19 7:37 Claudio Bley 2013-11-19 8:24 ` Glenn Morris 2020-09-22 15:42 ` Mauro Aranda 0 siblings, 2 replies; 19+ messages in thread From: Claudio Bley @ 2013-11-19 7:37 UTC (permalink / raw) To: 15925 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2013-11-19 7:37 bug#15925: 24.3.50; error when customizing whitespace-display-mappings Claudio Bley @ 2013-11-19 8:24 ` Glenn Morris 2013-11-20 10:35 ` Claudio Bley 2020-09-22 15:42 ` Mauro Aranda 1 sibling, 1 reply; 19+ messages in thread From: Glenn Morris @ 2013-11-19 8:24 UTC (permalink / raw) To: Claudio Bley; +Cc: 15925 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2013-11-19 8:24 ` Glenn Morris @ 2013-11-20 10:35 ` Claudio Bley 2013-11-20 19:28 ` Stefan Monnier 0 siblings, 1 reply; 19+ messages in thread From: Claudio Bley @ 2013-11-20 10:35 UTC (permalink / raw) To: Glenn Morris; +Cc: 15925 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2013-11-20 10:35 ` Claudio Bley @ 2013-11-20 19:28 ` Stefan Monnier 0 siblings, 0 replies; 19+ messages in thread From: Stefan Monnier @ 2013-11-20 19:28 UTC (permalink / raw) To: Claudio Bley; +Cc: 15925 > 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2013-11-19 7:37 bug#15925: 24.3.50; error when customizing whitespace-display-mappings Claudio Bley 2013-11-19 8:24 ` Glenn Morris @ 2020-09-22 15:42 ` Mauro Aranda 2020-09-23 13:46 ` Lars Ingebrigtsen 1 sibling, 1 reply; 19+ messages in thread From: Mauro Aranda @ 2020-09-22 15:42 UTC (permalink / raw) To: 15925; +Cc: Glenn Morris, claudio.bley [-- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-22 15:42 ` Mauro Aranda @ 2020-09-23 13:46 ` Lars Ingebrigtsen 2020-09-23 15:26 ` Mauro Aranda 0 siblings, 1 reply; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-23 13:46 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-23 13:46 ` Lars Ingebrigtsen @ 2020-09-23 15:26 ` Mauro Aranda 2020-09-24 14:38 ` Lars Ingebrigtsen 0 siblings, 1 reply; 19+ messages in thread From: Mauro Aranda @ 2020-09-23 15:26 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-23 15:26 ` Mauro Aranda @ 2020-09-24 14:38 ` Lars Ingebrigtsen 2020-09-24 15:29 ` Mauro Aranda 0 siblings, 1 reply; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-24 14:38 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-24 14:38 ` Lars Ingebrigtsen @ 2020-09-24 15:29 ` Mauro Aranda 2020-09-24 15:36 ` Mauro Aranda 2020-09-25 10:00 ` Lars Ingebrigtsen 0 siblings, 2 replies; 19+ messages in thread From: Mauro Aranda @ 2020-09-24 15:29 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-24 15:29 ` Mauro Aranda @ 2020-09-24 15:36 ` Mauro Aranda 2020-09-25 10:00 ` Lars Ingebrigtsen 1 sibling, 0 replies; 19+ messages in thread From: Mauro Aranda @ 2020-09-24 15:36 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-24 15:29 ` Mauro Aranda 2020-09-24 15:36 ` Mauro Aranda @ 2020-09-25 10:00 ` Lars Ingebrigtsen 2020-09-25 11:06 ` Eli Zaretskii 2020-09-25 11:10 ` Mauro Aranda 1 sibling, 2 replies; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-25 10:00 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-25 10:00 ` Lars Ingebrigtsen @ 2020-09-25 11:06 ` Eli Zaretskii 2020-09-25 11:10 ` Mauro Aranda 1 sibling, 0 replies; 19+ messages in thread From: Eli Zaretskii @ 2020-09-25 11:06 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: rgm, 15925, maurooaranda, claudio.bley > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-25 10:00 ` Lars Ingebrigtsen 2020-09-25 11:06 ` Eli Zaretskii @ 2020-09-25 11:10 ` Mauro Aranda 2020-09-25 14:32 ` Mauro Aranda 1 sibling, 1 reply; 19+ messages in thread From: Mauro Aranda @ 2020-09-25 11:10 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-25 11:10 ` Mauro Aranda @ 2020-09-25 14:32 ` Mauro Aranda 2020-09-26 13:24 ` Lars Ingebrigtsen 2020-09-26 13:26 ` Lars Ingebrigtsen 0 siblings, 2 replies; 19+ messages in thread From: Mauro Aranda @ 2020-09-25 14:32 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-25 14:32 ` Mauro Aranda @ 2020-09-26 13:24 ` Lars Ingebrigtsen 2020-09-26 13:26 ` Lars Ingebrigtsen 1 sibling, 0 replies; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-26 13:24 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-25 14:32 ` Mauro Aranda 2020-09-26 13:24 ` Lars Ingebrigtsen @ 2020-09-26 13:26 ` Lars Ingebrigtsen 2020-09-26 13:45 ` Mauro Aranda 1 sibling, 1 reply; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-26 13:26 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-26 13:26 ` Lars Ingebrigtsen @ 2020-09-26 13:45 ` Mauro Aranda 2020-09-26 15:10 ` Lars Ingebrigtsen 0 siblings, 1 reply; 19+ messages in thread From: Mauro Aranda @ 2020-09-26 13:45 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Glenn Morris, 15925, claudio.bley [-- 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 --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings 2020-09-26 13:45 ` Mauro Aranda @ 2020-09-26 15:10 ` Lars Ingebrigtsen 0 siblings, 0 replies; 19+ messages in thread From: Lars Ingebrigtsen @ 2020-09-26 15:10 UTC (permalink / raw) To: Mauro Aranda; +Cc: Glenn Morris, 15925, claudio.bley 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <<87mwl0vo36.wl%claudio.bley@gmail.com>]
[parent not found: <<CABczVwdS5QJkQh1+GBBoDB4WDtSCCOJRF48whp+it0TyJ1x23Q@mail.gmail.com>]
[parent not found: <<87a6xgd2rw.fsf@gnus.org>]
[parent not found: <<CABczVweQk+Jiiix62mS49n1zoHNn-6uifOEB1H4==EKNSV970g@mail.gmail.com>]
[parent not found: <<871rirb5ot.fsf@gnus.org>]
[parent not found: <<CABczVweW=gbepHOLzBQZbAxS9yX-Fo3h2_hPDL7FwmhfjAdk6w@mail.gmail.com>]
[parent not found: <<87ft76rxaw.fsf@gnus.org>]
[parent not found: <<83v9g2ru8f.fsf@gnu.org>]
* bug#15925: 24.3.50; error when customizing whitespace-display-mappings [not found] ` <<83v9g2ru8f.fsf@gnu.org> @ 2020-09-25 16:07 ` Drew Adams 0 siblings, 0 replies; 19+ messages in thread From: Drew Adams @ 2020-09-25 16:07 UTC (permalink / raw) To: Eli Zaretskii, Lars Ingebrigtsen; +Cc: rgm, 15925, maurooaranda, claudio.bley > 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.) ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-09-26 15:10 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-19 7:37 bug#15925: 24.3.50; error when customizing whitespace-display-mappings Claudio Bley 2013-11-19 8:24 ` Glenn Morris 2013-11-20 10:35 ` Claudio Bley 2013-11-20 19:28 ` Stefan Monnier 2020-09-22 15:42 ` Mauro Aranda 2020-09-23 13:46 ` Lars Ingebrigtsen 2020-09-23 15:26 ` Mauro Aranda 2020-09-24 14:38 ` Lars Ingebrigtsen 2020-09-24 15:29 ` Mauro Aranda 2020-09-24 15:36 ` Mauro Aranda 2020-09-25 10:00 ` Lars Ingebrigtsen 2020-09-25 11:06 ` Eli Zaretskii 2020-09-25 11:10 ` Mauro Aranda 2020-09-25 14:32 ` Mauro Aranda 2020-09-26 13:24 ` Lars Ingebrigtsen 2020-09-26 13:26 ` Lars Ingebrigtsen 2020-09-26 13:45 ` Mauro Aranda 2020-09-26 15:10 ` Lars Ingebrigtsen [not found] <<87mwl0vo36.wl%claudio.bley@gmail.com> [not found] ` <<CABczVwdS5QJkQh1+GBBoDB4WDtSCCOJRF48whp+it0TyJ1x23Q@mail.gmail.com> [not found] ` <<87a6xgd2rw.fsf@gnus.org> [not found] ` <<CABczVweQk+Jiiix62mS49n1zoHNn-6uifOEB1H4==EKNSV970g@mail.gmail.com> [not found] ` <<871rirb5ot.fsf@gnus.org> [not found] ` <<CABczVweW=gbepHOLzBQZbAxS9yX-Fo3h2_hPDL7FwmhfjAdk6w@mail.gmail.com> [not found] ` <<87ft76rxaw.fsf@gnus.org> [not found] ` <<83v9g2ru8f.fsf@gnu.org> 2020-09-25 16:07 ` Drew Adams
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).