From: "Basil L. Contovounesios" <contovob@tcd.ie>
To: <30207@debbugs.gnu.org>
Cc: Noam Postavsky <npostavs@users.sourceforge.net>
Subject: bug#30207: 27.0.50; [PATCH] other-window-for-scrolling returns window on daemon frame
Date: Wed, 02 May 2018 19:57:59 +0100 [thread overview]
Message-ID: <87zi1h6g20.fsf@tcd.ie> (raw)
In-Reply-To: <87tvvcya9i.fsf@tcd.ie> (Basil L. Contovounesios's message of "Tue, 23 Jan 2018 13:28:41 +0000")
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Limit "other window" scrolling to current terminal --]
[-- Type: text/x-diff, Size: 1598 bytes --]
From da3f96bac102e731b5d7d99d26a53921d431c0ad Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Mon, 30 Apr 2018 18:02:15 +0100
Subject: [PATCH 1/4] Limit "other window" scrolling to current terminal
* src/window.c (Fother_window_for_scrolling): Limit next-window
search to visible frames on the current terminal. (bug#30207)
---
src/window.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/window.c b/src/window.c
index e6d0280d9b..59c9422029 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5709,8 +5709,7 @@ specifies the window. This takes precedence over
&& !NILP (Vminibuf_scroll_window))
window = Vminibuf_scroll_window;
/* If buffer is specified and live, scroll that buffer. */
- else if (!NILP (Vother_window_scroll_buffer)
- && BUFFERP (Vother_window_scroll_buffer)
+ else if (BUFFERP (Vother_window_scroll_buffer)
&& BUFFER_LIVE_P (XBUFFER (Vother_window_scroll_buffer)))
{
window = Fget_buffer_window (Vother_window_scroll_buffer, Qnil);
@@ -5725,11 +5724,8 @@ specifies the window. This takes precedence over
if (EQ (window, selected_window))
/* That didn't get us anywhere; look for a window on another
- visible frame. */
- do
- window = Fnext_window (window, Qnil, Qt);
- while (! FRAME_VISIBLE_P (XFRAME (WINDOW_FRAME (XWINDOW (window))))
- && ! EQ (window, selected_window));
+ visible frame on the current terminal. */
+ window = Fnext_window (window, Qnil, Qvisible);
}
CHECK_LIVE_WINDOW (window);
--
2.17.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Improve documentation for "other window" scrolling --]
[-- Type: text/x-diff, Size: 6181 bytes --]
From 5d47174ea4205742094e3e8f8bba38f3fc9bef8a Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Mon, 30 Apr 2018 18:06:59 +0100
Subject: [PATCH 2/4] Improve documentation for "other window" scrolling
* doc/emacs/windows.texi (Other Window):
* doc/lispref/windows.texi (Textual Scrolling):
Document scroll-other-window-down.
* doc/lispref/minibuf.texi (Minibuffer Misc):
Cross-reference minibuffer-scroll-window with Textual Scrolling.
* src/window.c (Fother_window_for_scrolling):
Clarify how "other window" is determined in docstring.
(Fscroll_other_window):
Simplify docstring, pointing to that of Fother_window_for_scrolling.
---
doc/emacs/windows.texi | 18 ++++++++++++------
doc/lispref/minibuf.texi | 2 +-
doc/lispref/windows.texi | 7 +++++++
src/window.c | 25 ++++++++++++-------------
4 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/doc/emacs/windows.texi b/doc/emacs/windows.texi
index 809363305f..cbcf8f645f 100644
--- a/doc/emacs/windows.texi
+++ b/doc/emacs/windows.texi
@@ -157,7 +157,9 @@ Other Window
@item C-x o
Select another window (@code{other-window}).
@item C-M-v
-Scroll the next window (@code{scroll-other-window}).
+Scroll the next window upward (@code{scroll-other-window}).
+@item C-M-S-v
+Scroll the next window downward (@code{scroll-other-window-down}).
@item mouse-1
@kbd{mouse-1}, in the text area of a window, selects the window and
moves point to the position clicked. Clicking in the mode line
@@ -181,13 +183,17 @@ Other Window
@kindex C-M-v
@findex scroll-other-window
+@kindex C-M-S-v
+@findex scroll-other-window-down
The usual scrolling commands (@pxref{Display}) apply to the selected
-window only, but there is one command to scroll the next window.
+window only, but there are two commands to scroll the next window.
@kbd{C-M-v} (@code{scroll-other-window}) scrolls the window that
-@kbd{C-x o} would select. It takes arguments, positive and negative,
-like @kbd{C-v}. (In the minibuffer, @kbd{C-M-v} scrolls the help
-window associated with the minibuffer, if any, rather than the next
-window in the standard cyclic order; @pxref{Minibuffer Edit}.)
+@kbd{C-x o} would select upward. It takes arguments, positive and
+negative, like @kbd{C-v}. (In the minibuffer, @kbd{C-M-v} scrolls the
+help window associated with the minibuffer, if any, rather than the
+next window in the standard cyclic order; @pxref{Minibuffer Edit}.)
+@kbd{C-M-S-v} (@code{scroll-other-window-down}) scrolls the next
+window downward in a similar way.
@vindex mouse-autoselect-window
If you set @code{mouse-autoselect-window} to a non-@code{nil} value,
diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index 338e03ef74..889b64af8a 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -2462,7 +2462,7 @@ Minibuffer Misc
@anchor{Definition of minibuffer-scroll-window}
If the value of this variable is non-@code{nil}, it should be a window
object. When the function @code{scroll-other-window} is called in the
-minibuffer, it scrolls this window.
+minibuffer, it scrolls this window (@pxref{Textual Scrolling}).
@end defvar
@defun minibuffer-selected-window
diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index f5de2fc90b..315ffd4f48 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4028,6 +4028,13 @@ Textual Scrolling
@samp{End of buffer}.
@end deffn
+@deffn Command scroll-other-window-down &optional count
+This function scrolls the text in another window downward @var{count}
+lines. Negative values of @var{count}, or @code{nil}, are handled as
+in @code{scroll-down}. In other respects, it behaves the same way as
+@code{scroll-other-window} does.
+@end deffn
+
@defvar other-window-scroll-buffer
If this variable is non-@code{nil}, it tells @code{scroll-other-window}
which buffer's window to scroll.
diff --git a/src/window.c b/src/window.c
index 59c9422029..af317674bb 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5696,11 +5696,12 @@ When calling from a program, supply as argument a number, nil, or `-'. */)
\f
DEFUN ("other-window-for-scrolling", Fother_window_for_scrolling, Sother_window_for_scrolling, 0, 0, 0,
doc: /* Return the other window for \"other window scroll\" commands.
-If `other-window-scroll-buffer' is non-nil, a window
-showing that buffer is used.
If in the minibuffer, `minibuffer-scroll-window' if non-nil
-specifies the window. This takes precedence over
-`other-window-scroll-buffer'. */)
+specifies the window.
+Otherwise, if `other-window-scroll-buffer' is non-nil, a window
+showing that buffer is used, popping the buffer up if necessary.
+Finally, look for a neighboring window on the selected frame,
+followed by all visible frames on the current terminal. */)
(void)
{
Lisp_Object window;
@@ -5739,16 +5740,14 @@ specifies the window. This takes precedence over
DEFUN ("scroll-other-window", Fscroll_other_window, Sscroll_other_window, 0, 1, "P",
doc: /* Scroll next window upward ARG lines; or near full screen if no ARG.
A near full screen is `next-screen-context-lines' less than a full screen.
-The next window is the one below the current one; or the one at the top
-if the current one is at the bottom. Negative ARG means scroll downward.
-If ARG is the atom `-', scroll downward by nearly full screen.
-When calling from a program, supply as argument a number, nil, or `-'.
+Negative ARG means scroll downward. If ARG is the atom `-', scroll
+downward by nearly full screen. When calling from a program, supply
+as argument a number, nil, or `-'.
-If `other-window-scroll-buffer' is non-nil, scroll the window
-showing that buffer, popping the buffer up if necessary.
-If in the minibuffer, `minibuffer-scroll-window' if non-nil
-specifies the window to scroll. This takes precedence over
-`other-window-scroll-buffer'. */)
+The next window is usually the one below the current one;
+or the one at the top if the current one is at the bottom.
+It is determined by the function `other-window-for-scrolling',
+which see. */)
(Lisp_Object arg)
{
Lisp_Object window;
--
2.17.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Simplify "other window" bob/eob motion commands --]
[-- Type: text/x-diff, Size: 2215 bytes --]
From 8e465eb0a766c01901618a6308c9c8920698edf1 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Tue, 1 May 2018 16:05:52 +0100
Subject: [PATCH 3/4] Simplify "other window" bob/eob motion commands
* lisp/window.el (beginning-of-buffer-other-window)
(end-of-buffer-other-window): Simplify via with-selected-window.
---
lisp/window.el | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/lisp/window.el b/lisp/window.el
index 8055e5babc..bc2008e1d9 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8939,19 +8939,12 @@ beginning-of-buffer-other-window
Leave mark at previous position.
With arg N, put point N/10 of the way from the true beginning."
(interactive "P")
- (let ((orig-window (selected-window))
- (window (other-window-for-scrolling)))
- ;; We use unwind-protect rather than save-window-excursion
- ;; because the latter would preserve the things we want to change.
- (unwind-protect
- (progn
- (select-window window)
- ;; Set point and mark in that window's buffer.
- (with-no-warnings
- (beginning-of-buffer arg))
- ;; Set point accordingly.
- (recenter '(t)))
- (select-window orig-window))))
+ (with-selected-window (other-window-for-scrolling)
+ ;; Set point and mark in that window's buffer.
+ (with-no-warnings
+ (beginning-of-buffer arg))
+ ;; Set point accordingly.
+ (recenter '(t))))
(defun end-of-buffer-other-window (arg)
"Move point to the end of the buffer in the other window.
@@ -8959,15 +8952,10 @@ end-of-buffer-other-window
With arg N, put point N/10 of the way from the true end."
(interactive "P")
;; See beginning-of-buffer-other-window for comments.
- (let ((orig-window (selected-window))
- (window (other-window-for-scrolling)))
- (unwind-protect
- (progn
- (select-window window)
- (with-no-warnings
- (end-of-buffer arg))
- (recenter '(t)))
- (select-window orig-window))))
+ (with-selected-window (other-window-for-scrolling)
+ (with-no-warnings
+ (end-of-buffer arg))
+ (recenter '(t))))
\f
(defvar mouse-autoselect-window-timer nil
"Timer used by delayed window autoselection.")
--
2.17.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: Rewrite scroll-other-window-down in C --]
[-- Type: text/x-diff, Size: 6553 bytes --]
From 877b5a5690bdfc4bd34516189dd858f66275b75b Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Tue, 1 May 2018 13:48:30 +0100
Subject: [PATCH 4/4] Rewrite scroll-other-window-down in C (bug#30207)
* lisp/window.el (scroll-other-window-down):
Move to src/window.c as Fscroll_other_window_down.
* src/window.c (scroll_command): Generalise for arbitrary windows.
(Fscroll_up, Fscroll_down): Use scroll_command with selected_window.
(Fscroll_other_window, Fscroll_other_window_down):
Rewrite in terms of scroll_command.
(syms_of_window): Add Sscroll_other_window_down.
---
lisp/window.el | 11 -------
src/window.c | 85 ++++++++++++++++++++++++++------------------------
2 files changed, 45 insertions(+), 51 deletions(-)
diff --git a/lisp/window.el b/lisp/window.el
index bc2008e1d9..085e51646f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8923,17 +8923,6 @@ scroll-down-line
(put 'scroll-down-line 'scroll-command t)
\f
-(defun scroll-other-window-down (&optional lines)
- "Scroll the \"other window\" down.
-For more details, see the documentation for `scroll-other-window'."
- (interactive "P")
- (scroll-other-window
- ;; Just invert the argument's meaning.
- ;; We can do that without knowing which window it will be.
- (if (eq lines '-) nil
- (if (null lines) '-
- (- (prefix-numeric-value lines))))))
-
(defun beginning-of-buffer-other-window (arg)
"Move point to the beginning of the buffer in the other window.
Leave mark at previous position.
diff --git a/src/window.c b/src/window.c
index af317674bb..f654d87e14 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5634,35 +5634,54 @@ window_scroll_line_based (Lisp_Object window, int n, bool whole, bool noerror)
}
-/* Scroll selected_window up or down. If N is nil, scroll a
+/* Scroll WINDOW up or down. If N is nil, scroll upward by a
screen-full which is defined as the height of the window minus
- next_screen_context_lines. If N is the symbol `-', scroll.
- DIRECTION may be 1 meaning to scroll down, or -1 meaning to scroll
- up. This is the guts of Fscroll_up and Fscroll_down. */
+ next_screen_context_lines. If N is the symbol `-', scroll downward
+ by a screen-full. DIRECTION may be 1 meaning to scroll down, or -1
+ meaning to scroll up. */
static void
-scroll_command (Lisp_Object n, int direction)
+scroll_command (Lisp_Object window, Lisp_Object n, int direction)
{
+ struct window *w;
+ bool other_window;
ptrdiff_t count = SPECPDL_INDEX ();
eassert (eabs (direction) == 1);
- /* If selected window's buffer isn't current, make it current for
+ w = XWINDOW (window);
+ other_window = ! EQ (window, selected_window);
+
+ /* If given window's buffer isn't current, make it current for
the moment. But don't screw up if window_scroll gets an error. */
- if (XBUFFER (XWINDOW (selected_window)->contents) != current_buffer)
+ if (XBUFFER (w->contents) != current_buffer)
{
record_unwind_protect (save_excursion_restore, save_excursion_save ());
- Fset_buffer (XWINDOW (selected_window)->contents);
+ Fset_buffer (w->contents);
+ }
+
+ if (other_window)
+ {
+ SET_PT_BOTH (marker_position (w->pointm),
+ marker_byte_position (w->pointm));
+ SET_PT_BOTH (marker_position (w->old_pointm),
+ marker_byte_position (w->old_pointm));
}
if (NILP (n))
- window_scroll (selected_window, direction, true, false);
+ window_scroll (window, direction, true, false);
else if (EQ (n, Qminus))
- window_scroll (selected_window, -direction, true, false);
+ window_scroll (window, -direction, true, false);
else
{
n = Fprefix_numeric_value (n);
- window_scroll (selected_window, XINT (n) * direction, false, false);
+ window_scroll (window, XINT (n) * direction, false, false);
+ }
+
+ if (other_window)
+ {
+ set_marker_both (w->pointm, Qnil, PT, PT_BYTE);
+ set_marker_both (w->old_pointm, Qnil, PT, PT_BYTE);
}
unbind_to (count, Qnil);
@@ -5677,7 +5696,7 @@ If ARG is the atom `-', scroll downward by nearly full screen.
When calling from a program, supply as argument a number, nil, or `-'. */)
(Lisp_Object arg)
{
- scroll_command (arg, 1);
+ scroll_command (selected_window, arg, 1);
return Qnil;
}
@@ -5690,7 +5709,7 @@ If ARG is the atom `-', scroll upward by nearly full screen.
When calling from a program, supply as argument a number, nil, or `-'. */)
(Lisp_Object arg)
{
- scroll_command (arg, -1);
+ scroll_command (selected_window, arg, -1);
return Qnil;
}
\f
@@ -5750,36 +5769,21 @@ It is determined by the function `other-window-for-scrolling',
which see. */)
(Lisp_Object arg)
{
- Lisp_Object window;
- struct window *w;
ptrdiff_t count = SPECPDL_INDEX ();
-
- window = Fother_window_for_scrolling ();
- w = XWINDOW (window);
-
- /* Don't screw up if window_scroll gets an error. */
- record_unwind_protect (save_excursion_restore, save_excursion_save ());
-
- Fset_buffer (w->contents);
- SET_PT_BOTH (marker_position (w->pointm), marker_byte_position (w->pointm));
- SET_PT_BOTH (marker_position (w->old_pointm), marker_byte_position (w->old_pointm));
-
- if (NILP (arg))
- window_scroll (window, 1, true, true);
- else if (EQ (arg, Qminus))
- window_scroll (window, -1, true, true);
- else
- {
- if (CONSP (arg))
- arg = XCAR (arg);
- CHECK_NUMBER (arg);
- window_scroll (window, XINT (arg), false, true);
- }
-
- set_marker_both (w->pointm, Qnil, PT, PT_BYTE);
- set_marker_both (w->old_pointm, Qnil, PT, PT_BYTE);
+ scroll_command (Fother_window_for_scrolling (), arg, 1);
unbind_to (count, Qnil);
+ return Qnil;
+}
+DEFUN ("scroll-other-window-down", Fscroll_other_window_down,
+ Sscroll_other_window_down, 0, 1, "P",
+ doc: /* Scroll next window downward ARG lines; or near full screen if no ARG.
+For more details, see the documentation for `scroll-other-window'. */)
+ (Lisp_Object arg)
+{
+ ptrdiff_t count = SPECPDL_INDEX ();
+ scroll_command (Fother_window_for_scrolling (), arg, -1);
+ unbind_to (count, Qnil);
return Qnil;
}
\f
@@ -7831,6 +7835,7 @@ displayed after a scrolling operation to be somewhat inaccurate. */);
defsubr (&Sscroll_right);
defsubr (&Sother_window_for_scrolling);
defsubr (&Sscroll_other_window);
+ defsubr (&Sscroll_other_window_down);
defsubr (&Sminibuffer_selected_window);
defsubr (&Srecenter);
defsubr (&Swindow_text_width);
--
2.17.0
[-- Attachment #5: Type: text/plain, Size: 1506 bytes --]
I'm sorry about the long delay in getting back to this (SSD with
unpushed changes was TRIMmed by Windows partition updates a few months
ago; I got distracted by other things...). Attached are four patches.
The first avoids scrolling the phantom daemon's window by limiting the
next-window search in Fother_window_for_scrolling to visible frames on
the current terminal, as per the solution to bug#3442. So far, everyone
seems to agree this is the reasonable thing to do here as well.
The second extends the Emacs and Elisp manuals to document
scroll-other-window-down and tries to make the docstrings of
Fother_window_for_scrolling and Fscroll_other_window clearer and more
accurate.
The third tries to simplify beginning-of-buffer-other-window and
end-of-buffer-other-window by using the macro with-selected-window.
The fourth tries to reduce code duplication in src/window.c by
generalising scroll_command to suit all three commands Fscroll_up,
Fscroll_down, and Fscroll_other_window. Doing this made it clear that
scroll-other-window-down could also be written in terms of
scroll_command for simplicity, so this patch moves the former from
lisp/window.el to src/window.c. The patch does not move the command's
corresponding binding from lisp/bindings.el to keys_of_window in
src/window.c because it doesn't look like any of the keys_of_* functions
can (or do) make use of the shift modifier.
Let me know how the patches can be improved,
should these proposals be accepted.
Thanks,
--
Basil
next prev parent reply other threads:[~2018-05-02 18:57 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-22 15:57 bug#30207: 27.0.50; [PATCH] other-window-for-scrolling returns window on daemon frame Basil L. Contovounesios
2018-01-22 18:59 ` martin rudalics
2018-01-23 0:07 ` Basil L. Contovounesios
2018-01-23 0:29 ` Noam Postavsky
2018-01-23 1:06 ` Basil L. Contovounesios
2018-01-23 1:29 ` Noam Postavsky
2018-01-23 13:28 ` Basil L. Contovounesios
2018-05-02 18:57 ` Basil L. Contovounesios [this message]
2018-05-02 19:44 ` Eli Zaretskii
2018-05-03 13:03 ` Basil L. Contovounesios
2018-05-03 19:00 ` Eli Zaretskii
2018-05-03 22:44 ` Basil L. Contovounesios
2018-05-10 23:48 ` Noam Postavsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zi1h6g20.fsf@tcd.ie \
--to=contovob@tcd.ie \
--cc=30207@debbugs.gnu.org \
--cc=npostavs@users.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.