all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline
@ 2024-12-22  7:43 Gerd Möllmann
  2024-12-22  8:22 ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Gerd Möllmann @ 2024-12-22  7:43 UTC (permalink / raw)
  To: 75020

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

Tags: patch

To reproduce, emacs -nw -Q on a terminal not supporting underlining (in
my case Terminal.app on macOS).

- M-x display-line-number-mode RET
- Eval (insert (amke-separator-line))

=> the separator line is too long

Attached patch fixes that.



In GNU Emacs 31.0.50 (build 4, aarch64-apple-darwin24.2.0) of 2024-12-21
 built on pro2
Repository revision: cbafbb2dd57993397c0d624461e3611831414e91
Repository branch: cl-packages
System Description:  macOS 15.2

Configured using:
 'configure --without-ns --cache-file
 /var/folders/1d/k_6t25f94sl83szqbf8gpkrh0000gn/T//config.cache.cl-packages
 --with-native-compilation --with-mps=yes CC=clang
 'CFLAGS=-Wgnu-imaginary-constant -Wunused-result -g
 -fno-omit-frame-pointer -F
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks
 -Wno-ignored-attributes -Wno-flag-enum -Wno-missing-method-return-type
 -Wno-variadic-macros -Wno-strict-prototypes -Wno-availability
 -Wno-nullability-completeness' --prefix=/Users/gerd/.local'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-make-separator-line-for-ttys-not-supporting-unde.patch --]
[-- Type: text/patch, Size: 1188 bytes --]

From d481da70010eab163d12f770ed11f8fef171406a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <gerd@gnu.org>
Date: Sun, 22 Dec 2024 08:35:40 +0100
Subject: [PATCH] Fix make-separator-line for ttys not supporting underline

* lisp/simple.el (make-separator-line): Use window-max-chars-per-line
instead of window-width.
---
 lisp/simple.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index cb3b12d4402..bd8466a5b1a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -745,8 +745,9 @@ make-separator-line
           (concat (propertize (make-string length ?\s) 'face 'separator-line)
                   "\n")
         (propertize "\n" 'face '(:inherit separator-line :extend t)))
-    ;; In terminals (that don't support underline), use a line of dashes.
-    (concat (propertize (make-string (or length (1- (window-width))) ?-)
+    ;; In terminals that don't support underline, use a line of dashes.
+    (concat (propertize (make-string (or length
+                                         (1- (window-max-chars-per-line))) ?-)
                         'face 'separator-line)
             "\n")))
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline
  2024-12-22  7:43 bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline Gerd Möllmann
@ 2024-12-22  8:22 ` Eli Zaretskii
  2024-12-22  8:49   ` Gerd Möllmann
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2024-12-22  8:22 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 75020

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Sun, 22 Dec 2024 08:43:14 +0100
> 
> To reproduce, emacs -nw -Q on a terminal not supporting underlining (in
> my case Terminal.app on macOS).
> 
> - M-x display-line-number-mode RET
> - Eval (insert (amke-separator-line))
> 
> => the separator line is too long
> 
> Attached patch fixes that.

Thanks.  But I'm not sure this is for make-separator-line to decide.
For example, after applying the patch, using this recipe:

  M-: (insert (make-separator-line)) RET
  M-x display-line-number-mode RET

we will again get a too-long separator line.  And with this recipe:

  M-x display-line-number-mode RET
  M-: (insert (make-separator-line)) RET
  M-x display-line-number-mode RET

we will get a too-short separator line.

So arguably, in these special cases, the caller should pass the
required length as the optional argument, because only the caller
knows the context in which the function is called and the purpose for
which the separator will be used.  Which would mean the default of
using window-width is correct.

Does this make sense?





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline
  2024-12-22  8:22 ` Eli Zaretskii
@ 2024-12-22  8:49   ` Gerd Möllmann
  2024-12-22 12:20     ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Gerd Möllmann @ 2024-12-22  8:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 75020

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Sun, 22 Dec 2024 08:43:14 +0100
>> 
>> To reproduce, emacs -nw -Q on a terminal not supporting underlining (in
>> my case Terminal.app on macOS).
>> 
>> - M-x display-line-number-mode RET
>> - Eval (insert (amke-separator-line))
>> 
>> => the separator line is too long
>> 
>> Attached patch fixes that.
>
> Thanks.  But I'm not sure this is for make-separator-line to decide.
> For example, after applying the patch, using this recipe:
>
>   M-: (insert (make-separator-line)) RET
>   M-x display-line-number-mode RET
>
> we will again get a too-long separator line.  And with this recipe:
>
>   M-x display-line-number-mode RET
>   M-: (insert (make-separator-line)) RET
>   M-x display-line-number-mode RET
>
> we will get a too-short separator line.
>
> So arguably, in these special cases, the caller should pass the
> required length as the optional argument, because only the caller
> knows the context in which the function is called and the purpose for
> which the separator will be used.  Which would mean the default of
> using window-width is correct.
>
> Does this make sense?

Yes, makes sense.

I noticed this too now with C-h f context-menu-mode, for example.
If the separator line size depends on the window which it currently
does, one gets different results.

And when the help buffer is shown in a different window, and in my case
to the left or right, it's almost always too long and wraps to 2 or 3
lines. It looks pretty weird.

But whatever, I'll close the bug in a minute. Thanks!






^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline
  2024-12-22  8:49   ` Gerd Möllmann
@ 2024-12-22 12:20     ` Eli Zaretskii
  2024-12-22 13:19       ` Gerd Möllmann
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2024-12-22 12:20 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 75020

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 75020@debbugs.gnu.org
> Date: Sun, 22 Dec 2024 09:49:34 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks.  But I'm not sure this is for make-separator-line to decide.
> > For example, after applying the patch, using this recipe:
> >
> >   M-: (insert (make-separator-line)) RET
> >   M-x display-line-number-mode RET
> >
> > we will again get a too-long separator line.  And with this recipe:
> >
> >   M-x display-line-number-mode RET
> >   M-: (insert (make-separator-line)) RET
> >   M-x display-line-number-mode RET
> >
> > we will get a too-short separator line.
> >
> > So arguably, in these special cases, the caller should pass the
> > required length as the optional argument, because only the caller
> > knows the context in which the function is called and the purpose for
> > which the separator will be used.  Which would mean the default of
> > using window-width is correct.
> >
> > Does this make sense?
> 
> Yes, makes sense.
> 
> I noticed this too now with C-h f context-menu-mode, for example.
> If the separator line size depends on the window which it currently
> does, one gets different results.
> 
> And when the help buffer is shown in a different window, and in my case
> to the left or right, it's almost always too long and wraps to 2 or 3
> lines. It looks pretty weird.

So I think we need to have bug reports for those applications where
this happens, in particular in C-h.  That's where this should be
fixed.

> But whatever, I'll close the bug in a minute. Thanks!

Thanks.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline
  2024-12-22 12:20     ` Eli Zaretskii
@ 2024-12-22 13:19       ` Gerd Möllmann
  0 siblings, 0 replies; 5+ messages in thread
From: Gerd Möllmann @ 2024-12-22 13:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 75020

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: 75020@debbugs.gnu.org
>> Date: Sun, 22 Dec 2024 09:49:34 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Thanks.  But I'm not sure this is for make-separator-line to decide.
>> > For example, after applying the patch, using this recipe:
>> >
>> >   M-: (insert (make-separator-line)) RET
>> >   M-x display-line-number-mode RET
>> >
>> > we will again get a too-long separator line.  And with this recipe:
>> >
>> >   M-x display-line-number-mode RET
>> >   M-: (insert (make-separator-line)) RET
>> >   M-x display-line-number-mode RET
>> >
>> > we will get a too-short separator line.
>> >
>> > So arguably, in these special cases, the caller should pass the
>> > required length as the optional argument, because only the caller
>> > knows the context in which the function is called and the purpose for
>> > which the separator will be used.  Which would mean the default of
>> > using window-width is correct.
>> >
>> > Does this make sense?
>> 
>> Yes, makes sense.
>> 
>> I noticed this too now with C-h f context-menu-mode, for example.
>> If the separator line size depends on the window which it currently
>> does, one gets different results.
>> 
>> And when the help buffer is shown in a different window, and in my case
>> to the left or right, it's almost always too long and wraps to 2 or 3
>> lines. It looks pretty weird.
>
> So I think we need to have bug reports for those applications where
> this happens, in particular in C-h.  That's where this should be
> fixed.

Actually, something else was also going on: bug#75024. Now I don't
have a terminal anymore where the dashes are used.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-12-22 13:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-22  7:43 bug#75020: [PATCH] Fix make-separator-line for ttys not supporting underline Gerd Möllmann
2024-12-22  8:22 ` Eli Zaretskii
2024-12-22  8:49   ` Gerd Möllmann
2024-12-22 12:20     ` Eli Zaretskii
2024-12-22 13:19       ` Gerd Möllmann

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.