From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: joaotavora@gmail.com (=?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?=) Newsgroups: gmane.emacs.bugs Subject: bug#28814: [BUMP, PATCH] (26.0.90; When *xref* window is needed, original window-switching intent is lost ) Date: Mon, 23 Oct 2017 09:23:05 +0100 Message-ID: <87lgk22ryu.fsf@gmail.com> References: <87infjm3p3.fsf@gmail.com> <871slyi3lk.fsf_-_@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1508747057 19714 195.159.176.226 (23 Oct 2017 08:24:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 23 Oct 2017 08:24:17 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.90 (gnu/linux) Cc: 28814@debbugs.gnu.org To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Oct 23 10:24:12 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e6Y1o-0003jU-HG for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Oct 2017 10:24:09 +0200 Original-Received: from localhost ([::1]:37457 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e6Y1t-0007FG-G8 for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Oct 2017 04:24:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53475) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e6Y1n-0007Ez-05 for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 04:24:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e6Y1h-0000w4-Ux for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 04:24:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:47910) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e6Y1h-0000w0-Oc for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 04:24:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e6Y1h-0000Zj-I8 for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 04:24:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: joaotavora@gmail.com (=?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 23 Oct 2017 08:24:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 28814 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 28814-submit@debbugs.gnu.org id=B28814.15087469972149 (code B ref 28814); Mon, 23 Oct 2017 08:24:01 +0000 Original-Received: (at 28814) by debbugs.gnu.org; 23 Oct 2017 08:23:17 +0000 Original-Received: from localhost ([127.0.0.1]:56591 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e6Y0z-0000Ya-33 for submit@debbugs.gnu.org; Mon, 23 Oct 2017 04:23:17 -0400 Original-Received: from mail-wm0-f45.google.com ([74.125.82.45]:56167) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e6Y0y-0000YM-0S for 28814@debbugs.gnu.org; Mon, 23 Oct 2017 04:23:16 -0400 Original-Received: by mail-wm0-f45.google.com with SMTP id u138so8127491wmu.4 for <28814@debbugs.gnu.org>; Mon, 23 Oct 2017 01:23:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=72Qu1lAwjPqMVGsFdmftYH5jYQJJ3mVR9yU/g+x+njo=; b=mbzEuohrBKHgO9wynBWBiaT7SujMtIZTjXsYdnWbBjeD0SALtVwuUtEMP6oHsE6hTf c6qsV0PFVGCfE7wczXfIrG1lPJW5QhtGMpi2zFpSRYZWNk+XZRv9hzqayCjxTeUYgihd HNN4XumuH9fyHR5vnnRUHNj7Ax1uAqyIxifk0NPlNO618ILbwGLpB3EtMrXicez2u5jM 4sV9gBXF3fMWLIKAMefw0H1LETbeHwEAh6Dbn6k8g4Tt4facur0TwjDiT/aZw/6SvxqS WaxvaQ9/WtiFBwkGx47+4ft54TGs89HzFndXpaTRUWau+yhbKoBgn3NL+l8qzTcMLDH8 bFwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=72Qu1lAwjPqMVGsFdmftYH5jYQJJ3mVR9yU/g+x+njo=; b=ShSCoWzheREcilasbS/M8UVTGrjA7WCCfAp81i/OGZ3npqTpyhiINyh394ZW8iFF/0 Ugze3HN3EIKmwVJeOvHj/eQvx8ml5KiHZF0CxDEJLJrECt5KDDMQor3WHCE3584v9TAd AeM8NjhvicRIT5LOsC3V8CKxm/txkLOMzIvCW3waOjQSYO2WeA3WTy6rFNo3H++59xFM 9vQxx5RyVxkHkijkPcv0t0+oagofyVa6jHTAjz8lRs61M3Mq/VF8PfxgQmvJa5CSH28w +xY00+UvqVKy3IZ9IVXVzipBhgir9PQZGDtpoMeVAa7TXPfK/G+u7Vq38CArfXlVeGxN EjOw== X-Gm-Message-State: AMCzsaVVt2Ht9BDlU8cSNU8jLtNDpyi9ByUaVmhJRnxQ9pZxoNoTbAWO lEtnY+mJnv9IdN3anuMPcYE4/kwB X-Google-Smtp-Source: ABhQp+Q9LEeT3czXkSb/g17NILX516a5QJP/tUj5mEx7RQ1rtivqurCz0QgnfShM+70ZjnqYdcekuQ== X-Received: by 10.28.17.7 with SMTP id 7mr4427921wmr.66.1508746989844; Mon, 23 Oct 2017 01:23:09 -0700 (PDT) Original-Received: from lolita.yourcompany.com (188.139.62.94.rev.vodafone.pt. [94.62.139.188]) by smtp.gmail.com with ESMTPSA id o11sm5320151wrg.5.2017.10.23.01.23.08 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 23 Oct 2017 01:23:08 -0700 (PDT) In-Reply-To: (Dmitry Gutov's message of "Mon, 23 Oct 2017 05:06:37 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:138880 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Dmitry Gutov writes: > You are working on something that I agree is a real problem, but doing > this would effectively revert commit > 5698947ff9335cf150c73cca257a5e7e5951b045, which was based on a > previous discussion (see the link in the commit message), and in > particular, would bring back "disappearing *xref* buffer problem", as > Eli put it. > [...] > > I have a rough idea on how to fix the situation, but nothing even > close to an implementation. > Thanks for the pointer. After I read that discussion I will probably ask you about that. >> emacs -Q >> C-x 3 [split-window-right] >> C-x 2 [split-window-below] >> M-. xref-backend-definitions RET [xref-find-definitions] >> C-n [next-line] >> RET [xref-goto-xref] >> >> Expected the definition to be found in the original window where I >> pressed M-. but instead it was found in another. Another case: >> >> emacs -Q >> C-x 4 . xref-backend-definitions RET [xref-find-definitions-other-wi= ndow] >> C-n >> RET >> >> Expected the definition to be found in some other window, different from >> the one I pressed M-. on. Instead went to the same one. > > With your patches applied, this example pops a new frame for me if I > press 'n' instead of 'C-n'. This is not hugely important in the light > of the larger problems above, but demonstrated difficulties with > window management when we try to pretend that the xref buffer "was > never there". You are absolutely right (and you don't have to tell me how hairy window-management code is). This particular problem, which I had also noticed, slipped through. I have a fix attached as a patch.=20 The cause of this problem is that split-window-sensibly refuses to split a window whose dimensions are below those of split-height-threshold and split-width-threshold. The reason you don't see frames popping up every time you do C-x 4 b on a small frame is that this function contains a safety net for these cases: if the window to be split is the only one available in the frame, it disregards the dimension threshholds and splits anyway. The attached window.el patch is a correct way to generalize this to account for dedicated windows. If this is not accepted this local fix in xref.el will also work fine. @@ -504,7 +504,8 @@ xref--show-location (t (save-selected-window (xref--with-dedicated-window - (xref--show-pos-in-buf marker buf)))))) + (let ((split-height-threshold 0)) + (xref--show-pos-in-buf marker buf))))))) (user-error (message (error-message-string err))))) =20=20=20=20 (defun xref-show-location-at-point () >> Also, in both >> situations, expected the window configuration to be the same as if I had >> searched for, say xref-backend-functions. >> >> This is fixed by the patches that I reattach after minor tweaks. The >> general idea is to have the *xref*, whose sudden appearance is hard to >> predict, obtrude as little as possible on the user's window >> configuration.p > > If we don't bring back the "disappearing *xref* buffer problem", > *xref* has to stay obtrusive. Do you think the rest of your patch will > make sense with that change? I see and I will try to answer. I proposed two patches previously: * a first one to fix the non-determinism of window popping/selecting behaviour; * a second one to make the *xref* buffer less obstrusive. * (now there is the third one that fixes the frame-popping glitch) IIUC it is the second one that clashes with "the dissapearing *xref* problem" that I have yet to read up on. If we don't come up with a solution for that, I would be OK with a solution that leaves it unsolved but adds some customization point (hook) for the user to put this behaviour in. Jo=C3=A3o --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0003-Allow-split-window-sensibly-to-split-threshold-in-fu.patch >From 47480926f328db5d840ba518125e0866d29f8665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Mon, 23 Oct 2017 09:05:32 +0100 Subject: [PATCH 3/3] Allow split-window-sensibly to split threshold in further edge case As a fallback, and to avoid creating a frame, split-window-sensibly would previously disregard split-height-threshold if the window to be split is the frame's root window. This change slightly expands on that: it disregards the threshold if the window to be split is the frame's only usable window (it is either the only one, as before, or all the other windows are dedicated to some buffer and thus cannot be touched). * lisp/window.el (split-height-threshold): Adjust doc to match split-window-sensibly. (split-window-sensibly): Also disregard threshold if all other windows are dedicated. --- lisp/window.el | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lisp/window.el b/lisp/window.el index 5ba9a305f9..21271944c0 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -6449,8 +6449,9 @@ split-height-threshold vertically only if it has at least this many lines. If this is nil, `split-window-sensibly' is not allowed to split a window vertically. If, however, a window is the only window on its -frame, `split-window-sensibly' may split it vertically -disregarding the value of this variable." +frame, or all the other ones are dedicated, +`split-window-sensibly' may split it vertically disregarding the +value of this variable." :type '(choice (const nil) (integer :tag "lines")) :version "23.1" :group 'windows) @@ -6557,15 +6558,25 @@ split-window-sensibly ;; Split window horizontally. (with-selected-window window (split-window-right))) - (and (eq window (frame-root-window (window-frame window))) - (not (window-minibuffer-p window)) - ;; If WINDOW is the only window on its frame and is not the - ;; minibuffer window, try to split it vertically disregarding - ;; the value of `split-height-threshold'. - (let ((split-height-threshold 0)) - (when (window-splittable-p window) - (with-selected-window window - (split-window-below)))))))) + (and + (let ((frame (window-frame window))) + (or + (eq window (frame-root-window frame)) + (let ((windows (delete window (window-list frame))) + w) + (while (and (setq w (pop windows)) + (window-dedicated-p w))) + (not windows)))) + (not (window-minibuffer-p window)) + ;; If WINDOW is the only usable window on its frame (it is + ;; the only one or,not being the only one, all the other ones + ;; are dedicated) and is not the minibuffer window, try to + ;; split it vertically disregarding the value of + ;; `split-height-threshold'. + (let ((split-height-threshold 0)) + (when (window-splittable-p window) + (with-selected-window window + (split-window-below)))))))) (defun window--try-to-split-window (window &optional alist) "Try to split WINDOW. -- 2.14.2 --=-=-=--