* bug#74750: clone-frame and make-frame pixelwise issues @ 2024-12-09 15:51 Ship Mints 2024-12-10 12:27 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-09 15:51 UTC (permalink / raw) To: 74750 [-- Attachment #1: Type: text/plain, Size: 5696 bytes --] While trying to reconcile pixelwise frame sizing behaviors, I narrowed down two related issues. clone-frame does not correctly clone frames on a pixelwise basis. make-frame's text-pixels geometry support does not produce specified pixelwise geometry. This also impacts frameset-restore's ability to precisely reproduce pixelwise frame sizes. I consider these to be related as clone-frame's use of make-frame could be using text-pixels but if that doesn't work then pixelwise cloning won't work. I did read through the code base as best as I could but could not find the source of the text-pixels issue. The following reproducer, under -Q, shows the same results on 29.4 and 30.0.92. My main platform is NS and I also did some testing on GTK. GTK's issues seem a bit "messier" and I didn't spend any time trying to understand them in depth as I was more interested to know if GTK worked correctly or not, which it doesn't. (switch-to-buffer "*Messages*") (let ((target-text-width 1700) (target-text-height 1000) (native-width) (native-height) (msg (lambda (s frame) (message "%s text-width=%d (Δ%d) text-height=%d (Δ%d) native-width=%d (Δ%d) native-height %d (Δ%d)\n" s (frame-text-width frame) (- (frame-text-width frame) target-text-width) (frame-text-height frame) (- (frame-text-height frame) target-text-height) (frame-native-width frame) (- (frame-native-width frame) native-width) (frame-native-height frame) (- (frame-native-height frame) native-height))))) (set-frame-position nil 0 0) (set-frame-size nil target-text-width target-text-height 'pixelwise) (setq native-width (frame-native-width) native-height (frame-native-height)) (message "Targets: text-width=%d text-height=%d\n" target-text-width target-text-height) (funcall msg "orig" (selected-frame)) (message "clone-frame under frame-resize-pixelwise nil; expectation: use lines/columns geometry; outcome: met") (let ((frame-resize-pixelwise nil)) (let ((new-frame (clone-frame))) (funcall msg "new" new-frame) (delete-frame new-frame))) (message "clone-frame under frame-resize-pixelwise t; expectation: pixelwise geometry; outcome: unmet") (let ((frame-resize-pixelwise t)) (let ((new-frame (clone-frame))) (funcall msg "new" new-frame) (delete-frame new-frame))) (message "clone-frame followed by manual resize; expectation: pixelwise geometry; outcome: met (but two steps)") (let ((new-frame (clone-frame))) (set-frame-size new-frame target-text-width target-text-height 'pixelwise) (funcall msg "new" new-frame) (delete-frame new-frame)) (message "manual clone under frame-resize-pixelwise using text-pixels; expectation: pixelwise geometry; outcome: unmet") ;; code lifted from clone-frame ;; incorrect width offset seems to be equal to frame-scroll-bar-width (let* ((frame-resize-pixelwise t) (frame (selected-frame)) (no-windows nil) (windows (unless no-windows (window-state-get (frame-root-window frame)))) (default-frame-alist (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) (new-frame)) (when (and (display-graphic-p frame) frame-resize-pixelwise) (push (cons 'width (cons 'text-pixels (frame-text-width frame))) default-frame-alist) (push (cons 'height (cons 'text-pixels (frame-text-height frame))) default-frame-alist)) (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) (unless (display-graphic-p frame) (select-frame new-frame)) (funcall msg "new" new-frame) (delete-frame new-frame))) This is an implementation of clone-frame that uses text-pixels under make-frame. This depends on make-frame text-pixels being corrected. Happy to supply this as a patch should the discussion of these issues progress in that direction. (defun clone-frame (&optional frame no-windows pixelwise) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. When PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame is not text-only, clone the originating frame's pixel size. 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 (list (selected-frame) current-prefix-arg)) (let* ((frame (or frame (selected-frame))) (windows (unless no-windows (window-state-get (frame-root-window frame)))) (default-frame-alist (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) (new-frame)) (when (and (display-graphic-p frame) (or pixelwise frame-resize-pixelwise)) (push (cons 'width (cons 'text-pixels (frame-text-width frame))) default-frame-alist) (push (cons 'height (cons 'text-pixels (frame-text-height frame))) default-frame-alist)) (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) -Stephane [-- Attachment #2: Type: text/html, Size: 6564 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-09 15:51 bug#74750: clone-frame and make-frame pixelwise issues Ship Mints @ 2024-12-10 12:27 ` Eli Zaretskii 2024-12-10 15:56 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2024-12-10 12:27 UTC (permalink / raw) To: Ship Mints, martin rudalics; +Cc: 74750 > From: Ship Mints <shipmints@gmail.com> > Date: Mon, 9 Dec 2024 10:51:23 -0500 > > While trying to reconcile pixelwise frame sizing behaviors, I narrowed down > two related issues. > > clone-frame does not correctly clone frames on a pixelwise basis. > > make-frame's text-pixels geometry support does not produce specified > pixelwise geometry. This also impacts frameset-restore's ability to > precisely reproduce pixelwise frame sizes. > > I consider these to be related as clone-frame's use of make-frame could be > using text-pixels but if that doesn't work then pixelwise cloning won't > work. I did read through the code base as best as I could but could not > find the source of the text-pixels issue. > > The following reproducer, under -Q, shows the same results on 29.4 and > 30.0.92. My main platform is NS and I also did some testing on GTK. GTK's > issues seem a bit "messier" and I didn't spend any time trying to > understand them in depth as I was more interested to know if GTK worked > correctly or not, which it doesn't. > > (switch-to-buffer "*Messages*") > (let ((target-text-width 1700) > (target-text-height 1000) > (native-width) > (native-height) > (msg (lambda (s frame) > (message "%s text-width=%d (Δ%d) text-height=%d (Δ%d) > native-width=%d (Δ%d) native-height %d (Δ%d)\n" > s > (frame-text-width frame) (- (frame-text-width frame) > target-text-width) > (frame-text-height frame) (- (frame-text-height > frame) target-text-height) > (frame-native-width frame) (- (frame-native-width > frame) native-width) > (frame-native-height frame) (- (frame-native-height > frame) native-height))))) > (set-frame-position nil 0 0) > (set-frame-size nil target-text-width target-text-height 'pixelwise) > (setq native-width (frame-native-width) > native-height (frame-native-height)) > (message "Targets: text-width=%d text-height=%d\n" target-text-width > target-text-height) > (funcall msg "orig" (selected-frame)) > > (message "clone-frame under frame-resize-pixelwise nil; expectation: use > lines/columns geometry; outcome: met") > (let ((frame-resize-pixelwise nil)) > (let ((new-frame (clone-frame))) > (funcall msg "new" new-frame) > (delete-frame new-frame))) > > (message "clone-frame under frame-resize-pixelwise t; expectation: > pixelwise geometry; outcome: unmet") > (let ((frame-resize-pixelwise t)) > (let ((new-frame (clone-frame))) > (funcall msg "new" new-frame) > (delete-frame new-frame))) > > (message "clone-frame followed by manual resize; expectation: pixelwise > geometry; outcome: met (but two steps)") > (let ((new-frame (clone-frame))) > (set-frame-size new-frame target-text-width target-text-height > 'pixelwise) > (funcall msg "new" new-frame) > (delete-frame new-frame)) > > (message "manual clone under frame-resize-pixelwise using text-pixels; > expectation: pixelwise geometry; outcome: unmet") > ;; code lifted from clone-frame > ;; incorrect width offset seems to be equal to frame-scroll-bar-width > (let* ((frame-resize-pixelwise t) > (frame (selected-frame)) > (no-windows nil) > (windows (unless no-windows > (window-state-get (frame-root-window frame)))) > (default-frame-alist > (seq-remove (lambda (elem) > (memq (car elem) frame-internal-parameters)) > (frame-parameters frame))) > (new-frame)) > (when (and (display-graphic-p frame) frame-resize-pixelwise) > (push (cons 'width (cons 'text-pixels (frame-text-width frame))) > default-frame-alist) > (push (cons 'height (cons 'text-pixels (frame-text-height frame))) > default-frame-alist)) > (setq new-frame (make-frame)) > (when windows > (window-state-put windows (frame-root-window new-frame) 'safe)) > (unless (display-graphic-p frame) > (select-frame new-frame)) > (funcall msg "new" new-frame) > (delete-frame new-frame))) > > This is an implementation of clone-frame that uses text-pixels under > make-frame. This depends on make-frame text-pixels being corrected. Happy > to supply this as a patch should the discussion of these issues progress in > that direction. > > (defun clone-frame (&optional frame no-windows pixelwise) > "Make a new frame with the same parameters and windows as FRAME. > With a prefix arg NO-WINDOWS, don't clone the window configuration. When > PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame > is not text-only, clone the originating frame's pixel size. > > 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 (list (selected-frame) current-prefix-arg)) > (let* ((frame (or frame (selected-frame))) > (windows (unless no-windows > (window-state-get (frame-root-window frame)))) > (default-frame-alist > (seq-remove (lambda (elem) > (memq (car elem) frame-internal-parameters)) > (frame-parameters frame))) > (new-frame)) > (when (and (display-graphic-p frame) > (or pixelwise frame-resize-pixelwise)) > (push (cons 'width (cons 'text-pixels (frame-text-width frame))) > default-frame-alist) > (push (cons 'height (cons 'text-pixels (frame-text-height frame))) > default-frame-alist)) > (setq new-frame (make-frame)) > (when windows > (window-state-put windows (frame-root-window new-frame) 'safe)) > (unless (display-graphic-p frame) > (select-frame new-frame)) > new-frame)) Thanks. I hope Martin (CC'ed) will have some useful inputs. ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-10 12:27 ` Eli Zaretskii @ 2024-12-10 15:56 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-10 16:24 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-10 15:56 UTC (permalink / raw) To: Eli Zaretskii, Ship Mints; +Cc: 74750 >> clone-frame does not correctly clone frames on a pixelwise basis. >> >> make-frame's text-pixels geometry support does not produce specified >> pixelwise geometry. This also impacts frameset-restore's ability to >> precisely reproduce pixelwise frame sizes. This would be a bug in frameset.el. One problem I see is that 'frame-resize-pixelwise' must be set "very early" when restoring a session - at least _before_ the first time we send size hints to the window manager. >> I consider these to be related as clone-frame's use of make-frame could be >> using text-pixels but if that doesn't work then pixelwise cloning won't >> work. I did read through the code base as best as I could but could not >> find the source of the text-pixels issue. >> >> The following reproducer, under -Q, shows the same results on 29.4 and >> 30.0.92. My main platform is NS and I also did some testing on GTK. GTK's >> issues seem a bit "messier" and I didn't spend any time trying to >> understand them in depth as I was more interested to know if GTK worked >> correctly or not, which it doesn't. Your code binds 'frame-resize-pixelwise' temporarily. This cannot work reliably. That variable should never change in an Emacs session because its value affects the way we send size hint increments to the window manager. This is, admittedly, a design error that would have to be fixed as follows: - Implement a new frame parameter 'resize-pixelwise'. - Send size hints according to the value of this parameter. When the parameter is set, new size hints must be sent. Alternatively, we could send new size hints for all live frames whenever 'frame-resize-pixelwise' is changed. This would have to be done with a variable watcher. Still, let-binding this variable would confuse the hell out of our interactions with the window manager. When the scope of the let-binding is left, we would have to send size hints again. >> This is an implementation of clone-frame that uses text-pixels under >> make-frame. This depends on make-frame text-pixels being corrected. Happy >> to supply this as a patch should the discussion of these issues progress in >> that direction. >> >> (defun clone-frame (&optional frame no-windows pixelwise) What would the WM do in a situation where PIXELWISE is non-nil and 'frame-resize-pixelwise' is nil? martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-10 15:56 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-10 16:24 ` Ship Mints 2024-12-11 9:37 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-10 16:24 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 4482 bytes --] On Tue, Dec 10, 2024 at 10:56 AM martin rudalics <rudalics@gmx.at> wrote: > >> clone-frame does not correctly clone frames on a pixelwise basis. > >> > >> make-frame's text-pixels geometry support does not produce specified > >> pixelwise geometry. This also impacts frameset-restore's ability to > >> precisely reproduce pixelwise frame sizes. > > This would be a bug in frameset.el. One problem I see is that > 'frame-resize-pixelwise' must be set "very early" when restoring a > session - at least _before_ the first time we send size hints to the > window manager. > The goal is, I think, to achieve pixelwise harmony among clone, make, and restore. FWIW, my GUI sessions always run with frame-resize-pixelwise set to t and see these sizing issues consistently. My reproducer is intended to engender the conversation as an illustration. I have resorted to manual labor to set frame sizes in various places and including squirreling away frame pixel size in frameset-save just to manually reset it after restoration due to text-pixel reliability issues. Somewhere in the bowels of frame.c, et.al., is an issue where text-pixels behaves differently than set-frame-size which is reliable on NS where make-frame using text-pixels is incorrect by the vertical scroll bar width. GTK it's off, too, but I didn't analyze that further. > >> I consider these to be related as clone-frame's use of make-frame > could be > >> using text-pixels but if that doesn't work then pixelwise cloning won't > >> work. I did read through the code base as best as I could but could not > >> find the source of the text-pixels issue. > >> > >> The following reproducer, under -Q, shows the same results on 29.4 and > >> 30.0.92. My main platform is NS and I also did some testing on GTK. > GTK's > >> issues seem a bit "messier" and I didn't spend any time trying to > >> understand them in depth as I was more interested to know if GTK worked > >> correctly or not, which it doesn't. > > Your code binds 'frame-resize-pixelwise' temporarily. This cannot work > reliably. That variable should never change in an Emacs session because > its value affects the way we send size hint increments to the window > manager. > Again, this is just a reproducer for discussion and is intended to illustrate the issues, not to specifically discuss user habits for frame-resize-pixelwise across a GUI session. I think most people these days probably do have it set to t, even if it's become reflexive. This is, admittedly, a design error that would have to be fixed as > follows: > > - Implement a new frame parameter 'resize-pixelwise'. > I certainly prefer something more explicit such as this. - Send size hints according to the value of this parameter. When the > parameter is set, new size hints must be sent. > > Alternatively, we could send new size hints for all live frames whenever > 'frame-resize-pixelwise' is changed. This would have to be done with a > variable watcher. Still, let-binding this variable would confuse the > hell out of our interactions with the window manager. When the scope of > the let-binding is left, we would have to send size hints again. > No, please. >> This is an implementation of clone-frame that uses text-pixels under > >> make-frame. This depends on make-frame text-pixels being corrected. > Happy > >> to supply this as a patch should the discussion of these issues > progress in > >> that direction. > >> > >> (defun clone-frame (&optional frame no-windows pixelwise) > > What would the WM do in a situation where PIXELWISE is non-nil and > 'frame-resize-pixelwise' is nil? > The simple interim code (vs. the resize-pixelwise proposal) respects frame-resize-pixelwise as the user's preference. Same with being explicit by saying 'pixelwise, it's user intention. (when (and (display-graphic-p frame) (or pixelwise frame-resize-pixelwise)) FWIW, there are surely weird WM issues but at the very least, we're talking about the inner geometry text-width and text-height that Emacs controls and I stayed away from external geometry to start off at least seeing if we can correct for what Emacs controls completely. GTK is definitely worse than NS but at least in NS every user has the same experience as the WM options and behaviors are more constrained than X11-derived kind of free-for-all WMs. martin > [-- Attachment #2: Type: text/html, Size: 6900 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-10 16:24 ` Ship Mints @ 2024-12-11 9:37 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-11 22:41 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-11 9:37 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 496 bytes --] > Somewhere in the bowels > of frame.c, et.al., is an issue where text-pixels behaves differently than > set-frame-size which is reliable on NS where make-frame using text-pixels > is incorrect by the vertical scroll bar width. GTK it's off, too, but I > didn't analyze that further. If you mean that with (progn (setq frame-resize-pixelwise t) (make-frame '((width . (text-pixels . 800))))) then (frame-text-width) evaluates to 784, please try the attached patch. Thanks, martin [-- Attachment #2: make-frame.diff --] [-- Type: text/x-patch, Size: 671 bytes --] diff --git a/src/frame.c b/src/frame.c index f6053fca3ef..4c21ce5b51b 100644 --- a/src/frame.c +++ b/src/frame.c @@ -168,8 +168,10 @@ frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter) || (!horizontal && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth)) || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f)) - : ((horizontal && f->inhibit_horizontal_resize) - || (!horizontal && f->inhibit_vertical_resize))); + : ((horizontal && f->inhibit_horizontal_resize + && !EQ (parameter, Qscroll_bar_width)) + || (!horizontal && f->inhibit_vertical_resize + && !EQ (parameter, Qscroll_bar_height)))); return inhibit; } ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-11 9:37 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-11 22:41 ` Ship Mints 2024-12-12 6:05 ` Eli Zaretskii 2024-12-12 9:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 40+ messages in thread From: Ship Mints @ 2024-12-11 22:41 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 1062 bytes --] That patch works to address make-frame's respect for text-pixels, at least on NS (the only platform I tested). Now to address the clone-frame implementation to respect pixelwise, and/or as you suggested, perhaps a formal frame parameter resize-pixelwise. If we adopt the change I proposed it could be used in Emacs 30 or 31 since this is a bug (but not a regression). If we go for resize-pixelwise, it'll be 31, right? On Wed, Dec 11, 2024 at 4:38 AM martin rudalics <rudalics@gmx.at> wrote: > > Somewhere in the bowels > > of frame.c, et.al., is an issue where text-pixels behaves differently > than > > set-frame-size which is reliable on NS where make-frame using > text-pixels > > is incorrect by the vertical scroll bar width. GTK it's off, too, but I > > didn't analyze that further. > > If you mean that with > > (progn > (setq frame-resize-pixelwise t) > (make-frame '((width . (text-pixels . 800))))) > > then > > (frame-text-width) > > evaluates to 784, please try the attached patch. > > Thanks, martin [-- Attachment #2: Type: text/html, Size: 1675 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-11 22:41 ` Ship Mints @ 2024-12-12 6:05 ` Eli Zaretskii 2024-12-12 9:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2024-12-12 6:05 UTC (permalink / raw) To: Ship Mints; +Cc: rudalics, 74750 > From: Ship Mints <shipmints@gmail.com> > Date: Wed, 11 Dec 2024 17:41:01 -0500 > Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org > > That patch works to address make-frame's respect for text-pixels, at least on NS (the only platform I tested). > > Now to address the clone-frame implementation to respect pixelwise, and/or as you suggested, perhaps a > formal frame parameter resize-pixelwise. If we adopt the change I proposed it could be used in Emacs 30 or > 31 since this is a bug (but not a regression). If we go for resize-pixelwise, it'll be 31, right? Actually, it's too late for Emacs 30, unless the bug is very serious and the fix is very safe. So I think this should end up on master regardless. ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-11 22:41 ` Ship Mints 2024-12-12 6:05 ` Eli Zaretskii @ 2024-12-12 9:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 10:30 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-12 9:22 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > That patch works to address make-frame's respect for text-pixels, at least > on NS (the only platform I tested). I don't like it much and I have to further test the behavior with fringes. Maybe I find better solution. > Now to address the clone-frame implementation to respect pixelwise, and/or > as you suggested, perhaps a formal frame parameter resize-pixelwise. If we > adopt the change I proposed it could be used in Emacs 30 or 31 since this > is a bug (but not a regression). If we go for resize-pixelwise, it'll be > 31, right? Everything we do here would have to go into Emacs 31. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-12 9:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-13 10:30 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 16:28 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-13 10:30 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 420 bytes --] > > That patch works to address make-frame's respect for text-pixels, at least > > on NS (the only platform I tested). > > I don't like it much and I have to further test the behavior with fringes. > Maybe I find better solution. I attach a patch that does away with a frame's inhibit_horizontal_resize and inhibit_vertical_resize slots. Please test it. If it works for you, I'll install it on master. martin [-- Attachment #2: make-frame.diff --] [-- Type: text/x-patch, Size: 2596 bytes --] diff --git a/src/frame.c b/src/frame.c index f6053fca3ef..2cf5fefa77c 100644 --- a/src/frame.c +++ b/src/frame.c @@ -158,20 +158,15 @@ get_frame_param (struct frame *frame, Lisp_Object prop) frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter) { Lisp_Object fullscreen = get_frame_param (f, Qfullscreen); - bool inhibit - = (f->after_make_frame - ? (EQ (frame_inhibit_implied_resize, Qt) + + return (EQ (frame_inhibit_implied_resize, Qt) || (CONSP (frame_inhibit_implied_resize) && !NILP (Fmemq (parameter, frame_inhibit_implied_resize))) || (horizontal && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight)) || (!horizontal && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth)) - || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f)) - : ((horizontal && f->inhibit_horizontal_resize) - || (!horizontal && f->inhibit_vertical_resize))); - - return inhibit; + || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f)); } @@ -957,8 +952,6 @@ make_frame (bool mini_p) f->garbaged = true; f->can_set_window_size = false; f->after_make_frame = false; - f->inhibit_horizontal_resize = false; - f->inhibit_vertical_resize = false; f->tab_bar_redisplayed = false; f->tab_bar_resized = false; f->tool_bar_redisplayed = false; @@ -3128,8 +3121,6 @@ DEFUN ("frame-after-make-frame", { struct frame *f = decode_live_frame (frame); f->after_make_frame = !NILP (made); - f->inhibit_horizontal_resize = false; - f->inhibit_vertical_resize = false; return made; } @@ -5918,7 +5909,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p, xsignal1 (Qargs_out_of_range, XCDR (width)); text_width = XFIXNUM (XCDR (width)); - f->inhibit_horizontal_resize = true; } else if (FLOATP (width)) { @@ -5954,7 +5944,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p, xsignal1 (Qargs_out_of_range, XCDR (height)); text_height = XFIXNUM (XCDR (height)); - f->inhibit_vertical_resize = true; } else if (FLOATP (height)) { diff --git a/src/frame.h b/src/frame.h index 1d920d1a6bc..172eb5eca99 100644 --- a/src/frame.h +++ b/src/frame.h @@ -519,10 +519,6 @@ #define EMACS_FRAME_H bool_bf tool_bar_redisplayed : 1; bool_bf tool_bar_resized : 1; - /* Inhibit implied resize before after_make_frame is set. */ - bool_bf inhibit_horizontal_resize : 1; - bool_bf inhibit_vertical_resize : 1; - /* Non-zero if this frame's faces need to be recomputed. */ bool_bf face_change : 1; ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-13 10:30 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-13 16:28 ` Ship Mints 2024-12-13 18:15 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-13 16:28 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 3693 bytes --] Thank you for the patch and continued focus on this. The following is the behavior I see on NS with the patch applied with target width 1700 and height 1000. These are results from clone-frame using text-pixels which I've included below without explicit pixelwise argument so it respects frame-resize-pixelwise. Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting on NS: #if defined (USE_GTK) || defined (HAVE_NS) frame_inhibit_implied_resize = list1 (Qtab_bar_lines); frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil text-width=1692 (Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988 (Δ-16) Result: respects lines/cols as expected. frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines) text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 (Δ0) Result: Okay by accident, I think, only because tab-bar-lines parameter is nil during adjust_frame_height invocations? frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 (Δ0) Result: Okay but with frame-inhibit-implied-resize nil, I'd have expected rows/cols vs. pixelwise. frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685 (Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004 (Δ0) Result: I think this case remains broken needing the adjustment from your first patch that you wanted to also account for fringes? My default GUI setup is frame-resize-pixelwise t frame-inhibit-implied-resize t as I expect many people have adopted these days. (defun my/clone-frame (&optional frame no-windows pixelwise) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. When PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame is not text-only, clone the originating frame's pixel size. 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 (list (selected-frame) current-prefix-arg)) (let* ((frame (or frame (selected-frame))) (windows (unless no-windows (window-state-get (frame-root-window frame)))) (default-frame-alist (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) (new-frame)) (when (and (display-graphic-p frame) (or pixelwise frame-resize-pixelwise)) (push (cons 'width (cons 'text-pixels (frame-text-width frame))) default-frame-alist) (push (cons 'height (cons 'text-pixels (frame-text-height frame))) default-frame-alist)) (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) On Fri, Dec 13, 2024 at 5:30 AM martin rudalics <rudalics@gmx.at> wrote: > > > That patch works to address make-frame's respect for text-pixels, at > least > > > on NS (the only platform I tested). > > > > I don't like it much and I have to further test the behavior with > fringes. > > Maybe I find better solution. > > I attach a patch that does away with a frame's inhibit_horizontal_resize > and inhibit_vertical_resize slots. Please test it. If it works for > you, I'll install it on master. > > martin [-- Attachment #2: Type: text/html, Size: 5404 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-13 16:28 ` Ship Mints @ 2024-12-13 18:15 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 18:25 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-13 18:15 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > Thank you for the patch and continued focus on this. The following is the > behavior I see on NS with the patch applied with target width 1700 and > height 1000. These are results from clone-frame using text-pixels which > I've included below without explicit pixelwise argument so it respects > frame-resize-pixelwise. Thank you for conducting these experiments. > Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting > on NS: > > #if defined (USE_GTK) || defined (HAVE_NS) > frame_inhibit_implied_resize = list1 (Qtab_bar_lines); > > frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil text-width=1692 > (Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988 > (Δ-16) > > Result: respects lines/cols as expected. It's problematic to clone a frame made with 'frame-resize-pixelwise' non-nil in a setting with 'frame-resize-pixelwise' nil. I always have 'frame-resize-pixelwise' t and never change it. > frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines) > text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) > native-height 1004 (Δ0) > > Result: Okay by accident, I think, only because tab-bar-lines parameter is > nil during adjust_frame_height invocations? adjust_frame_size you mean, I suppose. Does your frame have a tab bar? > frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700 > (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 (Δ0) > > Result: Okay but with frame-inhibit-implied-resize nil, I'd have expected > rows/cols vs. pixelwise. Why? 'frame-inhibit-implied-resize' is about _not_ resizing a frame's window when one removes/adds one of the items it mentions. It should work with pixelwise and normal resizing. > frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685 > (Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004 > (Δ0) > > Result: I think this case remains broken needing the adjustment from your > first patch that you wanted to also account for fringes? 15 is an odd number so it can't be the default fringes. IIUC it's your scroll bar which gets set up after the "frame was made" and while resizing is inhibited. Note that the fringes are purely Emacs internal - we can set them up any way we like. The scroll bar is more difficult since the toolkit usually determines its default width. You could try to debug this with a breakpoint in 'gui_set_scroll_bar_width' and after that one in 'frame_inhibit_resize'. Here on xfwm/GTK-3 the text width remains unchanged. > My default GUI setup is frame-resize-pixelwise t > frame-inhibit-implied-resize t as I expect many people have adopted these > days. Few people have AFAICT. Note that all these experiments are borderline. Cloning a frame should respect the settings that were active at the time the original was made. We can try to make it behave reasonably when these values change but I am not sure whether we will succeed. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-13 18:15 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-13 18:25 ` Ship Mints 2024-12-14 8:27 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-13 18:25 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 4518 bytes --] On Fri, Dec 13, 2024 at 1:15 PM martin rudalics <rudalics@gmx.at> wrote: > > Thank you for the patch and continued focus on this. The following is > the > > behavior I see on NS with the patch applied with target width 1700 and > > height 1000. These are results from clone-frame using text-pixels which > > I've included below without explicit pixelwise argument so it respects > > frame-resize-pixelwise. > > Thank you for conducting these experiments. > My pleasure, actually. I live inside Emacs so the better we make it for us the better for all. > Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting > > on NS: > > > > #if defined (USE_GTK) || defined (HAVE_NS) > > frame_inhibit_implied_resize = list1 (Qtab_bar_lines); > > > > frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil > text-width=1692 > > (Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988 > > (Δ-16) > > > > Result: respects lines/cols as expected. > > It's problematic to clone a frame made with 'frame-resize-pixelwise' > non-nil in a setting with 'frame-resize-pixelwise' nil. I always have > 'frame-resize-pixelwise' t and never change it. > As you pointed out, these are experiments and reflect the desire to fully understand (and control) Emacs behavior under various circumstances we encounter. > > frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines) > > text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) > > native-height 1004 (Δ0) > > > > Result: Okay by accident, I think, only because tab-bar-lines parameter > is > > nil during adjust_frame_height invocations? > > adjust_frame_size you mean, I suppose. Does your frame have a tab bar? > Yes, typo. These results are all under -Q as we need to repro, so no visible tab bar, just the default NS view which is the tool bar which under master, now appears on the title bar. Something I didn't notice until today but it's neither here nor there, I suppose. I disable tool-bar under all my own real-world circumstances. > frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700 > > (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 > (Δ0) > > > > Result: Okay but with frame-inhibit-implied-resize nil, I'd have > expected > > rows/cols vs. pixelwise. > > Why? 'frame-inhibit-implied-resize' is about _not_ resizing a frame's > window when one removes/adds one of the items it mentions. It should > work with pixelwise and normal resizing. > I said that because the latest patch respects frame-inhibit-implied-resize not frame-resize-pixelwise. > frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685 > > (Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004 > > (Δ0) > > > > Result: I think this case remains broken needing the adjustment from > your > > first patch that you wanted to also account for fringes? > > 15 is an odd number so it can't be the default fringes. IIUC it's your > scroll bar which gets set up after the "frame was made" and while > resizing is inhibited. Note that the fringes are purely Emacs internal > - we can set them up any way we like. The scroll bar is more difficult > since the toolkit usually determines its default width. You could try > to debug this with a breakpoint in 'gui_set_scroll_bar_width' and after > that one in 'frame_inhibit_resize'. Here on xfwm/GTK-3 the text width > remains unchanged. > Indeed 15 is the vertical scroll bar width. This was what I reported in the original bug submission. You suggested a patch that would accommodate fringes, et.al. If you'd like me to make adjustments; e.g., resizing fringes or whatever, happy to do it and rerun. These are all ostensively calls to clone-frame. I'd expect, as I guess most people would, that cloning produces the precise geometry of the originating frame, scroll bar or not. > > My default GUI setup is frame-resize-pixelwise t > > frame-inhibit-implied-resize t as I expect many people have adopted > these > > days. > > Few people have AFAICT. Note that all these experiments are borderline. > Cloning a frame should respect the settings that were active at the time > the original was made. We can try to make it behave reasonably when > these values change but I am not sure whether we will succeed. > Let's try. -Stephane [-- Attachment #2: Type: text/html, Size: 6892 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-13 18:25 ` Ship Mints @ 2024-12-14 8:27 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-15 20:34 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-14 8:27 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 1149 bytes --] > Indeed 15 is the vertical scroll bar width. This was what I reported in the > original bug submission. You suggested a patch that would accommodate > fringes, et.al. If you'd like me to make adjustments; e.g., resizing > fringes or whatever, happy to do it and rerun. I've been throwing out the child with the bathwater. Please try the attached patch which retains an important conjunct. > These are all ostensively calls to clone-frame. I'd expect, as I guess most > people would, that cloning produces the precise geometry of the originating > frame, scroll bar or not. The major purpose of 'frame-inhibit-implied-resize' is to avoid resizes when a frame has been tailored to fit into some arrangement of windows on the display as, for example, with a tiling window manager. Here I hardly ever use it. By design, it should have no effect when making a new frame which is what the corrected patch should support. Still, it might not work for elements like the external tool bar. >> We can try to make it behave reasonably when >> these values change but I am not sure whether we will succeed. >> > > Let's try. Let's. martin [-- Attachment #2: make-frame.diff --] [-- Type: text/x-patch, Size: 2909 bytes --] diff --git a/src/frame.c b/src/frame.c index f6053fca3ef..f22bd501a8d 100644 --- a/src/frame.c +++ b/src/frame.c @@ -158,20 +158,16 @@ get_frame_param (struct frame *frame, Lisp_Object prop) frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter) { Lisp_Object fullscreen = get_frame_param (f, Qfullscreen); - bool inhibit - = (f->after_make_frame - ? (EQ (frame_inhibit_implied_resize, Qt) - || (CONSP (frame_inhibit_implied_resize) - && !NILP (Fmemq (parameter, frame_inhibit_implied_resize))) - || (horizontal - && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight)) - || (!horizontal - && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth)) - || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f)) - : ((horizontal && f->inhibit_horizontal_resize) - || (!horizontal && f->inhibit_vertical_resize))); - return inhibit; + return (f->after_make_frame + && (EQ (frame_inhibit_implied_resize, Qt) + || (CONSP (frame_inhibit_implied_resize) + && !NILP (Fmemq (parameter, frame_inhibit_implied_resize))) + || (horizontal + && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight)) + || (!horizontal + && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth)) + || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f))); } @@ -957,8 +953,6 @@ make_frame (bool mini_p) f->garbaged = true; f->can_set_window_size = false; f->after_make_frame = false; - f->inhibit_horizontal_resize = false; - f->inhibit_vertical_resize = false; f->tab_bar_redisplayed = false; f->tab_bar_resized = false; f->tool_bar_redisplayed = false; @@ -3128,8 +3122,6 @@ DEFUN ("frame-after-make-frame", { struct frame *f = decode_live_frame (frame); f->after_make_frame = !NILP (made); - f->inhibit_horizontal_resize = false; - f->inhibit_vertical_resize = false; return made; } @@ -5918,7 +5910,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p, xsignal1 (Qargs_out_of_range, XCDR (width)); text_width = XFIXNUM (XCDR (width)); - f->inhibit_horizontal_resize = true; } else if (FLOATP (width)) { @@ -5954,7 +5945,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p, xsignal1 (Qargs_out_of_range, XCDR (height)); text_height = XFIXNUM (XCDR (height)); - f->inhibit_vertical_resize = true; } else if (FLOATP (height)) { diff --git a/src/frame.h b/src/frame.h index 1d920d1a6bc..172eb5eca99 100644 --- a/src/frame.h +++ b/src/frame.h @@ -519,10 +519,6 @@ #define EMACS_FRAME_H bool_bf tool_bar_redisplayed : 1; bool_bf tool_bar_resized : 1; - /* Inhibit implied resize before after_make_frame is set. */ - bool_bf inhibit_horizontal_resize : 1; - bool_bf inhibit_vertical_resize : 1; - /* Non-zero if this frame's faces need to be recomputed. */ bool_bf face_change : 1; ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-14 8:27 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-15 20:34 ` Ship Mints 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors ` (2 more replies) 0 siblings, 3 replies; 40+ messages in thread From: Ship Mints @ 2024-12-15 20:34 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 3339 bytes --] That patch seems to work. Thank you, Martin. I tested on NS with and without vertical scroll bars and with and without fringes. A basic test of frameset-save and frameset-restore now seem to correctly respect its embedded text-pixel geometry. Now, about clone-frame. Are there any objections to the below implementation that uses text-pixels? (defun my/clone-frame (&optional frame no-windows pixelwise) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. When PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame is not text-only, clone the originating frame's pixel size. 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 (list (selected-frame) current-prefix-arg)) (let* ((frame (or frame (selected-frame))) (windows (unless no-windows (window-state-get (frame-root-window frame)))) (default-frame-alist (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) (frame-resize-pixelwise frame-resize-pixelwise) (new-frame)) (when (and (display-graphic-p frame) (or pixelwise frame-resize-pixelwise)) (setq frame-resize-pixelwise t) (push (cons 'width (cons 'text-pixels (frame-text-width frame))) default-frame-alist) (push (cons 'height (cons 'text-pixels (frame-text-height frame))) default-frame-alist)) (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) I may be able to test on GTK early this week, but I think you have GNU Linux/GTK on your end? -Stephane On Sat, Dec 14, 2024 at 3:27 AM martin rudalics <rudalics@gmx.at> wrote: > > Indeed 15 is the vertical scroll bar width. This was what I reported in > the > > original bug submission. You suggested a patch that would accommodate > > fringes, et.al. If you'd like me to make adjustments; e.g., resizing > > fringes or whatever, happy to do it and rerun. > > I've been throwing out the child with the bathwater. Please try the > attached patch which retains an important conjunct. > > > These are all ostensively calls to clone-frame. I'd expect, as I guess > most > > people would, that cloning produces the precise geometry of the > originating > > frame, scroll bar or not. > > The major purpose of 'frame-inhibit-implied-resize' is to avoid resizes > when a frame has been tailored to fit into some arrangement of windows > on the display as, for example, with a tiling window manager. Here I > hardly ever use it. By design, it should have no effect when making a > new frame which is what the corrected patch should support. Still, it > might not work for elements like the external tool bar. > > >> We can try to make it behave reasonably when > >> these values change but I am not sure whether we will succeed. > >> > > > > Let's try. > > Let's. > > martin [-- Attachment #2: Type: text/html, Size: 4652 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-15 20:34 ` Ship Mints @ 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 9:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-20 15:39 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 9:23 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > Now, about clone-frame. Are there any objections to the below > implementation that uses text-pixels? ... > When > PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame > is not text-only, clone the originating frame's pixel size. I'd write that as If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's terminal is not text-only, use the pixel size of FRAME for the cloned frame. Otherwise, use the number of columns and lines of FRAME for the cloned frame. The behavior of the 'fullscreen' parameter might be queer if 'frame-resize-pixelwise' is nil and PIXELWISE is non-nil but that's to be expected. > I may be able to test on GTK early this week, but I think you have GNU > Linux/GTK on your end? I've tried here with a GTK-3 and a Motif build and have seen no problems. What I've seen is a slight misbehavior in setting up the 'fullscreen' parameter on the GTK build (so it's not related to your function). With (setq frame-resize-pixelwise t) (setq frame-inhibit-implied-resize t) setting it to 'maximized' works as expected but setting it to 'fullheight' leaves a gap at the bottom. Surprisingly, cloning a 'fullheight' frame with your function removes the gap. The Motif frames do not have the problem so it might be tool bar related but that should affect the maximized frame as well. I'll look into this later but would be interested if you see the same with a GTK build: To test: (setq frame-resize-pixelwise t) (setq frame-inhibit-implied-resize t) (set-frame-parameter nil 'fullscreen 'fullheight) martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 9:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 10:40 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 9:32 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > I'd write that as > > If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's terminal > is not text-only, use the pixel size of FRAME for the cloned frame. > Otherwise, use the number of columns and lines of FRAME for the cloned > frame. But may be we should write (and pixelwise frame-resize-pixelwise)) and say If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's terminal is not text-only, use the pixel size of FRAME for the cloned frame. Otherwise, use the number of columns and lines of FRAME for the cloned frame. Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is nil means asking for trouble since the window manager might not honor it. WDYT? martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 9:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 10:40 ` Ship Mints 2024-12-16 10:48 ` Ship Mints 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 40+ messages in thread From: Ship Mints @ 2024-12-16 10:40 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 1738 bytes --] My expectation was that clone-frame, when frame-resize-pixelwise t, would implicitly DTRT. I expect most people would have the same expectation. Same as frameset-restore (now fixed with the patch). When packages we use invoke clone-frame, we can't control their parameterization of those invocations without advice, so implied pixelwise seems wise. I added the explicit parameter more as awareness than anything else, but I imagine that carefully-crafted pixelwise child frames in packages may want explicit pixelwise clones if the authors aren't going to let-bind frame-resize-pixelwise around clone-frame calls. I'd prefer that we recommend that, actually. Let's just do away with the pixelwise parameter and rely on the user setting? That would be in keeping with the rest of the implementation, I think. On Mon, Dec 16, 2024 at 4:32 AM martin rudalics <rudalics@gmx.at> wrote: > > I'd write that as > > > > If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's > terminal > > is not text-only, use the pixel size of FRAME for the cloned frame. > > Otherwise, use the number of columns and lines of FRAME for the > cloned > > frame. > > But may be we should write > > (and pixelwise frame-resize-pixelwise)) > > and say > > If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's > terminal is not text-only, use the pixel size of FRAME for the cloned > frame. Otherwise, use the number of columns and lines of FRAME for > the cloned frame. > > Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is > nil means asking for trouble since the window manager might not honor > it. WDYT? > > martin > [-- Attachment #2: Type: text/html, Size: 2477 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 10:40 ` Ship Mints @ 2024-12-16 10:48 ` Ship Mints 2024-12-16 15:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-16 10:48 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 3444 bytes --] With your suggested wording changes but without the pixelwise argument: (defun clone-frame (&optional frame no-windows) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. When the user option `frame-resize-pixelwise' is non-nil, and FRAME is not text-only, clone the originating frame's pixel size. Otherwise, use the number of FRAME's columns and lines in the clone. 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 (list (selected-frame) current-prefix-arg)) (let* ((frame (or frame (selected-frame))) (windows (unless no-windows (window-state-get (frame-root-window frame)))) (default-frame-alist (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) (new-frame)) (when (and (display-graphic-p frame) frame-resize-pixelwise) (push (cons 'width (cons 'text-pixels (frame-text-width frame))) default-frame-alist) (push (cons 'height (cons 'text-pixels (frame-text-height frame))) default-frame-alist)) (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) On Mon, Dec 16, 2024 at 5:40 AM Ship Mints <shipmints@gmail.com> wrote: > My expectation was that clone-frame, when frame-resize-pixelwise t, would > implicitly DTRT. I expect most people would have the same expectation. Same > as frameset-restore (now fixed with the patch). > > When packages we use invoke clone-frame, we can't control their > parameterization of those invocations without advice, so > implied pixelwise seems wise. I added the explicit parameter more as > awareness than anything else, but I imagine that carefully-crafted > pixelwise child frames in packages may want explicit pixelwise clones if > the authors aren't going to let-bind frame-resize-pixelwise around > clone-frame calls. I'd prefer that we recommend that, actually. > > Let's just do away with the pixelwise parameter and rely on the user > setting? That would be in keeping with the rest of the implementation, I > think. > > On Mon, Dec 16, 2024 at 4:32 AM martin rudalics <rudalics@gmx.at> wrote: > >> > I'd write that as >> > >> > If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's >> terminal >> > is not text-only, use the pixel size of FRAME for the cloned frame. >> > Otherwise, use the number of columns and lines of FRAME for the >> cloned >> > frame. >> >> But may be we should write >> >> (and pixelwise frame-resize-pixelwise)) >> >> and say >> >> If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's >> terminal is not text-only, use the pixel size of FRAME for the cloned >> frame. Otherwise, use the number of columns and lines of FRAME for >> the cloned frame. >> >> Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is >> nil means asking for trouble since the window manager might not honor >> it. WDYT? >> >> martin >> > [-- Attachment #2: Type: text/html, Size: 4816 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 10:48 ` Ship Mints @ 2024-12-16 15:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 15:55 ` Ship Mints 2024-12-16 16:39 ` Ship Mints 0 siblings, 2 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 15:49 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > With your suggested wording changes but without the pixelwise argument: ... > (new-frame)) Please omit the parentheses here. ... > (unless (display-graphic-p frame) This is a fix you should mention in the commit message. ... Please provide a diff to the current frame.el of master and a commit message. Have you done your Emacs paperwork already? If not, I can install this as a tiny change because it's short enough. Still you should start the paperwork since otherwise we cannot install further changes from you. Thanks, martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 15:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 15:55 ` Ship Mints 2024-12-16 16:01 ` Ship Mints 2024-12-16 16:39 ` Ship Mints 1 sibling, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-16 15:55 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 827 bytes --] Yes, paperwork on file. I will submit a patch to the latest master now that we're agreed. Thank you for your help with these changes. On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote: > > With your suggested wording changes but without the pixelwise argument: > ... > > (new-frame)) > > Please omit the parentheses here. > ... > > (unless (display-graphic-p frame) > > This is a fix you should mention in the commit message. > ... > > Please provide a diff to the current frame.el of master and a commit > message. Have you done your Emacs paperwork already? If not, I can > install this as a tiny change because it's short enough. Still you > should start the paperwork since otherwise we cannot install further > changes from you. > > Thanks, martin > [-- Attachment #2: Type: text/html, Size: 1251 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 15:55 ` Ship Mints @ 2024-12-16 16:01 ` Ship Mints 2024-12-16 16:02 ` Ship Mints 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-16 16:01 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 1008 bytes --] Martin, I'm assuming you'll do the patch against frame.h and frame.c? On Mon, Dec 16, 2024 at 10:55 AM Ship Mints <shipmints@gmail.com> wrote: > Yes, paperwork on file. I will submit a patch to the latest master now > that we're agreed. Thank you for your help with these changes. > > On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote: > >> > With your suggested wording changes but without the pixelwise argument: >> ... >> > (new-frame)) >> >> Please omit the parentheses here. >> ... >> > (unless (display-graphic-p frame) >> >> This is a fix you should mention in the commit message. >> ... >> >> Please provide a diff to the current frame.el of master and a commit >> message. Have you done your Emacs paperwork already? If not, I can >> install this as a tiny change because it's short enough. Still you >> should start the paperwork since otherwise we cannot install further >> changes from you. >> >> Thanks, martin >> > [-- Attachment #2: Type: text/html, Size: 1764 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 16:01 ` Ship Mints @ 2024-12-16 16:02 ` Ship Mints 0 siblings, 0 replies; 40+ messages in thread From: Ship Mints @ 2024-12-16 16:02 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 1149 bytes --] Aha, you beat me to it. On Mon, Dec 16, 2024 at 11:01 AM Ship Mints <shipmints@gmail.com> wrote: > Martin, I'm assuming you'll do the patch against frame.h and frame.c? > > On Mon, Dec 16, 2024 at 10:55 AM Ship Mints <shipmints@gmail.com> wrote: > >> Yes, paperwork on file. I will submit a patch to the latest master now >> that we're agreed. Thank you for your help with these changes. >> >> On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote: >> >>> > With your suggested wording changes but without the pixelwise >>> argument: >>> ... >>> > (new-frame)) >>> >>> Please omit the parentheses here. >>> ... >>> > (unless (display-graphic-p frame) >>> >>> This is a fix you should mention in the commit message. >>> ... >>> >>> Please provide a diff to the current frame.el of master and a commit >>> message. Have you done your Emacs paperwork already? If not, I can >>> install this as a tiny change because it's short enough. Still you >>> should start the paperwork since otherwise we cannot install further >>> changes from you. >>> >>> Thanks, martin >>> >> [-- Attachment #2: Type: text/html, Size: 2223 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 15:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 15:55 ` Ship Mints @ 2024-12-16 16:39 ` Ship Mints 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 17:32 ` Eli Zaretskii 1 sibling, 2 replies; 40+ messages in thread From: Ship Mints @ 2024-12-16 16:39 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1.1: Type: text/plain, Size: 708 bytes --] Patch attached. On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote: > > With your suggested wording changes but without the pixelwise argument: > ... > > (new-frame)) > > Please omit the parentheses here. > ... > > (unless (display-graphic-p frame) > > This is a fix you should mention in the commit message. > ... > > Please provide a diff to the current frame.el of master and a commit > message. Have you done your Emacs paperwork already? If not, I can > install this as a tiny change because it's short enough. Still you > should start the paperwork since otherwise we cannot install further > changes from you. > > Thanks, martin > [-- Attachment #1.2: Type: text/html, Size: 1129 bytes --] [-- Attachment #2: 0001-Support-pixelwise-frame-cloning-bug-74750.patch --] [-- Type: application/octet-stream, Size: 2003 bytes --] From ae7de06ce1a285960c5c77955f66fb30a88a828c Mon Sep 17 00:00:00 2001 From: shipmints <shipmints@gmail.com> Date: Mon, 16 Dec 2024 11:36:27 -0500 Subject: [PATCH] Support pixelwise frame cloning (bug#74750) * lisp/frame.el (clone-frame): Honor frame-resize-pixelwise when cloning a frame's geometry via make-frame text-pixel feature. Correctly specify source frame in display-graphic-p test for tty selection behavior. --- lisp/frame.el | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lisp/frame.el b/lisp/frame.el index 1b5aa8cff08..02f3f9f9dae 100644 --- a/lisp/frame.el +++ b/lisp/frame.el @@ -863,6 +863,9 @@ make-frame-command (defun clone-frame (&optional frame no-windows) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. +When the user option `frame-resize-pixelwise' is non-nil, and FRAME is +not text-only, clone the originating frame's pixel size. Otherwise, use +the number of FRAME's columns and lines for the clone. 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 @@ -875,10 +878,17 @@ clone-frame (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) - (new-frame (make-frame))) + new-frame) + (when (and frame-resize-pixelwise + (display-graphic-p frame)) + (push (cons 'width (cons 'text-pixels (frame-text-width frame))) + default-frame-alist) + (push (cons 'height (cons 'text-pixels (frame-text-height frame))) + default-frame-alist)) + (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) - (unless (display-graphic-p) + (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) -- 2.47.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 16:39 ` Ship Mints @ 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 17:32 ` Eli Zaretskii 1 sibling, 0 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 17:06 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > Patch attached. Installed on master. Thanks, martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 16:39 ` Ship Mints 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 17:32 ` Eli Zaretskii 2024-12-16 17:51 ` Ship Mints 2024-12-17 9:00 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 40+ messages in thread From: Eli Zaretskii @ 2024-12-16 17:32 UTC (permalink / raw) To: Ship Mints; +Cc: rudalics, 74750 > From: Ship Mints <shipmints@gmail.com> > Date: Mon, 16 Dec 2024 11:39:02 -0500 > Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org > > Patch attached. Thanks. > (defun clone-frame (&optional frame no-windows) > "Make a new frame with the same parameters and windows as FRAME. > With a prefix arg NO-WINDOWS, don't clone the window configuration. > +When the user option `frame-resize-pixelwise' is non-nil, and FRAME is Please avoid using "when" as a conditional; use "if" instead. "When" can be interpreted as time-related condition, which is not what you want. > +not text-only, clone the originating frame's pixel size. Otherwise, use > +the number of FRAME's columns and lines for the clone. This uses double negation, which makes the documentation harder to understand. Suggest to rephrase: By default, clone the pixel-size of the original frame, but if `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use the FRAME's number of lines and columns for the clone. > - (unless (display-graphic-p) > + (unless (display-graphic-p frame) > (select-frame new-frame)) Why do you need this condition? ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 17:32 ` Eli Zaretskii @ 2024-12-16 17:51 ` Ship Mints 2024-12-16 19:10 ` Eli Zaretskii 2024-12-16 19:13 ` Eli Zaretskii 2024-12-17 9:00 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 40+ messages in thread From: Ship Mints @ 2024-12-16 17:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: rudalics, 74750 [-- Attachment #1.1: Type: text/plain, Size: 1827 bytes --] Updated patch with docstring amendments. The tty test you're asking about was already in place, I merely added the missing frame argument that ensures the test is against the source frame rather than the ambient selected-frame. As to its utility, I can go only by the docstring sentence that reads "If the terminal is a text-only terminal then also select the new frame." Seems to have originated with this commit 2c662e6d66165db8ead2f4d19a61af521807b8ba in 2021 when clone-frame was introduced. On Mon, Dec 16, 2024 at 12:32 PM Eli Zaretskii <eliz@gnu.org> wrote: > > From: Ship Mints <shipmints@gmail.com> > > Date: Mon, 16 Dec 2024 11:39:02 -0500 > > Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org > > > > Patch attached. > > Thanks. > > > (defun clone-frame (&optional frame no-windows) > > "Make a new frame with the same parameters and windows as FRAME. > > With a prefix arg NO-WINDOWS, don't clone the window configuration. > > +When the user option `frame-resize-pixelwise' is non-nil, and FRAME is > > Please avoid using "when" as a conditional; use "if" instead. "When" > can be interpreted as time-related condition, which is not what you > want. > > > +not text-only, clone the originating frame's pixel size. Otherwise, use > > +the number of FRAME's columns and lines for the clone. > > This uses double negation, which makes the documentation harder to > understand. Suggest to rephrase: > > By default, clone the pixel-size of the original frame, but if > `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use > the FRAME's number of lines and columns for the clone. > > > - (unless (display-graphic-p) > > + (unless (display-graphic-p frame) > > (select-frame new-frame)) > > Why do you need this condition? > [-- Attachment #1.2: Type: text/html, Size: 2804 bytes --] [-- Attachment #2: 0001-Support-pixelwise-frame-cloning-bug-74750.patch --] [-- Type: application/octet-stream, Size: 1997 bytes --] From ae7de06ce1a285960c5c77955f66fb30a88a828c Mon Sep 17 00:00:00 2001 From: shipmints <shipmints@gmail.com> Date: Mon, 16 Dec 2024 11:36:27 -0500 Subject: [PATCH] Support pixelwise frame cloning (bug#74750) * lisp/frame.el (clone-frame): Honor frame-resize-pixelwise when cloning a frame's geometry via make-frame text-pixel feature. Correctly specify source frame in display-graphic-p test for tty selection behavior. --- lisp/frame.el | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lisp/frame.el b/lisp/frame.el index 1b5aa8cff08..02f3f9f9dae 100644 --- a/lisp/frame.el +++ b/lisp/frame.el @@ -863,3 +863,6 @@ make-frame-command (defun clone-frame (&optional frame no-windows) "Make a new frame with the same parameters and windows as FRAME. With a prefix arg NO-WINDOWS, don't clone the window configuration. +If the user option `frame-resize-pixelwise' is non-nil, and FRAME is +graphical, clone the originating frame's pixel size. Otherwise, use +the number of FRAME's columns and lines for the clone. 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 @@ -875,10 +878,17 @@ clone-frame (seq-remove (lambda (elem) (memq (car elem) frame-internal-parameters)) (frame-parameters frame))) - (new-frame (make-frame))) + new-frame) + (when (and frame-resize-pixelwise + (display-graphic-p frame)) + (push (cons 'width (cons 'text-pixels (frame-text-width frame))) + default-frame-alist) + (push (cons 'height (cons 'text-pixels (frame-text-height frame))) + default-frame-alist)) + (setq new-frame (make-frame)) (when windows (window-state-put windows (frame-root-window new-frame) 'safe)) - (unless (display-graphic-p) + (unless (display-graphic-p frame) (select-frame new-frame)) new-frame)) -- 2.47.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 17:51 ` Ship Mints @ 2024-12-16 19:10 ` Eli Zaretskii 2024-12-17 1:20 ` Ship Mints 2024-12-17 9:02 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 19:13 ` Eli Zaretskii 1 sibling, 2 replies; 40+ messages in thread From: Eli Zaretskii @ 2024-12-16 19:10 UTC (permalink / raw) To: Ship Mints; +Cc: rudalics, 74750 > From: Ship Mints <shipmints@gmail.com> > Date: Mon, 16 Dec 2024 12:51:14 -0500 > Cc: rudalics@gmx.at, 74750@debbugs.gnu.org > > The tty test you're asking about was already in place, I merely added the missing frame argument that > ensures the test is against the source frame rather than the ambient selected-frame. As to its utility, I can go > only by the docstring sentence that reads "If the terminal is a text-only terminal then also select the new > frame." Seems to have originated with this commit 2c662e6d66165db8ead2f4d19a61af521807b8ba in 2021 > when clone-frame was introduced. Martin, do we want to remove the condition? I dislike such differences if they are not really necessary. I understand why in the TTY case we must select the frame, but I don't understand why we refrain from doing so in the GUI case. WDYT? ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 19:10 ` Eli Zaretskii @ 2024-12-17 1:20 ` Ship Mints 2024-12-17 9:02 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 40+ messages in thread From: Ship Mints @ 2024-12-17 1:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: rudalics, 74750 [-- Attachment #1: Type: text/plain, Size: 1088 bytes --] Maybe the compromise to unify the behavior is to always select the frame if invoked interactively, otherwise not? On Mon, Dec 16, 2024 at 2:10 PM Eli Zaretskii <eliz@gnu.org> wrote: > > From: Ship Mints <shipmints@gmail.com> > > Date: Mon, 16 Dec 2024 12:51:14 -0500 > > Cc: rudalics@gmx.at, 74750@debbugs.gnu.org > > > > The tty test you're asking about was already in place, I merely added > the missing frame argument that > > ensures the test is against the source frame rather than the ambient > selected-frame. As to its utility, I can go > > only by the docstring sentence that reads "If the terminal is a > text-only terminal then also select the new > > frame." Seems to have originated with this commit > 2c662e6d66165db8ead2f4d19a61af521807b8ba in 2021 > > when clone-frame was introduced. > > Martin, do we want to remove the condition? I dislike such > differences if they are not really necessary. I understand why in the > TTY case we must select the frame, but I don't understand why we > refrain from doing so in the GUI case. > > WDYT? > [-- Attachment #2: Type: text/html, Size: 1682 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 19:10 ` Eli Zaretskii 2024-12-17 1:20 ` Ship Mints @ 2024-12-17 9:02 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-17 13:34 ` Eli Zaretskii 1 sibling, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-17 9:02 UTC (permalink / raw) To: Eli Zaretskii, Ship Mints; +Cc: 74750 > Martin, do we want to remove the condition? I dislike such > differences if they are not really necessary. I understand why in the > TTY case we must select the frame, but I don't understand why we > refrain from doing so in the GUI case. Shouldn't we have decided that at the time 'clone-frame' was proposed? Nowadays people might want to keep the selected frame alone; in particular if FRAME is not the selected frame. It would make no difference here since 'make-frame' will automatically select the clone and give it input focus. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-17 9:02 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-17 13:34 ` Eli Zaretskii 0 siblings, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2024-12-17 13:34 UTC (permalink / raw) To: martin rudalics; +Cc: 74750, shipmints > Date: Tue, 17 Dec 2024 10:02:13 +0100 > Cc: 74750@debbugs.gnu.org > From: martin rudalics <rudalics@gmx.at> > > > Martin, do we want to remove the condition? I dislike such > > differences if they are not really necessary. I understand why in the > > TTY case we must select the frame, but I don't understand why we > > refrain from doing so in the GUI case. > > Shouldn't we have decided that at the time 'clone-frame' was proposed? > Nowadays people might want to keep the selected frame alone; in > particular if FRAME is not the selected frame. It would make no > difference here since 'make-frame' will automatically select the clone > and give it input focus. I see your point. So let's leave that aspect alone. ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 17:51 ` Ship Mints 2024-12-16 19:10 ` Eli Zaretskii @ 2024-12-16 19:13 ` Eli Zaretskii 2024-12-16 19:26 ` Ship Mints 1 sibling, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2024-12-16 19:13 UTC (permalink / raw) To: Ship Mints; +Cc: rudalics, 74750 > From: Ship Mints <shipmints@gmail.com> > Date: Mon, 16 Dec 2024 12:51:14 -0500 > Cc: rudalics@gmx.at, 74750@debbugs.gnu.org > > Updated patch with docstring amendments. Thanks, but since martin already installed the previous patch, we now need only the followup changes. ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 19:13 ` Eli Zaretskii @ 2024-12-16 19:26 ` Ship Mints 0 siblings, 0 replies; 40+ messages in thread From: Ship Mints @ 2024-12-16 19:26 UTC (permalink / raw) To: Eli Zaretskii; +Cc: rudalics, 74750 [-- Attachment #1.1: Type: text/plain, Size: 411 bytes --] Docstring clarifications attached. On Mon, Dec 16, 2024 at 2:14 PM Eli Zaretskii <eliz@gnu.org> wrote: > > From: Ship Mints <shipmints@gmail.com> > > Date: Mon, 16 Dec 2024 12:51:14 -0500 > > Cc: rudalics@gmx.at, 74750@debbugs.gnu.org > > > > Updated patch with docstring amendments. > > Thanks, but since martin already installed the previous patch, we now > need only the followup changes. > [-- Attachment #1.2: Type: text/html, Size: 969 bytes --] [-- Attachment #2: 0001-Support-pixelwise-frame-cloning-bug-74750.patch --] [-- Type: application/octet-stream, Size: 1324 bytes --] From 7985a18470f28bb3f49dffa6897d9cc238dc0021 Mon Sep 17 00:00:00 2001 From: shipmints <shipmints@gmail.com> Date: Mon, 16 Dec 2024 14:24:06 -0500 Subject: [PATCH] Support pixelwise frame cloning (bug#74750) * lisp/frame.el (clone-frame): Minor docstring clarification. --- lisp/frame.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lisp/frame.el b/lisp/frame.el index 02f3f9f9dae..6d0379e76a9 100644 --- a/lisp/frame.el +++ b/lisp/frame.el @@ -862,10 +862,10 @@ make-frame-command (defun clone-frame (&optional frame no-windows) "Make a new frame with the same parameters and windows as FRAME. -With a prefix arg NO-WINDOWS, don't clone the window configuration. -When the user option `frame-resize-pixelwise' is non-nil, and FRAME is -not text-only, clone the originating frame's pixel size. Otherwise, use -the number of FRAME's columns and lines for the clone. +With a prefix arg NO-WINDOWS, don't clone the window configuration. If +the user option `frame-resize-pixelwise' is non-nil, and FRAME is +graphical, clone the originating frame's pixel size. Otherwise, use the +number of FRAME's columns and lines for the clone. 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 -- 2.47.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 17:32 ` Eli Zaretskii 2024-12-16 17:51 ` Ship Mints @ 2024-12-17 9:00 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-17 13:29 ` Eli Zaretskii 1 sibling, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-17 9:00 UTC (permalink / raw) To: Eli Zaretskii, Ship Mints; +Cc: 74750 > Suggest to rephrase: > > By default, clone the pixel-size of the original frame, but if > `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use > the FRAME's number of lines and columns for the clone. This is irritating as well because the default value of `frame-resize-pixelwise' _is_ nil. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-17 9:00 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-17 13:29 ` Eli Zaretskii 2024-12-18 10:05 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2024-12-17 13:29 UTC (permalink / raw) To: martin rudalics; +Cc: 74750, shipmints > Date: Tue, 17 Dec 2024 10:00:33 +0100 > Cc: 74750@debbugs.gnu.org > From: martin rudalics <rudalics@gmx.at> > > > Suggest to rephrase: > > > > By default, clone the pixel-size of the original frame, but if > > `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use > > the FRAME's number of lines and columns for the clone. > > This is irritating as well because the default value of > `frame-resize-pixelwise' _is_ nil. OK, I've now rewritten that part more thoroughly. ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-17 13:29 ` Eli Zaretskii @ 2024-12-18 10:05 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-18 10:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 74750, shipmints >> This is irritating as well because the default value of >> `frame-resize-pixelwise' _is_ nil. > > OK, I've now rewritten that part more thoroughly. Thanks, martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 10:40 ` Ship Mints 2024-12-16 10:48 ` Ship Mints @ 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:41 ` Ship Mints 1 sibling, 1 reply; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 16:07 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > Let's just do away with the pixelwise parameter and rely on the user > setting? That would be in keeping with the rest of the implementation, I > think. I'll add a 'resize-pixelwise' frame parameter soon. Then 'clone-frame' would simply have to inspect the parameter of the frame to clone and we are done. Other packages might have to adapt too since with the new parameter a user could never have set 'frame-resize-pixelwise' and still have frames that resize pixelwise. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 16:41 ` Ship Mints 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 40+ messages in thread From: Ship Mints @ 2024-12-16 16:41 UTC (permalink / raw) To: martin rudalics; +Cc: Eli Zaretskii, 74750 [-- Attachment #1: Type: text/plain, Size: 799 bytes --] The notion here would be that any non-tty frame created under frame-resize-pixelwise is t has its resize-pixelwise parameter set by default? Should that affect frameset-save/restore also? On Mon, Dec 16, 2024 at 11:07 AM martin rudalics <rudalics@gmx.at> wrote: > > Let's just do away with the pixelwise parameter and rely on the user > > setting? That would be in keeping with the rest of the implementation, I > > think. > > I'll add a 'resize-pixelwise' frame parameter soon. Then 'clone-frame' > would simply have to inspect the parameter of the frame to clone and we > are done. > > Other packages might have to adapt too since with the new parameter a > user could never have set 'frame-resize-pixelwise' and still have frames > that resize pixelwise. > > martin > [-- Attachment #2: Type: text/html, Size: 1224 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-16 16:41 ` Ship Mints @ 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 17:06 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > The notion here would be that any non-tty frame created under > frame-resize-pixelwise is t has its resize-pixelwise parameter set by > default? Yes. > Should that affect frameset-save/restore also? Yes, for restoring on a non-tty terminal. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-15 20:34 ` Ship Mints 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-20 15:39 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 0 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-16 16:07 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > That patch seems to work. Thank you, Martin. I tested on NS with and > without vertical scroll bars and with and without fringes. A basic test of > frameset-save and frameset-restore now seem to correctly respect its > embedded text-pixel geometry. Installed on master. The purpose of the slots I removed was likely that I originally planned to implement something like (width . (outer-pixels . ...)) (height . (outer-pixels . ...)) in which case any implied resizing should have been completely inhibited during the entire initial phase. But I never did that. 'make-frame' might still need some tweaking for the tool bar when 'frame-inhibit-implied-resize' is non-nil. I'll have to look into that. martin ^ permalink raw reply [flat|nested] 40+ messages in thread
* bug#74750: clone-frame and make-frame pixelwise issues 2024-12-15 20:34 ` Ship Mints 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-20 15:39 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 0 replies; 40+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-20 15:39 UTC (permalink / raw) To: Ship Mints; +Cc: Eli Zaretskii, 74750 > I may be able to test on GTK early this week, but I think you have GNU > Linux/GTK on your end? I now hopefully fixed a bug with 'frame-inhibit-implied-resize' initially non-nil on GTK. If there are no more issues, I'd say we can close this bug. Do you agree? martin ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2024-12-20 15:39 UTC | newest] Thread overview: 40+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-09 15:51 bug#74750: clone-frame and make-frame pixelwise issues Ship Mints 2024-12-10 12:27 ` Eli Zaretskii 2024-12-10 15:56 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-10 16:24 ` Ship Mints 2024-12-11 9:37 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-11 22:41 ` Ship Mints 2024-12-12 6:05 ` Eli Zaretskii 2024-12-12 9:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 10:30 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 16:28 ` Ship Mints 2024-12-13 18:15 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-13 18:25 ` Ship Mints 2024-12-14 8:27 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-15 20:34 ` Ship Mints 2024-12-16 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 9:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 10:40 ` Ship Mints 2024-12-16 10:48 ` Ship Mints 2024-12-16 15:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 15:55 ` Ship Mints 2024-12-16 16:01 ` Ship Mints 2024-12-16 16:02 ` Ship Mints 2024-12-16 16:39 ` Ship Mints 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 17:32 ` Eli Zaretskii 2024-12-16 17:51 ` Ship Mints 2024-12-16 19:10 ` Eli Zaretskii 2024-12-17 1:20 ` Ship Mints 2024-12-17 9:02 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-17 13:34 ` Eli Zaretskii 2024-12-16 19:13 ` Eli Zaretskii 2024-12-16 19:26 ` Ship Mints 2024-12-17 9:00 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-17 13:29 ` Eli Zaretskii 2024-12-18 10:05 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:41 ` Ship Mints 2024-12-16 17:06 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-16 16:07 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-12-20 15:39 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).