GNU bug report logs - #43295
26.1: calc-mode header line [UPDATED PATCH]

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43295 in the body.
You can then email your comments to 43295 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Emacs Bug Reporting <bug-gnu-emacs <at> gnu.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: 26.1: calc-mode header line [UPDATED PATCH]
Date: Wed, 9 Sep 2020 14:31:27 -0400

[Message part 1 (text/plain, inline)]

This is summary of related mail on emacs-devel (an updated patch is attached):

On 2020-08-31 14:44, Boruch Baum wrote:
> Back in version 21, emacs introduced a static 'header-line' that could
> be inserted at the top of any buffer. Calc mode is one emacs package
> that does not use it and could benefit from it, so the attached patch
> offers that feature. The main benefit is that the 'calc trail' buffer
> (what some greybeards from the mechanical age would remember as the
> 'tape reel') no longer has its title line scroll off the visible
> window. The patch also includes:
>
> 1) Width-sensitive text for the header line, so that it is readable for
>    very narrow windows, and scales to very wide windows.
>
> 2) Display of the 'calc trail' buffer when invoking calc from a frame
>    that is split vertically (C-x 3, M-x split-window-right).
>
> 3) My version of emacs includes a unicode character at 'C-x 8 <return>
>    POCKET CALCULATOR', that I did not include in the header line as the
>    mode's icon, but that could be done.
>
> The patch was diff'ed against the version of emacs that I have: the
> latest-and-greatest that debian is distributing ... v26.1


On 2020-09-07 14:01, Boruch Baum wrote:
> First, congratulations on assuming your new responsibilities.
>
> On 2020-09-07 17:00, Lars Ingebrigtsen wrote:
> > The patch doesn't apply to Emacs 28, so I've respun it (included below).
>
> Oops. I didn't think there would be a difference. I'm using emacs 26.1
> in debian and I didn't download the v28 calc.el
>
> > This is somewhat inscrutable, and is repeated twice (once for the calc
> > buffer and once for the trail buffer).
> >
> > It just centres whatever the string like "--- this ---", so it seems
> > like it should land in a single function for reuse.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[calc.patch (text/x-diff, attachment)]

[NEWS.patch (text/x-diff, attachment)]

Message #8 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Thu, 10 Sep 2020 23:45:47 +0200

Boruch Baum <boruch_baum <at> gmx.com> writes:

> This is summary of related mail on emacs-devel (an updated patch is
> attached):

Thanks; I applied it to Emacs 28, but it needed some changes because it
didn't apply cleanly (and it didn't set the header in the main calc
buffer?)  But it looks like it works now, at least, but you should
perhaps take a look at the result and see whether it looks like you
imagined...

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

Message #15 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Thu, 10 Sep 2020 19:40:59 -0400

[Message part 1 (text/plain, inline)]

On 2020-09-10 23:45, Lars Ingebrigtsen wrote:
> Thanks; I applied it to Emacs 28, but it needed some changes because it
> didn't apply cleanly (and it didn't set the header in the main calc
> buffer?)  But it looks like it works now, at least, but you should
> perhaps take a look at the result and see whether it looks like you
> imagined...

I see a few things. Here are some comments, with reference to a new
patch, attached, which is a diff based upon the savannah-git (my
calc28.el).

1) I notice that I had forgotten to remove two lines of coding notes to
   myself @lines ~1395. They can be removed.

2) You removed two lines I had @lines ~1428, and copied a modified
   version of them to @lines ~2008. I think the absence of the two lines
   ~1428 may be cause the problem that you mentioned.

3) I see an additional problem with your modification to the patch that
   you'll notice upon starting calc with a very narrow window. Your line
   ~2009 reads

                        (* 2 (/ (window-width) 3)) -3))

   which is code for the trail buffer, but it should be for the main
   buffer, like in my line ~1429

+                       (/ (* (window-width) 2) 3) 1)

3) If you modify ~2009, then that snippet might be redundant together
   with my snippet ~1428, but it shouldn't do any harm (but remove the
   comment line 'Added by Lars?').

4) You also did something that I welcome, but that was done at Eli
   Zaretskii's insistence, so you may want to co-ordinate with him about
   it. My very original patch looked like your final result @line ~1419,
   but Eli on-list insisted that was wrong on the basis of 'breaking backward
   compatibility', because in the old behavior, the trail buffer always
   had a title line inside the buffer even when calc-show-banner was
   NIL. My position was that's a bug. That part should really read (modifed
   from my original):

  (if calc-show-banner
    (calc--header-line "Emacs Calculator Trail" "Calc Trail"
                       (/ (window-width) 3) -3)
   (when (zerop (buffer-size))
     (let ((buffer-read-only nil))
       (insert (propertize "Emacs Calculator Trail\n" 'face 'italic))))))


--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[calc28.patch (text/x-diff, attachment)]

Message #18 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Fri, 11 Sep 2020 14:15:33 +0200

Boruch Baum <boruch_baum <at> gmx.com> writes:

> I see a few things. Here are some comments, with reference to a new
> patch, attached, which is a diff based upon the savannah-git (my
> calc28.el).
>
> 1) I notice that I had forgotten to remove two lines of coding notes to
>    myself @lines ~1395. They can be removed.

[...]

> 3) If you modify ~2009, then that snippet might be redundant together
>    with my snippet ~1428, but it shouldn't do any harm (but remove the
>    comment line 'Added by Lars?').

Could you send a new patch that has all the changes that you want that I
can just apply?  Adding discussion into the patch itself isn't very
helpful.  :-)

And please submit the patch in a way that it can just be applied.  This
patch can't be applied either, since you've changed the name of the file
to "calc28.el".

Thanks.

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

Message #21 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Fri, 11 Sep 2020 09:37:44 -0400

On 2020-09-11 14:15, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum <at> gmx.com> writes:
>
> Could you send a new patch that has all the changes that you want that I
> can just apply?  Adding discussion into the patch itself isn't very
> helpful.  :-)
>
> And please submit the patch in a way that it can just be applied.  This
> patch can't be applied either, since you've changed the name of the file
> to "calc28.el".

I can't know everything to include in the patch until you tell me what
your decisions are about the comments I made, most importantly whether
Eli's decision stands about the old trail buffer behavior, and whether
to include your addition @line ~2000 something.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

Message #24 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: larsi <at> gnus.org, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Fri, 11 Sep 2020 16:55:29 +0300

> Date: Fri, 11 Sep 2020 09:37:44 -0400
> From: Boruch Baum <boruch_baum <at> gmx.com>
> Cc: 43295 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> 
> I can't know everything to include in the patch until you tell me what
> your decisions are about the comments I made, most importantly whether
> Eli's decision stands about the old trail buffer behavior, and whether
> to include your addition @line ~2000 something.

I'd prefer to keep the old behavior, at least as an option, yes.

Thanks.

Message #27 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Sat, 12 Sep 2020 20:43:24 -0400

[Message part 1 (text/plain, inline)]

On 2020-09-11 14:15, Lars Ingebrigtsen wrote:
> Could you send a new patch that has all the changes that you want that I
> can just apply?  Adding discussion into the patch itself isn't very
> helpful.  :-)

In the attached patch:

1) the hunk @1421 restores the behavior that Eli wants

2) the hunk @2010 corrects the width calculation in that case

3) the hunk @2133 better handles the header line for the trail display
--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[calc-003.patch (text/x-diff, attachment)]

Message #30 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Sun, 13 Sep 2020 15:15:27 +0200

[Message part 1 (text/plain, inline)]

Boruch Baum <boruch_baum <at> gmx.com> writes:

> In the attached patch:
>
> 1) the hunk @1421 restores the behavior that Eli wants
>
> 2) the hunk @2010 corrects the width calculation in that case
>
> 3) the hunk @2133 better handles the header line for the trail display

This results in a trail buffer that looks like:

[Message part 2 (image/png, inline)]

[Message part 3 (text/plain, inline)]

Which surely can't be optimal?  If modified as follows, the duplicated
text disappears:

diff --git a/lisp/calc/calc.el b/lisp/calc/calc.el
index bf8b006d7c..62c0bea6d4 100644
--- a/lisp/calc/calc.el
+++ b/lisp/calc/calc.el
@@ -1396,8 +1396,6 @@ calc--header-line
         (let* ((len-long (length long))
                (len-short (length short))
                (fudge (or fudge 0))
-               ;; fudge for trail is: -3 (added to len-long)
-               ;; (width  ) for trail
                (factor (if (> width (+ len-long fudge)) len-long len-short))
                (size   (max (/ (- width factor) 2) 0))
                (fill (make-string size ?-))
@@ -1428,6 +1426,12 @@ calc-create-buffer
   (set-buffer (get-buffer-create "*Calculator*"))
   (or (derived-mode-p 'calc-mode)
       (calc-mode))
+  (when calc-show-banner
+    (calc--header-line "Emacs Calculator Mode" "Emacs Calc"
+                       (if calc-display-trail
+                           (/ (* (window-width) 2) 3)
+                         (window-width))
+                       1))
   (setq max-lisp-eval-depth (max max-lisp-eval-depth 1000))
   (when calc-always-load-extensions
     (require 'calc-ext))
@@ -2009,8 +2013,8 @@ calc-refresh
 	 (setq calc-any-selections nil)
 	 (erase-buffer)
          (when calc-show-banner
-           (calc--header-line  "Emacs Calculator Mode" "Emacs Calc"
-                       (* 2 (/ (window-width) 3)) -3))
+           (calc--header-line "Emacs Calculator Mode" "Emacs Calc"
+                              (window-width) 1))
 	 (while thing
 	   (goto-char (point-min))
 	   (insert (math-format-stack-value (car thing)) "\n")
@@ -2133,29 +2137,32 @@ calc-record
 (defun calc-trail-display (flag &optional no-refresh interactive)
   (interactive "P\ni\np")
   (let ((win (get-buffer-window (calc-trail-buffer))))
-    (if (setq calc-display-trail
-	      (not (if flag (memq flag '(nil 0)) win)))
-	(if (null win)
-	    (progn
-              (if calc-trail-window-hook
-                  (run-hooks 'calc-trail-window-hook)
-                (let ((w (split-window nil (/ (* (window-width) 2) 3) t)))
-                  (set-window-buffer w calc-trail-buffer)))
-              (calc-wrapper
-               (setq overlay-arrow-string calc-trail-overlay
-                     overlay-arrow-position calc-trail-pointer)
-               (or no-refresh
-                   (if interactive
-                       (calc-do-refresh)
-                     (calc-refresh))))))
-      (if win
-	  (progn
-	    (delete-window win)
-	    (calc-wrapper
-	     (or no-refresh
-		 (if interactive
-		     (calc-do-refresh)
-		   (calc-refresh))))))))
+    (cond
+     ((setq calc-display-trail
+	    (not (if flag (memq flag '(nil 0)) win)))
+      (when (null win)
+        (if calc-trail-window-hook
+            (run-hooks 'calc-trail-window-hook)
+          (setq win (split-window nil (/ (* (window-width) 2) 3) t))
+          (set-window-buffer win calc-trail-buffer))
+        (calc-wrapper
+         (setq overlay-arrow-string calc-trail-overlay
+               overlay-arrow-position calc-trail-pointer)
+         (or no-refresh
+             (if interactive
+                 (calc-do-refresh)
+               (calc-refresh)))))
+      (with-current-buffer calc-trail-buffer
+        (when calc-show-banner
+          (calc--header-line "Emacs Calculator Trail" "Calc Trail"
+                             (window-width win) -3))))
+     (win                               ; not calc-display-trail
+      (delete-window win)
+      (calc-wrapper
+       (or no-refresh
+	   (if interactive
+	       (calc-do-refresh)
+             (calc-refresh)))))))
   calc-trail-buffer)
 
 (defun calc-trail-here ()


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

Message #33 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Sun, 13 Sep 2020 10:54:11 -0400

On 2020-09-13 15:15, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum <at> gmx.com> writes:
>
> This results in a trail buffer that looks like:

Which is intentional. This is the 'prior behavior' that I mentioned Eli
didn't want to change so as not to 'break backward compatability'. It's
all in the message history on emacs-devel.

> Which surely can't be optimal?  If modified as follows, the duplicated
> text disappears:

I don't mind. My original submission had no duplicate text. Eli might
mind.

One possible benefit of Eli's position is that someone might want to
save the trail buffer to a file and expect the title string. Another
benefit would be if there exists third-party code written to parse the
buffer expecting to delete the first two lines.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

Message #36 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Sun, 13 Sep 2020 16:56:28 +0200

Boruch Baum <boruch_baum <at> gmx.com> writes:

> Which is intentional. This is the 'prior behavior' that I mentioned Eli
> didn't want to change so as not to 'break backward compatability'. It's
> all in the message history on emacs-devel.

I kinda doubt that that's the prior behaviour Eli had in mind.  My
powerful mind reading powers says that he probably meant that in the
"backward compatibility mode", there should be no headers, only text in
the buffer (like before).

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

Message #39 received at 43295 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: larsi <at> gnus.org, 43295 <at> debbugs.gnu.org
Subject: Re: bug#43295: 26.1: calc-mode header line [UPDATED PATCH]
Date: Sun, 13 Sep 2020 18:03:43 +0300

> Date: Sun, 13 Sep 2020 10:54:11 -0400
> From: Boruch Baum <boruch_baum <at> gmx.com>
> Cc: 43295 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> 
> On 2020-09-13 15:15, Lars Ingebrigtsen wrote:
> > Boruch Baum <boruch_baum <at> gmx.com> writes:
> >
> > This results in a trail buffer that looks like:
> 
> Which is intentional. This is the 'prior behavior' that I mentioned Eli
> didn't want to change so as not to 'break backward compatability'. It's
> all in the message history on emacs-devel.

To clarify, I think I wanted the prior behavior when the new one is
disabled.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.