From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: "... the window start at a meaningless point within a line." Date: Sat, 31 Oct 2015 15:21:02 +0200 Message-ID: <8337wryy0x.fsf@gnu.org> References: <83twpp51xz.fsf@gnu.org> <20151017115738.GA2522@acm.fritz.box> <83oafx4qsb.fsf@gnu.org> <83lhb14o6e.fsf@gnu.org> <83k2ql4lsy.fsf@gnu.org> <20151018150052.GD1639@acm.fritz.box> <83lhb0hy0h.fsf@gnu.org> <20151019102755.GB2438@acm.fritz.box> <20151027134025.GA2401@acm.fritz.box> <83fv0w41dg.fsf@gnu.org> <20151028131505.GB2538@acm.fritz.box> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1446297687 24381 80.91.229.3 (31 Oct 2015 13:21:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 31 Oct 2015 13:21:27 +0000 (UTC) Cc: emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Oct 31 14:21:18 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZsW5s-0003b3-Fi for ged-emacs-devel@m.gmane.org; Sat, 31 Oct 2015 14:21:16 +0100 Original-Received: from localhost ([::1]:55641 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZsW5r-00020G-Dg for ged-emacs-devel@m.gmane.org; Sat, 31 Oct 2015 09:21:15 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZsW5e-000201-1X for emacs-devel@gnu.org; Sat, 31 Oct 2015 09:21:03 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZsW5a-0004rB-1U for emacs-devel@gnu.org; Sat, 31 Oct 2015 09:21:02 -0400 Original-Received: from mtaout28.012.net.il ([80.179.55.184]:53039) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZsW5Z-0004qu-Jj for emacs-devel@gnu.org; Sat, 31 Oct 2015 09:20:57 -0400 Original-Received: from conversion-daemon.mtaout28.012.net.il by mtaout28.012.net.il (HyperSendmail v2007.08) id <0NX300300619CO00@mtaout28.012.net.il> for emacs-devel@gnu.org; Sat, 31 Oct 2015 15:19:57 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([84.94.185.246]) by mtaout28.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NX3001IM6D9Y720@mtaout28.012.net.il>; Sat, 31 Oct 2015 15:19:57 +0200 (IST) In-reply-to: <20151028131505.GB2538@acm.fritz.box> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 80.179.55.184 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:193016 Archived-At: > Date: Wed, 28 Oct 2015 13:15:05 +0000 > Cc: emacs-devel@gnu.org > From: Alan Mackenzie > > After applying the patch, to test things, in X (?or windows) > o - Start emacs -Q. > o - Load utilities.el. > o - Visit fragment.el. > o - Using the mouse, make the frame wide enough for two side by side > windows of width 79 and 80. > o - C-x 3. > o - Fiddle about with the mouse, M-{, M-}, until the windows are 79, and > 80 wide (`window-body-width' is your friend, here). > o - Scroll the buffer until L162 straddles window 1 and window 2, with > two display lines in each window. (S-/, C-S-/ > from utilies.el are handy, here). > > First of all, note that in L162, all characters are displayed exactly > once. This is the purpose of this change. > > With point in W2, note that C- doesn't go to the beginning of > the window, and C- doesn't go to the end. The actual end > positions are a line out in both cases. With point in the split line, > note that vertical-motion (C-u C-c m) doesn't go to the right > places, even when the target line is below the split line. Typing > and also produce interesting effects. > > Using M-{ or M-}, make W1 80 wide, and W2 79 wide. Scroll the windows > up and down, back to the same place, to ensure that Follow Mode has > resynchronised its windows[*]. Now repeat the experiments of the > previous paragraph. If anything, the results now are even worse. The patch below should go a long way towards solving these problems. The only issue I left unsolved is exposed when typing C-p on the second screen line of the right-side window: you will see that it doesn't succeed in keeping the visual column. Solving this is left as an exercise ;-) I hope the changes I made and the comments to those changes will speak for themselves, and show how to solve such problems. Please note that there's a very serious underlying problem here: the move_it_* functions and all their callers assume that moving beyond above or below the window keeps the same window dimensions, in particularly its width. Follow mode violates this assumption, because moving from the left-side to the right-side window or vice versa can change the width. Accounting for this would need very deep changes in the above mentioned functions. For now, I avoided that, and the patch below is just a band-aid. If it works well enough in practice, maybe we can avoid the surgery. But in general, I must say that fixing what you are trying to fix is an ambitious undertaking; perhaps it would be better to make sure the two windows are always the same width, using pixel-wise resizing where necessary. diff --git a/src/indent.c b/src/indent.c index 32597bb..993ef5f 100644 --- a/src/indent.c +++ b/src/indent.c @@ -2034,6 +2034,7 @@ whether or not it is currently displayed in some window. */) else { ptrdiff_t it_start, it_overshoot_count = 0; + ptrdiff_t w_start = marker_position (w->start); int first_x; bool overshoot_handled = 0; bool disp_string_at_start_p = 0; @@ -2043,6 +2044,8 @@ whether or not it is currently displayed in some window. */) int start_x IF_LINT (= 0); int to_x = -1; struct text_pos xdisp_ws; + bool on_window_border = false; + ptrdiff_t goal_pt = PT; bool start_x_given = !NILP (cur_col); @@ -2102,27 +2105,40 @@ whether or not it is currently displayed in some window. */) do this, we start moving with IT->current_x == 0, while PT is really at some x > 0. */ reseat_at_previous_visible_line_start (&it); + /* If we are in a right-side window under follow-mode, and + previous visible line start is before that window's + start, reseat to the window's start point instead, where + we know the horizontal coordinates are zero. This avoids + problems in iterator metrics when the 2 windows under + follow-mode have different widths. */ + if (w->exact_start + && IT_CHARPOS (it) < w_start) + { + on_window_border = true; + reseat_at_window_start (&it); + } it.current_x = it.hpos = 0; } if (IT_CHARPOS (it) != PT) - /* We used to temporarily disable selective display here; the - comment said this is "so we don't move too far" (2005-01-19 - checkin by kfs). But this does nothing useful that I can - tell, and it causes Bug#2694 . -- cyd */ - /* When the position we started from is covered by a display - string, move_it_to will overshoot it, while vertical-motion - wants to put the cursor _before_ the display string. So in - that case, we move to buffer position before the display - string, and avoid overshooting. But if the position before - the display string is a newline, we don't do this, because - otherwise we will end up in a screen line that is one too - far back. */ - move_it_to (&it, - (!disp_string_at_start_p - || FETCH_BYTE (IT_BYTEPOS (it)) == '\n') - ? PT - : PT - 1, - -1, -1, -1, MOVE_TO_POS); + { + /* We used to temporarily disable selective display here; the + comment said this is "so we don't move too far" (2005-01-19 + checkin by kfs). But this does nothing useful that I can + tell, and it causes Bug#2694 . -- cyd */ + /* When the position we started from is covered by a display + string, move_it_to will overshoot it, while vertical-motion + wants to put the cursor _before_ the display string. So in + that case, we move to buffer position before the display + string, and avoid overshooting. But if the position before + the display string is a newline, we don't do this, because + otherwise we will end up in a screen line that is one too + far back. */ + goal_pt = + (!disp_string_at_start_p || FETCH_BYTE (IT_BYTEPOS (it)) == '\n') + ? PT + : PT - 1; + move_it_to (&it, goal_pt, -1, -1, -1, MOVE_TO_POS); + } /* IT may move too far if truncate-lines is on and PT lies beyond the right margin. IT may also move too far if the @@ -2176,7 +2192,9 @@ whether or not it is currently displayed in some window. */) /* Do this even if LINES is 0, so that we move back to the beginning of the current line as we ought. */ if ((nlines < 0 && IT_CHARPOS (it) > 0) - || (nlines == 0 && !(start_x_given && start_x <= to_x))) + || (nlines == 0 + && !(start_x_given && start_x <= to_x) + && !on_window_border)) move_it_by_lines (&it, max (PTRDIFF_MIN, nlines)); } else if (overshoot_handled) @@ -2212,6 +2230,35 @@ whether or not it is currently displayed in some window. */) } } + /* If we moved up in the right-side window under follow-mode, + and found ourselves beyond the window's start, we must move + back from window start to PT, see how many lines is that, + then switch to the left-side window, and move back the rest + of NLINES starting from that window's last line. That's + because move_it_* functions use the window dimensions, which + might be different in the other window. */ + if (w->exact_start && IT_CHARPOS (it) < w_start) + { + int vpos = it.vpos, nlines_prev_window; + Lisp_Object prev_window = w->prev; + struct window *pw; + ptrdiff_t prev_window_endpos; + struct text_pos pw_top; + + eassert (WINDOW_LIVE_P (prev_window)); + reseat_at_window_start (&it); + it.current_x = it.hpos = it.vpos = 0; + move_it_to (&it, goal_pt, -1, -1, -1, MOVE_TO_POS); + nlines_prev_window = nlines + it.vpos + 1; + pw = XWINDOW (prev_window); + SET_TEXT_POS_FROM_MARKER (pw_top, pw->start); + start_display (&it, pw, pw_top); + move_it_to (&it, -1, -1, it.last_visible_y - 1, -1, MOVE_TO_Y); + move_it_by_lines (&it, nlines_prev_window); + /* Pretend the original move worked as intended. */ + it.vpos = vpos; + } + /* Move to the goal column, if one was specified. If the window was originally hscrolled, the goal column is interpreted as an addition to the hscroll amount. */ diff --git a/src/window.c b/src/window.c index b922511..38aadd6 100644 --- a/src/window.c +++ b/src/window.c @@ -7149,7 +7149,7 @@ and scrolling positions. */) DEFUN ("window-test-dump", Fwindow_test_dump, Swindow_test_dump, 0, 0, "", doc: /* Dump some critical components of the selected window to `message'.*/) - () + (void) { Lisp_Object window = Fselected_window (); struct window *w = decode_live_window (window);