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 21:03:55 +0100 Message-ID: <87fua91vis.fsf@gmail.com> References: <87infjm3p3.fsf@gmail.com> <871slyi3lk.fsf_-_@gmail.com> <87lgk22ryu.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1508789125 18280 195.159.176.226 (23 Oct 2017 20:05:25 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 23 Oct 2017 20:05:25 +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 22:05:20 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 1e6iyB-0002R5-A6 for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Oct 2017 22:05:07 +0200 Original-Received: from localhost ([::1]:40351 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e6iyI-0007u9-LI for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Oct 2017 16:05:14 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44676) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e6iyA-0007ty-2a for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 16:05:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e6iy6-00085b-PG for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 16:05:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:49354) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e6iy6-00084m-JI for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 16:05:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e6iy6-0006rF-5P for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2017 16:05:02 -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 20:05:02 +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.150878904826290 (code B ref 28814); Mon, 23 Oct 2017 20:05:02 +0000 Original-Received: (at 28814) by debbugs.gnu.org; 23 Oct 2017 20:04:08 +0000 Original-Received: from localhost ([127.0.0.1]:58035 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e6ixD-0006px-9S for submit@debbugs.gnu.org; Mon, 23 Oct 2017 16:04:08 -0400 Original-Received: from mail-wr0-f172.google.com ([209.85.128.172]:56300) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e6ixA-0006pR-Jn for 28814@debbugs.gnu.org; Mon, 23 Oct 2017 16:04:05 -0400 Original-Received: by mail-wr0-f172.google.com with SMTP id l8so5711320wre.12 for <28814@debbugs.gnu.org>; Mon, 23 Oct 2017 13:04:04 -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=15e5XEV9p9vhUBtyDenLe0Z9ptpKwyAs0k68T9IW/yA=; b=j425lr4CvU9Ebb5wt2zBKOIdru5SKZzEm4rDtg+FQ2VG7s5O3SR4CYn9UHJFt4CPfC vH180oGitGEMuHhfXN2UH+kyaJu1LW07ZUP3ro+Rj8XJ82+4HCUCZPlCneflyJFiXQZG N5mggKm7+AOcvVS+uM+ajk9jcIXKUq8USFOcG9aLDJlzNMknKt2G9/WmGaT0B5Is0wlJ U8JoDTno/enLp6G0OnVhJAruQawA9RecfAi5M3fnLzbgeb3GYmZ/2EM3ScT5hih8EI2G GIN1qTtBiI4woDjhvmap+Ko6c6WKkLjvMu70rFI8ZW+7lmuI+uihKpFKm5amvpVaMqIz TQDQ== 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=15e5XEV9p9vhUBtyDenLe0Z9ptpKwyAs0k68T9IW/yA=; b=OXA9XAaIqc+3b8zhMlnq0mSbO/GZE7ScqvrHunIzK1WiEFf6dx5/OvQZr7Eri28TjW AoJqgQYAz+nGjO+EeZe1UUCpT4G1QVr+BFxEAqJh6dCwSl+lo2RFL+JItahBOvYgPhVi E15A4oGqucq1Ac+Hsy1ycdmixilbtFK4s7Y9VuOE1NsmgLwjIK/33JZFCDVxI2eyE6M4 ZUbOBewdBNw+1Tq+XpOVlAwmt3lQSA7Dbdwt8gamOPBsgbGU23zlpsCT7ST4gytJ3cpR LkS5q737JTS1Ps2Y6GCNOJK6UOPLPRLbjHL+2b9vKYmcshfpezOk+qOGfd3NsK3GdGO7 4H1Q== X-Gm-Message-State: AMCzsaUtxOVkHVDuNGVetr+Ee6q0wyXZXx5frSaVc0QC9kMd29u1ET4K fUMOmc7UNUFOZSMRRw8zA+o= X-Google-Smtp-Source: ABhQp+TsMdLGUi6rDBXPcIYbhfxpFGdknwPrD4n7Mm7rVAoNv7Q3xaqOuwOEc8YN1g09dla1NEzpow== X-Received: by 10.223.135.197 with SMTP id c5mr10879239wrc.183.1508789038912; Mon, 23 Oct 2017 13:03:58 -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 f27sm16907282wrf.63.2017.10.23.13.03.57 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 23 Oct 2017 13:03:57 -0700 (PDT) In-Reply-To: <87lgk22ryu.fsf@gmail.com> ("=?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?="'s message of "Mon, 23 Oct 2017 09:23:05 +0100") 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:138909 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Dmitry, Eli, I read the discussion you pointed me to me by Dmitry in http://lists.gnu.org/archive/html/emacs-devel/2016-01/msg01235.html. Eli, If I understand your concerns there, then the first and third patches I proposed shouldn't in any way interfere with your use of *xref*-related facilities. If anything, they should improve it. The second patch does indeed bring the problem known as "the disappearing *xref* problem", so I mended that with a very simple fourth patch, described below. So now, to summarize, there are 4 patches in total, that I reattach: * 0001-Honor-window-switching-intents-in-xref-find-definiti.patch This fixes the problem with the non-deterministic behaviour of selecting a window for displaying cross-references, as well the problem where the initial window/frame switching intent is lost. * 0002-Quit-the-xref-window-if-user-decides-to-go-to-a-ref.patch This makes the xref buffer non-obstrusive by quitting it on xref-goto-xref. This is a change to a current behaviour that was specifically requested by Eli (the "the disappearing *xref* problem"). * 0003-Allow-split-window-sensibly-to-split-threshold-in-fu.patch This extends the exception granted by split-window-sensibly to single-window frames whose dimensions are below those of splitting thresholds to consider multi-window frames where all but one window is dedicated. In practice, it fixes the case where C-x 4 . xref-backend-definitions RET n would surprisingly pop-up a new frame if the original frame was already small to start with. This fix to window.el appears very sound to me, but if it is not desired for whatever reason, a more localized fix in xref.el is also possible. * 0004-Don-t-quit-xref-window-on-RET-only-on-C-u-RET.patch This fixes the "disappearing *xref* problem", by bringing back the default behaviour of not quitting the *xref* window on RET, although allowing for that if the user types C-u RET instead. Jo=C3=A3o --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Honor-window-switching-intents-in-xref-find-definiti.patch >From 1b760816ac8886313996c635ee1cf696b937c20e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Fri, 13 Oct 2017 15:13:14 +0100 Subject: [PATCH 1/4] Honor window-switching intents in xref-find-definitions When there is more than one xref to jump to, and an *xref* window appears to help the user choose, the original intent to open a definition another window or frame is remembered when the choice to go to or show a reference is finally made. * lisp/progmodes/xref.el (xref--show-pos-in-buf): Rewrite. (xref--original-window-intent): New variable. (xref--original-window): Rename from xref--window and move up here for clarity. (xref--show-pos-in-buf): Rewrite. Don't take SELECT arg here. (xref--show-location): Handle window selection decision here. (xref--window): Rename to xref--original-window. (xref-show-location-at-point): Don't attempt window management here. (xref--show-xrefs): Ensure display-action intent is saved. --- lisp/progmodes/xref.el | 69 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index 80cdcb3f18..c3e7982205 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -449,43 +449,68 @@ xref--with-dedicated-window (when xref-w (set-window-dedicated-p xref-w xref-w-dedicated))))) -(defun xref--show-pos-in-buf (pos buf select) - (let ((xref-buf (current-buffer)) - win) +(defvar-local xref--original-window-intent nil + "Original window-switching intent before xref buffer creation.") + +(defvar-local xref--original-window nil + "The original window this xref buffer was created from.") + +(defun xref--show-pos-in-buf (pos buf) + "Goto and display position POS of buffer BUF in a window. +Honour `xref--original-window-intent', run `xref-after-jump-hook' +and finally return the window." + (let* ((xref-buf (current-buffer)) + (pop-up-frames + (or (eq xref--original-window-intent 'frame) + pop-up-frames)) + (action + (cond ((memq + xref--original-window-intent + '(window frame)) + t) + ((and + (window-live-p xref--original-window) + (or (not (window-dedicated-p xref--original-window)) + (eq (window-buffer xref--original-window) buf))) + `(,(lambda (buf _alist) + (set-window-buffer xref--original-window buf) + xref--original-window)))))) (with-selected-window - (xref--with-dedicated-window - (display-buffer buf)) + (with-selected-window + ;; Just before `display-buffer', place ourselves in the + ;; original window to suggest preserving it. Of course, if + ;; user has deleted the original window, all bets are off, + ;; just use the selected one. + (or (and (window-live-p xref--original-window) + xref--original-window) + (selected-window)) + (display-buffer buf action)) (xref--goto-char pos) (run-hooks 'xref-after-jump-hook) (let ((buf (current-buffer))) - (setq win (selected-window)) (with-current-buffer xref-buf - (setq-local other-window-scroll-buffer buf)))) - (when select - (select-window win)))) + (setq-local other-window-scroll-buffer buf))) + (selected-window)))) (defun xref--show-location (location &optional select) (condition-case err (let* ((marker (xref-location-marker location)) (buf (marker-buffer marker))) - (xref--show-pos-in-buf marker buf select)) + (cond (select + (select-window (xref--show-pos-in-buf marker buf))) + (t + (save-selected-window + (xref--with-dedicated-window + (xref--show-pos-in-buf marker buf)))))) (user-error (message (error-message-string err))))) -(defvar-local xref--window nil - "The original window this xref buffer was created from.") - (defun xref-show-location-at-point () "Display the source of xref at point in the appropriate window, if any." (interactive) (let* ((xref (xref--item-at-point)) (xref--current-item xref)) (when xref - ;; Try to avoid the window the current xref buffer was - ;; originally created from. - (if (window-live-p xref--window) - (with-selected-window xref--window - (xref--show-location (xref-item-location xref))) - (xref--show-location (xref-item-location xref)))))) + (xref--show-location (xref-item-location xref))))) (defun xref-next-line () "Move to the next xref and display its source in the appropriate window." @@ -727,7 +752,8 @@ xref--show-xref-buffer (xref--xref-buffer-mode) (pop-to-buffer (current-buffer)) (goto-char (point-min)) - (setq xref--window (assoc-default 'window alist)) + (setq xref--original-window (assoc-default 'window alist) + xref--original-window-intent (assoc-default 'display-action alist)) (current-buffer))))) @@ -754,7 +780,8 @@ xref--show-xrefs (t (xref-push-marker-stack) (funcall xref-show-xrefs-function xrefs - `((window . ,(selected-window))))))) + `((window . ,(selected-window)) + (display-action . ,display-action)))))) (defun xref--prompt-p (command) (or (eq xref-prompt-for-identifier t) -- 2.14.2 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-Quit-the-xref-window-if-user-decides-to-go-to-a-ref.patch >From 9feaf473479dcd8edcf2c0bb14044995abb5eeb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Fri, 13 Oct 2017 16:37:47 +0100 Subject: [PATCH 2/4] Quit the *xref* window if user decides to go to a ref The idea is to have this window, whose sudden appearance is hard to predict, obtrude as little as possible on the user's window configuration. * lisp/progmodes/xref.el (xref--show-location): When SELECT is t, quit window. --- lisp/progmodes/xref.el | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index c3e7982205..cf9e027ba0 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -495,9 +495,12 @@ xref--show-pos-in-buf (defun xref--show-location (location &optional select) (condition-case err (let* ((marker (xref-location-marker location)) - (buf (marker-buffer marker))) + (buf (marker-buffer marker)) + (xref-buffer (current-buffer))) (cond (select - (select-window (xref--show-pos-in-buf marker buf))) + (quit-window nil nil) + (with-current-buffer xref-buffer + (select-window (xref--show-pos-in-buf marker buf)))) (t (save-selected-window (xref--with-dedicated-window -- 2.14.2 --=-=-= 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/4] 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 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0004-Don-t-quit-xref-window-on-RET-only-on-C-u-RET.patch >From 1b527b10ad44ee4863e87700fb50dcfda14c72f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Mon, 23 Oct 2017 20:51:54 +0100 Subject: [PATCH 4/4] Don't quit *xref* window on RET, only on C-u RET * lisp/progmodes/xref.el (xref--show-location): Handle new 'quit value for SELECT. (xref-goto-xref): Allow prefix argument. --- lisp/progmodes/xref.el | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index cf9e027ba0..9dc78397eb 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -493,12 +493,15 @@ xref--show-pos-in-buf (selected-window)))) (defun xref--show-location (location &optional select) + "Helper for `xref-show-xref' and `xref-goto-xref'. +Go to LOCATION and if SELECT is non-nil select its window. If +SELECT is `quit', also quit the *xref* window." (condition-case err (let* ((marker (xref-location-marker location)) (buf (marker-buffer marker)) (xref-buffer (current-buffer))) (cond (select - (quit-window nil nil) + (if (eq select 'quit) (quit-window nil nil)) (with-current-buffer xref-buffer (select-window (xref--show-pos-in-buf marker buf)))) (t @@ -532,12 +535,13 @@ xref--item-at-point (back-to-indentation) (get-text-property (point) 'xref-item))) -(defun xref-goto-xref () - "Jump to the xref on the current line and select its window." - (interactive) +(defun xref-goto-xref (&optional quit) + "Jump to the xref on the current line and select its window. +With QUIT (the prefix argument) also quit the *xref* window." + (interactive "P") (let ((xref (or (xref--item-at-point) (user-error "No reference at point")))) - (xref--show-location (xref-item-location xref) t))) + (xref--show-location (xref-item-location xref) (if quit 'quit t)))) (defun xref-query-replace-in-results (from to) "Perform interactive replacement of FROM with TO in all displayed xrefs. -- 2.14.2 --=-=-=--