Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Robert Pluim]

juri@jurta.org (Juri Linkov) writes:

> branch: master
> commit 57d2f24005a2e553fa8225eaff8465e5fad06d46
> Author: Juri Linkov <juri@linkov.net>
> Commit: Juri Linkov <juri@linkov.net>
>
>     * lisp/frame.el (make-frame-on-monitor): New command.  (Bug#34516)
>     
>     (make-frame-on-display): Add completion on available display names.

Iʼve lost track: did we still want to fake monitor names on recent
versions of macOS where the existing name retrieval methods donʼt work
anymore?

Robert

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Tue, 26 Feb 2019 10:12:26 +0100
> Cc: Juri Linkov <juri@linkov.net>
> 
> juri@jurta.org (Juri Linkov) writes:
> 
> > branch: master
> > commit 57d2f24005a2e553fa8225eaff8465e5fad06d46
> > Author: Juri Linkov <juri@linkov.net>
> > Commit: Juri Linkov <juri@linkov.net>
> >
> >     * lisp/frame.el (make-frame-on-monitor): New command.  (Bug#34516)
> >     
> >     (make-frame-on-display): Add completion on available display names.
> 
> Iʼve lost track: did we still want to fake monitor names on recent
> versions of macOS where the existing name retrieval methods donʼt work
> anymore?

In any case, this new command needs to be documented in the user
manual.  Please don't leave NEWS as the only documentation.

Thanks.

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Juri Linkov]

>> Iʼve lost track: did we still want to fake monitor names on recent
>> versions of macOS where the existing name retrieval methods donʼt work
>> anymore?

Yes, please add fake monitor names to support this command on macOS too.

> In any case, this new command needs to be documented in the user
> manual.  Please don't leave NEWS as the only documentation.

Now it's documented in the user manual.

But I'm not sure about the menu.  Currently the File menu contains:

  "New Frame on Display..."

but the File menu already is so long that I think a new menu item like

  "New Frame on Monitor..."

won't fit into it.

For the same reason I haven't added a menu item for ‘vc-root-version-diff’
to another overly long menu ‘vc-menu-map’.

BTW, a related question about the menu: shouldn't the menu item
"Search Files (Grep)..." run a more user-friendly command ‘rgrep’
instead of ‘grep’?

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Robert Pluim]

Juri Linkov <juri@linkov.net> writes:

>>> Iʼve lost track: did we still want to fake monitor names on recent
>>> versions of macOS where the existing name retrieval methods donʼt work
>>> anymore?
>
> Yes, please add fake monitor names to support this command on macOS too.
>

I can do that. I have a small problem with 'make-frame-on-monitor':
the resulting frame is too tall (itʼs correctly
positioned though). Perhaps toolbars etc need to be
accounted for?

(display-monitor-attributes-list) =>
(((name . "DISPLAY1") (geometry 0 0
1440 900) (workarea 0 23 1440 877) (mm-size 285 179) (frames #<frame
*scratch* 0x7faffa066230>) (source . "NS")))

The frame resulting from 'make-frame-on-monitor' has (frame-geometry) =>
((outer-position 0 . 23) (outer-size 1475 . 868)
(external-border-size 0 . 0) (title-bar-size 0 . 22)
(menu-bar-external) (menu-bar-size 0 . 0) (tool-bar-external . t)
(tool-bar-position . top) (tool-bar-size 1471 . 35)
(internal-border-width . 2))

If I maximize that frame so itʼs the correct height, (frame-geometry)
=>

((outer-position 0 . 23) (outer-size 1475 . 842) (external-border-size 0 . 0) (title-bar-size 0 . 22) (menu-bar-external) (menu-bar-size 0 . 0) (tool-bar-external . t) (tool-bar-position . top) (tool-bar-size 1471 . 35) (internal-border-width . 2))

(Iʼll note that both outer-size and tool-bar-size are wider than the
actual display, so perhaps this is an issue in the macOS code geometry
code).

Robert

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Robert Pluim]

Robert Pluim <rpluim@gmail.com> writes:

> I can do that. I have a small problem with 'make-frame-on-monitor':
> the resulting frame is too tall (itʼs correctly
> positioned though). Perhaps toolbars etc need to be
> accounted for?
>
> (Iʼll note that both outer-size and tool-bar-size are wider than the
> actual display, so perhaps this is an issue in the macOS code geometry
> code).

Thereʼs no issue on macOS: there was a scrollbar off to the right that
I hadn't realised was off-screen.

If Iʼve understood correctly, you want to create a frame that covers
the entire workarea of the chosen monitor. Would it not work to
specify the x,y of the top corner plus (fullscreen . maximized) rather
than specifying the width and height explicitly? That will do the
toolbar/titlebar/scrollbar subtractions for you (I haven't tested this).

Robert

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Robert Pluim <rpluim@gmail.com>,  emacs-devel@gnu.org
> Date: Wed, 27 Feb 2019 22:59:03 +0200
> 
> Now it's documented in the user manual.

Thanks.

> But I'm not sure about the menu.  Currently the File menu contains:
> 
>   "New Frame on Display..."
> 
> but the File menu already is so long that I think a new menu item like
> 
>   "New Frame on Monitor..."
> 
> won't fit into it.
> 
> For the same reason I haven't added a menu item for ‘vc-root-version-diff’
> to another overly long menu ‘vc-menu-map’.

I don't have problems with that.

> BTW, a related question about the menu: shouldn't the menu item
> "Search Files (Grep)..." run a more user-friendly command ‘rgrep’
> instead of ‘grep’?

That's an entirely different bikeshed, so feel free to start a new
thread for it, if you want.

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Juri Linkov]

> If Iʼve understood correctly, you want to create a frame that covers
> the entire workarea of the chosen monitor. Would it not work to
> specify the x,y of the top corner plus (fullscreen . maximized) rather
> than specifying the width and height explicitly? That will do the
> toolbar/titlebar/scrollbar subtractions for you (I haven't tested this).

I already have this for personal customization in ~/.emacs:

  (add-hook 'after-make-frame-functions 'toggle-frame-maximized)
  (advice-add 'make-frame-on-monitor :around
              (lambda (orig-fun monitor &optional display parameters)
                (funcall orig-fun monitor display '((undecorated . t))))
              '((name . make-frame-on-monitor-undecorated)))

But my intention was to implement a general command that would make
a new frame according to the physical monitor attributes returned by
display-monitor-attributes-list.

I have no idea why display-monitor-attributes-list returns
workarea that doesn't fit into the created frame on macOS.

While testing it without customization in GDK on GNU/Linux,
I see that the created frames cover the whole physical monitor,
including toolbar/titlebar/scrollbar.

Maybe there is still an issue in ns-display-monitor-attributes-list?

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Robert Pluim]

Juri Linkov <juri@linkov.net> writes:

>> If Iʼve understood correctly, you want to create a frame that covers
>> the entire workarea of the chosen monitor. Would it not work to
>> specify the x,y of the top corner plus (fullscreen . maximized) rather
>> than specifying the width and height explicitly? That will do the
>> toolbar/titlebar/scrollbar subtractions for you (I haven't tested this).

Now that Iʼve tested this, it works for my primary display, but not
the secondary, which is because the secondary is to my left,
and ns-parse-geometry has an issue with negative offsets.

In any case, I think using x-parse-geometry is not necessary. The
workarea already contains the relevant coördinates, and you can just
pass (fullscreen . maximized) to get the right size. Sample patch
below (which Iʼve tested only on macOS, not under X).

> I already have this for personal customization in ~/.emacs:
>
>   (add-hook 'after-make-frame-functions 'toggle-frame-maximized)
>   (advice-add 'make-frame-on-monitor :around
>               (lambda (orig-fun monitor &optional display parameters)
>                 (funcall orig-fun monitor display '((undecorated . t))))
>               '((name . make-frame-on-monitor-undecorated)))
>
> But my intention was to implement a general command that would make
> a new frame according to the physical monitor attributes returned by
> display-monitor-attributes-list.
>
> I have no idea why display-monitor-attributes-list returns
> workarea that doesn't fit into the created frame on macOS.

It does that because thatʼs what itʼs supposed to do, itʼs describing
the characteristics of the monitor. The issue is with make-frame. From
(elisp) Frame Size:

   The text size of any frame can be set and retrieved with the help of
the ‘height’ and ‘width’ frame parameters (*note Size Parameters::).

Note it says 'text size', which means you have to account for
scrollbars, toolbars, etc.

> While testing it without customization in GDK on GNU/Linux,
> I see that the created frames cover the whole physical monitor,
> including toolbar/titlebar/scrollbar.

That I suspect is window-manager dependent: there are window managers
that won't let you create a frame bigger than the monitor, or maybe
x-parse-geometry is performing clipping.

> Maybe there is still an issue in ns-display-monitor-attributes-list?

Not that I can see.

Sample implementation using (fullscreen . maximized):

diff --git a/lisp/frame.el b/lisp/frame.el
index 8a4a0b6639..f8e9a1ff1a 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -666,19 +666,19 @@ make-frame-on-monitor
                                    (when (equal (cdr (assq 'name a)) monitor)
                                      (cdr (assq 'workarea a))))
                                  (display-monitor-attributes-list display)))))
-         (frame-geometry
-          (when monitor-geometry
-            (x-parse-geometry (format "%dx%d+%d+%d"
-                                      (nth 2 monitor-geometry)
-                                      (nth 3 monitor-geometry)
-                                      (nth 0 monitor-geometry)
-                                      (nth 1 monitor-geometry)))))
          (frame-geometry-in-pixels
-          (when frame-geometry
-            `((top . ,(cdr (assq 'top frame-geometry)))
-              (left . ,(cdr (assq 'left frame-geometry)))
-              (height . (text-pixels . ,(cdr (assq 'height frame-geometry))))
-              (width . (text-pixels . ,(cdr (assq 'width frame-geometry))))))))
+          (when monitor-geometry
+            (let* ((top (nth 1 monitor-geometry))
+                   (left (nth 0 monitor-geometry))
+                   (top (if (< top 0)
+                                 `(- ,(abs top))
+                               top))
+                   (left (if (< left 0)
+                             `(- ,(abs left))
+                           left)))
+              `((top . ,top)
+                (left . ,left)
+                (fullscreen . maximized))))))
     (make-frame (append frame-geometry-in-pixels parameters))))

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Juri Linkov]

> In any case, I think using x-parse-geometry is not necessary. The
> workarea already contains the relevant coördinates, and you can just
> pass (fullscreen . maximized) to get the right size. Sample patch
> below (which Iʼve tested only on macOS, not under X).

We should not use (fullscreen . maximized) because this is not what
make-frame does by default.  I see that in 'emacs -Q', 'C-x 5 2' makes
a new frame with text sizes 80x35 in the center of the current monitor.

We should do the same also in case when a new frame is created on
another monitor.  So maybe just omit the attributes 'width' and
'height', without using 'fullscreen'.  Any other attributes are
a matter of personal customization.

RE: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Drew Adams]

> > In any case, I think using x-parse-geometry is not necessary. The
> > workarea already contains the relevant coördinates, and you can just
> > pass (fullscreen . maximized) to get the right size. Sample patch
> > below (which Iʼve tested only on macOS, not under X).
> 
> We should not use (fullscreen . maximized) because this is not what
> make-frame does by default.  I see that in 'emacs -Q', 'C-x 5 2' makes
> a new frame with text sizes 80x35 in the center of the current monitor.
> 
> We should do the same also in case when a new frame is created on
> another monitor.  So maybe just omit the attributes 'width' and
> 'height', without using 'fullscreen'.  Any other attributes are
> a matter of personal customization.

As an aside and FWIW, I think `C-x 5 2' should, by
default, make a new frame that is a clone of the
selected one (same parameters).  I proposed this in
bug #34715.

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Juri Linkov]

> As an aside and FWIW, I think `C-x 5 2' should, by
> default, make a new frame that is a clone of the
> selected one (same parameters).  I proposed this in
> bug #34715.

`clone-frame' would be a useful command since it will also
maximize the cloned frame if the original was maximized.

But why rebind `C-x 5 2' to it?  Like the following
existing key binding:

  `C-x 4 c' - clone-indirect-buffer-other-window

shouldn't the new command be bound to a similar key sequence:

  `C-x 5 c' - clone-frame

RE: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Drew Adams]

> > As an aside and FWIW, I think `C-x 5 2' should, by
> > default, make a new frame that is a clone of the
> > selected one (same parameters).  I proposed this in
> > bug #34715.
> 
> `clone-frame' would be a useful command since it will also
> maximize the cloned frame if the original was maximized.
> 
> But why rebind `C-x 5 2' to it?

Because I found I was pretty much always using
`make-frame-command' to open the current buffer
in another frame.  For my use it was thus kind of
a frame-duplication anyway, but without everything
about the frame getting duplicated.

See also below, for more "why". 

> Like the following existing key binding:
>   `C-x 4 c' - clone-indirect-buffer-other-window
> shouldn't the new command be bound to a similar key sequence:
>   `C-x 5 c' - clone-frame

Obviously such choices are judgment calls and subject
to opinion.

Normally I'd argue not to change the default behavior
(of `C-x 5 2'), for backward compatibility.  But in
this case I'd argue to change it, and keep the current
behavior on `C-u'.

But as I said, that's maybe partly because I typically
do not have multiple windows in a frame where I use
`C-x 5 2'.

To me it's natural to have a prefix arg change
between cloning and just doing `make-frame-command'.
Just which of those behaviors should be the default
behavior is a separate question.  I don't insist
that the default behavior must change.  But I do
think that the natural place for the clone behavior
is on `C-x 5 2', not `C-x 5 c'.

Yes, in some ways you can say this is similar to
`clone-indirect-buffer-other-window.  But it's not
really very similar.  It's not about indirect
buffers, and it's not about cloning a buffer.  It's
about showing a buffer in another place (frame).

FWIW, the code I actually use is a bit more involved,
as follows:

1. Cloning the frame to keep the same parameters,
particularly the same size, is handy.  But
giving the new frame the exact same position (as
well as size) puts it right on top of the frame
that's cloned.  I find it handier to position it
a bit offset.  That makes it more apparent that
the frame was cloned, and gives me instant access
to both frames.

Someone else might prefer the exactly-on-top
behavior.  Or someone might prefer a different
offset amount or direction.  Or someone might
prefer to always use some particular set of
frame parameters for cloning.

2. The code I use always selects the new frame
(except when `C-u' is used, to just invoke
`make-frame-command').  I find that this is
what I want nearly always.  And this behavior
is what `clone-indirect-buffer-other-window'
does: you generally clone a frame to work in
it, I think; you don't want to keep working
in the original (cloned) frame.

Below is the code I actually use, FWIW.  (The
binding of `fit-frame-inhibit-fitting-flag'
has an effect only if someone is using my
`fit-frame.el' code - you can ignore it.)

The user option lets you choose a particular
set of frame parameters to use.  Or you can
choose an offset and otherwise keep the same
parameters as the current frame.  Or you can
choose nil or zero offsets, to get exactly
the same frame parameters.

------8<------------

(defcustom clone-frame-parameters (cons 30 30)
  "Frame parameter settings that override those of the frame to clone.
The value can be an alist of frame parameters or a cons of two
integers, (LEFT-OFFSET . TOP-OFFSET).

The latter case sets parameters `left' and `top' of the new frame to
the `left' and `top' of the selected frame, offset by adding
LEFT-OFFSET and TOP-OFFSET to them, respectively."
  :type '(choice
          (cons :tag "Offset from current frame location"
                (integer :tag "Left")
                (integer :tag "Top"))
          (alist :tag "Parameters to augment/replace those of current frame"
                 :key-type (symbol :tag "Parameter")))
  :group 'Frame-Commands)

(defun clone-frame (&optional frame no-clone)
  "Make and select a new frame with the same parameters as FRAME.
With a prefix arg, don't clone - just call `make-frame-command'.
Return the new frame.

FRAME defaults to the selected frame.  The frame is created on the
same terminal as FRAME.  If the terminal is a text-only terminal then
also select the new frame."
  (interactive "i\nP")
  (if no-clone
      (make-frame-command)
    (let* ((fit-frame-inhibit-fitting-flag  t)
           (clone-frame-parameters
            (if (and clone-frame-parameters
                     (not (consp (car clone-frame-parameters))))
                `((left
                   . ,(+ (car clone-frame-parameters)
                         (or (cdr (assq 'left
                                        (frame-parameters frame)))
                             0)))
                  (top
                   . ,(+ (cdr clone-frame-parameters)
                         (or (cdr (assq 'top
                                        (frame-parameters frame)))
                             0))))
              clone-frame-parameters))
           (default-frame-alist  (append clone-frame-parameters
                                         (frame-parameters frame)))
           (new-fr               (make-frame)))
      (select-frame new-fr)
      new-fr)))

Re: master 57d2f24: * lisp/frame.el (make-frame-on-monitor): New command. (Bug#34516)

[Robert Pluim]

Juri Linkov <juri@linkov.net> writes:

>> In any case, I think using x-parse-geometry is not necessary. The
>> workarea already contains the relevant coördinates, and you can just
>> pass (fullscreen . maximized) to get the right size. Sample patch
>> below (which Iʼve tested only on macOS, not under X).
>
> We should not use (fullscreen . maximized) because this is not what
> make-frame does by default.  I see that in 'emacs -Q', 'C-x 5 2' makes
> a new frame with text sizes 80x35 in the center of the current monitor.
>
> We should do the same also in case when a new frame is created on
> another monitor.  So maybe just omit the attributes 'width' and
> 'height', without using 'fullscreen'.  Any other attributes are
> a matter of personal customization.

I was about to suggest the same :-)

I donʼt have any particular preference for the positioning of the new
frame, top-left or center are both fine ('C-x 5 2' does top-left for
me here).

Robert