bug#51041: 28.0.60; toggle-truncate-lines should not print message

bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Tyler Grinn]

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


(toggle-truncate-lines t)

Prints the message "Truncate long lines enabled". When called from
elisp, this message should be ignored. If called interactively, the
message should be displayed.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: toggle truncate lines patch --]
[-- Type: text/x-diff, Size: 1049 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index 3695415163..0df66ab8a8 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8313,13 +8313,14 @@ toggle-truncate-lines
 		      (if (eq buffer (window-buffer window))
 			  (set-window-hscroll window 0)))
 		    nil t)))
-  (message "Truncate long lines %s%s"
-	   (if truncate-lines "enabled" "disabled")
-           (if (and truncate-lines visual-line-mode)
-               (progn
-                 (visual-line-mode -1)
-                 (format-message " and `visual-line-mode' disabled"))
-             "")))
+  (let ((disable-vlm (and truncate-lines visual-line-mode)))
+    (if disable-vlm (visual-line-mode -1))
+    (if (called-interactively-p 'any)
+        (message "Truncate long lines %s%s"
+	         (if truncate-lines "enabled" "disabled")
+                 (if disable-vlm
+                     (format-message " and `visual-line-mode' disabled")
+                   "")))))
 
 (defun toggle-word-wrap (&optional arg)
   "Toggle whether to use word-wrapping for continuation lines.

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



In GNU Emacs 28.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-10-04 built on tyler-hp
Repository revision: 3a9d5f04fb23cc89797f5d5102607c790440201e
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-mailutils --with-cairo --with-native-compilation
 --with-json --with-modules --with-gnutls --with-harfbuzz
 --with-xwidgets --prefix=/home/tyler/code/emacs 'CFLAGS=-O3
 -mtune=native -march=native -fomit-frame-pointer''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

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
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
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
xwidget-internal dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 70085 8512)
 (symbols 48 6642 0)
 (strings 32 19589 1584)
 (string-bytes 1 672318)
 (vectors 16 14113)
 (vector-slots 8 298530 14069)
 (floats 8 22 31)
 (intervals 56 331 1)
 (buffers 992 11))

bug#51041: [External] : bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Drew Adams]

> (toggle-truncate-lines t)
> 
> Prints the message "Truncate long lines enabled". When called from
> elisp, this message should be ignored. If called interactively, the
> message should be displayed.

Yes.

But the right fix is to add "&optional msg",
use (interactive "P\np"), and test for non-nil
MSG as the condition for showing the message.

Lisp code for a command can use that toggle
function, and when _that_ command is invoked
interactively it too might make sense to show
the message (that can depend on the command
and when the toggling occurs as part of it).

The same kind of fix is no doubt appropriate
for some other existing commands that instead
just test `called-interactively-p'.  Surely
we shouldn't perpetuate such design by adding
more such.

bug#51041: [External] : bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Tyler Grinn]

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

Drew Adams <drew.adams@oracle.com> writes:

>> (toggle-truncate-lines t)
>> 
>> Prints the message "Truncate long lines enabled". When called from
>> elisp, this message should be ignored. If called interactively, the
>> message should be displayed.
>
> Yes.
>
> But the right fix is to add "&optional msg",
> use (interactive "P\np"), and test for non-nil
> MSG as the condition for showing the message.
>
> Lisp code for a command can use that toggle
> function, and when _that_ command is invoked
> interactively it too might make sense to show
> the message (that can depend on the command
> and when the toggling occurs as part of it).
>
> The same kind of fix is no doubt appropriate
> for some other existing commands that instead
> just test `called-interactively-p'.  Surely
> we shouldn't perpetuate such design by adding
> more such.

Sounds good to me.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: toggle-truncate-lines should not print message --]
[-- Type: text/x-diff, Size: 2015 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index 3695415163..47a00fca3c 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8294,14 +8294,15 @@ set-selective-display
 
 (defvaralias 'indicate-unused-lines 'indicate-empty-lines)
 
-(defun toggle-truncate-lines (&optional arg)
+(defun toggle-truncate-lines (&optional arg msg)
   "Toggle truncating of long lines for the current buffer.
-When truncating is off, long lines are folded.
-With prefix argument ARG, truncate long lines if ARG is positive,
-otherwise fold them.  Note that in side-by-side windows, this
-command has no effect if `truncate-partial-width-windows' is
-non-nil."
-  (interactive "P")
+When truncating is off, long lines are folded.  With prefix
+argument ARG, truncate long lines if ARG is positive, otherwise
+fold them.  When called interactively or if MSG is non-nil, print
+a message describing the new state of truncate-lines.  Note that
+in side-by-side windows, this command has no effect if
+`truncate-partial-width-windows' is non-nil."
+  (interactive "P\np")
   (setq truncate-lines
 	(if (null arg)
 	    (not truncate-lines)
@@ -8313,13 +8314,14 @@ toggle-truncate-lines
 		      (if (eq buffer (window-buffer window))
 			  (set-window-hscroll window 0)))
 		    nil t)))
-  (message "Truncate long lines %s%s"
-	   (if truncate-lines "enabled" "disabled")
-           (if (and truncate-lines visual-line-mode)
-               (progn
-                 (visual-line-mode -1)
-                 (format-message " and `visual-line-mode' disabled"))
-             "")))
+  (let ((disable-vlm (and truncate-lines visual-line-mode)))
+    (if disable-vlm (visual-line-mode -1))
+    (if msg
+        (message "Truncate long lines %s%s"
+	         (if truncate-lines "enabled" "disabled")
+                 (if disable-vlm
+                     (format-message " and `visual-line-mode' disabled")
+                   "")))))
 
 (defun toggle-word-wrap (&optional arg)
   "Toggle whether to use word-wrapping for continuation lines.

bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> (toggle-truncate-lines t)
>>
>> Prints the message "Truncate long lines enabled". When called from
>> elisp, this message should be ignored. If called interactively, the
>> message should be displayed.
>
> Yes.
>
> But the right fix is to add "&optional msg",
> use (interactive "P\np"), and test for non-nil
> MSG as the condition for showing the message.
>
> Lisp code for a command can use that toggle
> function, and when _that_ command is invoked
> interactively it too might make sense to show
> the message (that can depend on the command
> and when the toggling occurs as part of it).
>
> The same kind of fix is no doubt appropriate
> for some other existing commands that instead
> just test `called-interactively-p'.  Surely
> we shouldn't perpetuate such design by adding
> more such.

I don't think it's worth making the call signature of
`toggle-truncate-lines' more complicated just for the purpose of showing
a message.

Why should a different command want to show this message?  Why can't
that calling Lisp code just show an appropriate message itself?
Do we have a concrete use-case for it, or is it just a "maybe nice to
have"?

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Drew Adams]

> > Yes.
> >
> > But the right fix is to add "&optional msg",
> > use (interactive "P\np"), and test for non-nil
> > MSG as the condition for showing the message.
> >
> > Lisp code for a command can use that toggle
> > function, and when _that_ command is invoked
> > interactively it too might make sense to show
> > the message (that can depend on the command
> > and when the toggling occurs as part of it).
> >
> > The same kind of fix is no doubt appropriate
> > for some other existing commands that instead
> > just test `called-interactively-p'.  Surely
> > we shouldn't perpetuate such design by adding
> > more such.
> 
> I don't think it's worth making the call signature of
> `toggle-truncate-lines' more complicated just for the purpose of
> showing a message.
> 
> Why should a different command want to show this message?

Why shouldn't it?

> Why can't that calling Lisp code just show an appropriate message itself?
> Do we have a concrete use-case for it, or is it just a "maybe nice to
> have"?

Do as you like.  IMHO, using optional MSG corresponds
to our advertised suggestion and convention.  It makes
the code more flexible and more usable by other code.

(elisp) Distinguish Interactive:

  The recommended way to test whether the function
  was called using ‘call-interactively’ is to give
  it an optional argument ‘print-message’ and use
  the ‘interactive’ spec to make it non-‘nil’ in
  interactive calls.  Here’s an example:

     (defun foo (&optional print-message)
       (interactive "p")
       (when print-message
         (message "foo")))

`foo' is a command.  This is about calling `foo'
from Lisp, in particular by some other command.
That command can choose whether it makes sense,
for its own interaction, for a user to see the
message from `foo'.  `foo's signature lets the
calling command do TRT directly.

Typical use of such an approach is for a command
someone writes that does more than one thing, and
that can benefit from such reporting echoes -
multiple steps, for example.  The code making use
of such a command as a function might have no way
of knowing what/which message(s) the called command
might report.

This corresponds to what we recommend.  We do NOT
recommend testing with `called-interactively', for
example.  There's no reason to hard-code whether a
command called from Lisp can emit a message.  Using
an optional arg MSG or NOMSG is TRT.

There's lots of old code that does hard-code things
here.  `count-words' and `count-words-region' are
good examples of such bad examples.

But again, you will do what you will.

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

> This corresponds to what we recommend.  We do NOT
> recommend testing with `called-interactively', for
> example.  There's no reason to hard-code whether a
> command called from Lisp can emit a message.  Using
> an optional arg MSG or NOMSG is TRT.
>
> There's lots of old code that does hard-code things
> here.  `count-words' and `count-words-region' are
> good examples of such bad examples.

So this is about avoiding `called-interactively-p'?  If so, I think an
argument is indeed the way to do that.

Another solution would be to have one function meant for interactive
use, and another one for use from Lisp.  That might be too much in this
case, though, as this function seems somewhat uncommon.

A third one would be to do nothing, and say: it's up to the caller to
just call (message "") to clear the echo area, if they don't want the
message there.

But actually, isn't the way to truncate lines just to say this?

    (setq truncate-lines t)

In summary, I don't think I understand the use-case here.

bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> But actually, isn't the way to truncate lines just to say this?
>
>     (setq truncate-lines t)
>
> In summary, I don't think I understand the use-case here.

Yes, `toggle-truncate-lines' looks like a user-level command to me.  The
only places it's called in Emacs is from places like
`menu-bar--wrap-long-lines-window-edge' (where it should issue the
message) and `gnus-article-toggle-truncate-lines' (ditto).

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

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Drew Adams]

> Yes, `toggle-truncate-lines' looks like a user-level command to me.

Of course it's a user-level command.  That doesn't
preclude it from being usefully called from Lisp.

> The only places it's called in Emacs is ...

Irrelevant.  Functions can be, and are, called
from code written by Emacs users, including
3rd-party libraries.

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> Yes, `toggle-truncate-lines' looks like a user-level command to me.
>
> Of course it's a user-level command.  That doesn't
> preclude it from being usefully called from Lisp.

What is the use-case for calling it from Lisp?

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Drew Adams]

> >> Yes, `toggle-truncate-lines' looks like a user-level command to me.
> >
> > Of course it's a user-level command.  That doesn't
> > preclude it from being usefully called from Lisp.
> 
> What is the use-case for calling it from Lisp?

What is the use case for hard-coding behavior
based on called-interactively?  Why go against
what Emacs recommends?  Why reduce flexibility?

The guideline makes sense.  There's no need to
a priori justify following it by presupposing
some concrete code that invokes the command.

What's your specific justification for flouting
the guideline in this case?

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> What is the use-case for calling it from Lisp?
>
> What is the use case for hard-coding behavior
> based on called-interactively?

Drew, it is draining to discuss this way.  I have alredy agreed that
an argument is better than 'called-interactively-p'.  No one is
arguing for that.  Please drop it.  Read the thread.

The argument is that from Lisp you can just set the `truncate-lines'
variable instead of calling `toggle-truncate-lines'.

Does that not work?  If it doesn't, please explain the use-case.

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Drew Adams]

> >> What is the use-case for calling it from Lisp?
> >
> > What is the use case for hard-coding behavior
> > based on called-interactively?
> 
> Drew, it is draining to discuss this way.  I have alredy agreed that
> an argument is better than 'called-interactively-p'.  No one is
> arguing for that.  Please drop it.  Read the thread.
> 
> The argument is that from Lisp you can just set the `truncate-lines'
> variable instead of calling `toggle-truncate-lines'.
> 
> Does that not work?  If it doesn't, please explain the use-case.

Stefan - it's draining to discuss this way.

No one has argued that a command should invoke
`toggle-truncate-lines' instead of setting var
`truncate-lines'.  Please drop it.  Read the
thread.

You're free to close the bug as not-a-bug or
whatever.  The bug report asks _how to invoke
the command from Lisp_ without printing a msg.
You're free to tell the OP to just not do that.

The point is that the command _need not_ have
hard-coded behavior that always prints a msg.
That just complicates things needlessly.

It could simply follow the guidelines and thus
(1) print when invoked interactively and (2)
provide a way to invoke it without printing
(and with the bonus, not provided by setting
the variable, that code _can_ instead choose
to _show_ the message).

The command should have been coded in the
conventional way from the beginning (you
apparently agree now).  Is it worth fixing,
to DTRT now?  That's not my call.  Please
stop haranguing me (present examples etc.).
You will anyway do what you want.

But if you'd like to respond to the actual
bug report, and not just close as won't-fix,
then (you apparently agree) the right fix is
to follow the guideline: add "&optional msg"
for message control.

Do I care whether you do that?  Not a bit.
(Maybe a tiny bit, wishing the best for Emacs.)

Just trying to help, by being clear about how
the command _should_ have been coded (IMHO -
and following the convention, which is a good
one).  You agree with that now.  Underbar.

(Oh, BTW: `toggle-truncate-lines' does more
than change the variable value and print a msg,
in particular when turning off truncation.)

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Tyler Grinn]

Drew Adams <drew.adams@oracle.com> writes:

>> >> What is the use-case for calling it from Lisp?
>> >
>> > What is the use case for hard-coding behavior
>> > based on called-interactively?
>> 
>> Does that not work?  If it doesn't, please explain the use-case.
>
> Stefan - it's draining to discuss this way.
>
> You're free to close the bug as not-a-bug or
> whatever.  The bug report asks _how to invoke
> the command from Lisp_ without printing a msg.
> You're free to tell the OP to just not do that.
>
> (Oh, BTW: `toggle-truncate-lines' does more
> than change the variable value and print a msg,
> in particular when turning off truncation.)

Yeah, I would like some direction on this.

My understanding is that if some functionality or minor mode provides a
function to toggle it, it is a best practice to use that function rather
than re-implement it, such that if the behavior of the toggle function
changes my code doesn't have to change.

-- 

Best,

Tyler

bug#51041: [External] : Re: bug#51041: 28.0.60; toggle-truncate-lines should not print message

[Lars Ingebrigtsen]

Tyler Grinn <tylergrinn@gmail.com> writes:

> My understanding is that if some functionality or minor mode provides a
> function to toggle it, it is a best practice to use that function rather
> than re-implement it, such that if the behavior of the toggle function
> changes my code doesn't have to change.

We have a number of commands that are meant to be used for interactive
use only (and will give a warning if used from code).  We also have some
commands that are OK to use from code, but only if that code is
user-oriented, and `toggle-truncate-lines' seems very much like that.

Normal code that needs to change truncate-lines should just set the
variable -- which is what happens already.  There's about a hundred
instances of "setq.*truncate-lines" in the Emacs tree, while there's
three calls to `toggle-truncate-lines' (and all are from code that are
user oriented and should message).

So I agree with Stefan -- altering `toggle-truncate-lines' like
suggested isn't appropriate, and I'm closing this bug report.

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