bug#24651: 25.1; frame-inherited-parameters only partially works

bug#24651: 25.1; frame-inherited-parameters only partially works

[Colin Fraizer]

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

To: bug-gnu-emacs@gnu.org
Subject: 25.1; frame-inherited-parameters only partially works
--text follows this line--

    $ emacs -Q
    M-: (setq frame-inherited-parameters '(height width top left)) <RET>
    M-: (defun my-debug-fcn ()
          (interactive)
          (kill-new (prin1-to-string (mapcar (lambda (f) (cons f (mapcar
                                                                  (lambda
(elt) (cons elt (cons (frame-parameter f elt) nil)))

frame-inherited-parameters))) (frame-list))))) <RET>
    M-x my-debug-fcn <RET>
    ((#<frame  *Minibuf-1* 0x10484f5a0> (height 79) (width 131) (top 23)
(left 962)))

    C-x 5 2
    M-x my-debug-fcn <RET>
    ((#<frame save-me-frames.el 0x10837e638> (height 36) (width 131) (top
23) (left 962))
    (#<frame  *Minibuf-1* 0x10484f5a0> (height 79) (width 131) (top 23)
(left 962)))

The corresponding values of those four frame parameters should be
the same for the new frame and the old, but the height is incorrectly
set to 36.

I am running on MacOS Sierra.  (Details below.)

Best regards,
--Colin Fraizer
colin.fraizer@gmail.com




In GNU Emacs 25.1.1 (x86_64-apple-darwin16.0.0, NS appkit-1504.00 Version
10.12 (Build 16A323))
 of 2016-09-28 built on demwa.local
Windowing system distributor 'Apple', version 10.3.1504
Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/25.1/share/info/emacs
 --prefix=/usr/local/Cellar/emacs/25.1 --without-x --with-xml2
 --with-dbus --with-gnutls --with-imagemagick --with-modules --with-rsvg
 --without-pop --with-ns --disable-ns-self-contained'

Configured features:
JPEG RSVG IMAGEMAGICK DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS NS MODULES

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-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
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
"((#<frame save-me-frames.el 0x10837e638> (height 36) (width 131) (top 23)
(left 962)) (#<frame  *Minibuf-1* 0x10484f5a0> (height 79) (width 131) (top
23) (left 962)))"
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel ns-win ucs-normalize term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame 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 charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 196668 7450)
 (symbols 48 19638 0)
 (miscs 40 66 222)
 (strings 32 15072 5822)
 (string-bytes 1 434856)
 (vectors 16 32974)
 (vector-slots 8 653773 5373)
 (floats 8 166 326)
 (intervals 56 234 16)
 (buffers 976 20))

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

bug#24651: 25.1; frame-inherited-parameters only partially works

[Lars Ingebrigtsen]

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

Colin Fraizer <colin.fraizer@gmail.com> writes:

>     $ emacs -Q
>     M-: (setq frame-inherited-parameters '(height width top left)) <RET>
>     M-: (defun my-debug-fcn ()
>           (interactive)
>           (kill-new (prin1-to-string (mapcar (lambda (f) (cons f (mapcar
>                                                                   (lambda
> (elt) (cons elt (cons (frame-parameter f elt) nil)))
>
> frame-inherited-parameters))) (frame-list))))) <RET>

(This bug report unfortunately got no response at the time.)

The test case is a bit convoluted, but it's basically:

emacs -Q
(setq frame-inherited-parameters '(height width top left))
Make the frame shorter than the default, for instance 20 lines high.
C-x 5 2

I now get a frame that is, indeed, 20 lines high -- but not displaying a
mode line or a minibuffer.  The following:

(defun my-debug-fcn ()
  (mapcar (lambda (f)
	    (cons f (mapcar
                     (lambda
		       (elt) (cons elt (cons (frame-parameter f elt) nil)))
		     frame-inherited-parameters)))
	  (frame-list)))

evaluates to:

((#<frame *Messages* 0x56007c651568> (height 36) (width 80) (top 66) (left 1948)) (#<frame  *Minibuf-1* 0x56007bfd6390> (height 20) (width 80) (top 66) (left 1958)))

So the first frame is 20 lines, and the new frame is 20 lines, but
`height' is (the default in "emacs -Q") 36 lines "internally", somehow.
Here's what the frame looks like:


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

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


If I then resize the frame, then the mode line and minibuffer appears:

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

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



So it seems like something is going wrong when doing the inherited
height parameter -- it's not initialising everything the correct way.

Anybody got an insight here?

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

bug#24651: 25.1; frame-inherited-parameters only partially works

[martin rudalics]

 > So it seems like something is going wrong when doing the inherited
 > height parameter -- it's not initialising everything the correct way.
 >
 > Anybody got an insight here?

The implementation is overly optimistic.  Doing, for example,

     (when frame-inherited-parameters
       (sit-for 0.5)
       (dolist (param frame-inherited-parameters)
         (unless (assq param parameters)
           (let ((val (frame-parameter oldframe param)))
             (when val (set-frame-parameter frame param val))))))

works here.  I see two ways to fix this.  Add such a timeout or, more
cleanly, merge the inherited parameters into 'params' _before_ calling
`frame-creation-function'.  As a rule, never fiddle with frame sizes or
positions before the WM got back to us with the new frame.

martin

bug#24651: 25.1; frame-inherited-parameters only partially works

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> works here.  I see two ways to fix this.  Add such a timeout or, more
> cleanly, merge the inherited parameters into 'params' _before_ calling
> `frame-creation-function'.

Without looking at the code, I think the latter sounds better.

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

bug#24651: 25.1; frame-inherited-parameters only partially works

[martin rudalics]

>> works here.  I see two ways to fix this.  Add such a timeout or, more
>> cleanly, merge the inherited parameters into 'params' _before_ calling
>> `frame-creation-function'.
>
> Without looking at the code, I think the latter sounds better.

Let's see what Stefan says.

martin

bug#24651: 25.1; frame-inherited-parameters only partially works

[Stefan Monnier]

>> works here.  I see two ways to fix this.  Add such a timeout or, more
>> cleanly, merge the inherited parameters into 'params' _before_ calling
>> `frame-creation-function'.
> Without looking at the code, I think the latter sounds better.

Clearly, a `sit-for` is undesirable, so if we can avoid it we should.
Setting the params before creating the frame also has the advantage of
avoiding a visible resize.


        Stefan

bug#24651: 25.1; frame-inherited-parameters only partially works

[martin rudalics]

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

 > Clearly, a `sit-for` is undesirable, so if we can avoid it we should.
 > Setting the params before creating the frame also has the advantage of
 > avoiding a visible resize.

And no 100% untested patch?  The attached should do it but I think has
the some conceptual flaw as the old behavior: When a val is nil, it
won't override a default parameter whose value is non-nil.  But I never
figured out what inherited parameters are useful for in the first place
so maybe this is of no importance.

martin

[-- Attachment #2: frame-inherited-parameters.diff --]
[-- Type: text/x-patch, Size: 1478 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 772ba3d8c4..948c782328 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -864,7 +864,6 @@ make-frame
                   (error "Don't know how to interpret display %S"
                          display)))
              (t window-system)))
-	 (oldframe (selected-frame))
 	 (params parameters)
 	 frame child-frame)

@@ -882,8 +881,12 @@ make-frame
     (dolist (p default-frame-alist)
       (unless (assq (car p) params)
 	(push p params)))
-
-;;     (setq frame-size-history '(1000))
+    ;; Add parameters from `frame-inherited-parameters' unless they are
+    ;; overridden by explicit parameters.
+    (dolist (param frame-inherited-parameters)
+      (unless (assq param parameters)
+        (let ((val (frame-parameter nil param)))
+          (when val (push (cons param val) params)))))

     (when (eq (cdr (or (assq 'minibuffer params) '(minibuffer . t)))
               'child-frame)
@@ -916,12 +919,6 @@ make-frame
          frame 'minibuffer (frame-root-window child-frame))))

     (normal-erase-is-backspace-setup-frame frame)
-    ;; Inherit original frame's parameters unless they are overridden
-    ;; by explicit parameters.
-    (dolist (param frame-inherited-parameters)
-      (unless (assq param parameters)
-        (let ((val (frame-parameter oldframe param)))
-          (when val (set-frame-parameter frame param val)))))

     (when (numberp (car frame-size-history))
       (setq frame-size-history

bug#24651: 25.1; frame-inherited-parameters only partially works

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> And no 100% untested patch?  The attached should do it but I think has
> the some conceptual flaw as the old behavior: When a val is nil, it
> won't override a default parameter whose value is non-nil.  But I never
> figured out what inherited parameters are useful for in the first place
> so maybe this is of no importance.

Since it's, er, "feature compatible" (even if conceptually odd), then it
looks good to me.

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

bug#24651: 25.1; frame-inherited-parameters only partially works

[Stefan Monnier]

>> And no 100% untested patch?  The attached should do it but I think has
>> the some conceptual flaw as the old behavior: When a val is nil, it
>> won't override a default parameter whose value is non-nil.  But I never
>> figured out what inherited parameters are useful for in the first place
>> so maybe this is of no importance.

They were introduced so as to associate specific frames with specific emacsclients.

> Since it's, er, "feature compatible" (even if conceptually odd), then it
> looks good to me.

Agreed,


        Stefan

bug#24651: 25.1; frame-inherited-parameters only partially works

[martin rudalics]

 >>> When a val is nil, it
 >>> won't override a default parameter whose value is non-nil.  But I never
 >>> figured out what inherited parameters are useful for in the first place
 >>> so maybe this is of no importance.
 >
 > They were introduced so as to associate specific frames with specific emacsclients.

And there a non-nil parameter in 'default-frame-alist' is supposed to
override a nil parameter in 'frame-inherited-parameters'.  Such that
with emacs -Q setting

(setq default-frame-alist '((alpha . 50)))
(setq frame-inherited-parameters '(alpha))

in *scratch*, C-x 5 2 will produce a semi-transparent frame.

martin

bug#24651: 25.1; frame-inherited-parameters only partially works

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Since it's, er, "feature compatible" (even if conceptually odd), then it
>> looks good to me.
>
> Agreed,

And I've now tested Martin's patch, and it fixes the reported problem,
so I've now pushed it to Emacs 29.

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