From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Kelly Dean Newsgroups: gmane.emacs.devel Subject: [PATCH] Push mark before jumping to new location for xref Date: Tue, 17 Feb 2015 02:21:48 +0000 Message-ID: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1424139807 29760 80.91.229.3 (17 Feb 2015 02:23:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 17 Feb 2015 02:23:27 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Feb 17 03:23: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 1YNXoj-0005eI-Nb for ged-emacs-devel@m.gmane.org; Tue, 17 Feb 2015 03:23:17 +0100 Original-Received: from localhost ([::1]:43534 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNXoi-0003UU-Pq for ged-emacs-devel@m.gmane.org; Mon, 16 Feb 2015 21:23:16 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:40970) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNXoe-0003U1-Ll for emacs-devel@gnu.org; Mon, 16 Feb 2015 21:23:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNXob-0003Aa-Cc for emacs-devel@gnu.org; Mon, 16 Feb 2015 21:23:12 -0500 Original-Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:33636) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNXob-0003AO-4A for emacs-devel@gnu.org; Mon, 16 Feb 2015 21:23:09 -0500 Original-Received: from mfilter10-d.gandi.net (mfilter10-d.gandi.net [217.70.178.139]) by relay6-d.mail.gandi.net (Postfix) with ESMTP id 44EA6FB87D for ; Tue, 17 Feb 2015 03:23:08 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mfilter10-d.gandi.net Original-Received: from relay6-d.mail.gandi.net ([217.70.183.198]) by mfilter10-d.gandi.net (mfilter10-d.gandi.net [10.0.15.180]) (amavisd-new, port 10024) with ESMTP id XEQByUo1I1De for ; Tue, 17 Feb 2015 03:23:06 +0100 (CET) X-Originating-IP: 66.220.3.179 Original-Received: from localhost (gm179.geneticmail.com [66.220.3.179]) (Authenticated sender: kelly@prtime.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id A9EF5FB886 for ; Tue, 17 Feb 2015 03:23:04 +0100 (CET) X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4b98:c:538::198 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:183183 Archived-At: --=-=-= Content-Type: text/plain When following a link in the help page for a function or variable to open the source code file, and the file is already open, Emacs moves point in that buffer, but it doesn't push the mark first, so the user loses his previous place. It should push the mark first. The find-tag command does push the mark, but it's too trigger-happy: it pushes the mark even if the file was _not_ already open. Since the mark ring is for the user's use to keep track of where he was in the buffer, the extraneous push leaves the user later wondering whether that file had already been open for some reason, or whether Emacs was just being sloppy. This affects me when I look at several xref targets in the same files; if Emacs is fixed to avoid the extraneous push, then I can just try to go to the previous mark, and if it says there's no mark set, I immediately know that I looked at no other targets in the current buffer. The attached patch solves both issues. It pushes the mark for both help xref and find-tag if the buffer is already open, and it avoids pushing otherwise. (Applies to 24.4; not updated for trunk yet.) I posted an earlier version of this patch in 2012, but it was poorly designed, incomplete, and buggy, and was therefore ignored. Maybe this updated version will be considered poorly designed too, because it still uses a dynamic variable as an implicit return value. But if not this way, then what other practical solution is there? --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=xref-push-mark-24.4.patch --- lisp/files.el +++ lisp/files.el @@ -1816,12 +1816,15 @@ If a buffer exists visiting FILENAME, return that one, but verify that the file has not changed since visited or saved. The buffer is not selected, just returned to the caller. +If a buffer exists visiting FILENAME, and the dynamic variable +buffer-preexisted is bound, set it to t. Optional second arg NOWARN non-nil means suppress any warning messages. Optional third arg RAWFILE non-nil means the file is read literally. Optional fourth arg WILDCARDS non-nil means do wildcard processing and visit all the matching files. When wildcards are actually used and expanded, return a list of buffers that are visiting the various files." + (defvar buffer-preexisted) ; Tell byte compiler about free dynamic var (setq filename (abbreviate-file-name (expand-file-name filename))) @@ -1981,6 +1984,8 @@ (error (if rawfile "File already visited non-literally" "File already visited literally")))))) ;; Return the buffer we are using. + (if (boundp 'buffer-preexisted) + (setq buffer-preexisted t)) buf) ;; Create a new buffer. (setq buf (create-file-buffer filename)) --- lisp/emacs-lisp/find-func.el +++ lisp/emacs-lisp/find-func.el @@ -212,6 +212,7 @@ (indirect-function (find-function-advised-original fun-or-var))))) (with-current-buffer (find-file-noselect file) + (save-excursion (goto-char (point-min)) (unless (re-search-forward (if type @@ -223,7 +224,7 @@ "\"")) nil t) (error "Can't find source for %s" fun-or-var)) - (cons (current-buffer) (match-beginning 0)))) + (cons (current-buffer) (match-beginning 0))))) ;;;###autoload (defun find-library (library) @@ -283,6 +284,7 @@ (let* ((filename (find-library-name library)) (regexp-symbol (cdr (assq type find-function-regexp-alist)))) (with-current-buffer (find-file-noselect filename) + (save-excursion (let ((regexp (format (symbol-value regexp-symbol) ;; Entry for ` (backquote) macro in loaddefs.el, ;; (defalias (quote \`)..., has a \ but @@ -310,7 +312,7 @@ (progn (beginning-of-line) (cons (current-buffer) (point))) - (cons (current-buffer) nil)))))))) + (cons (current-buffer) nil))))))))) ;;;###autoload (defun find-function-noselect (function &optional lisp-only) --- lisp/help-mode.el +++ lisp/help-mode.el @@ -198,11 +198,16 @@ (help-C-file-name (indirect-function fun) 'fun))) ;; Don't use find-function-noselect because it follows ;; aliases (which fails for built-in functions). - (let ((location + (defvar buffer-preexisted) + (let* (buffer-preexisted + (location (find-function-search-for-symbol fun nil file))) (pop-to-buffer (car location)) (if (cdr location) - (goto-char (cdr location)) + (progn (if (and (/= (cdr location) (point)) + buffer-preexisted) + (push-mark)) + (goto-char (cdr location))) (message "Unable to find location in file")))) 'help-echo (purecopy "mouse-2, RET: find function's definition")) @@ -211,8 +216,11 @@ 'help-function (lambda (fun file) (setq file (locate-library file t)) (if (and file (file-readable-p file)) - (progn + (defvar buffer-preexisted) + (let* ((buffer-preexisted + (find-buffer-visiting file))) (pop-to-buffer (find-file-noselect file)) + (if buffer-preexisted (push-mark)) (goto-char (point-min)) (if (re-search-forward (format "^[ \t]*(\\(cl-\\)?define-compiler-macro[ \t]+%s" @@ -227,10 +235,15 @@ 'help-function (lambda (var &optional file) (when (eq file 'C-source) (setq file (help-C-file-name var 'var))) - (let ((location (find-variable-noselect var file))) + (defvar buffer-preexisted) + (let* (buffer-preexisted + (location (find-variable-noselect var file))) (pop-to-buffer (car location)) (if (cdr location) - (goto-char (cdr location)) + (progn (if (and (/= (cdr location) (point)) + buffer-preexisted) + (push-mark)) + (goto-char (cdr location))) (message "Unable to find location in file")))) 'help-echo (purecopy "mouse-2, RET: find variable's definition")) --- lisp/progmodes/etags.el +++ lisp/progmodes/etags.el @@ -1171,9 +1171,11 @@ ;; Get the local value in the tags table buffer before switching buffers. (setq goto-func goto-tag-location-function) + (defvar buffer-preexisted) + (let (buffer-preexisted) (tag-find-file-of-tag-noselect file) (widen) - (push-mark) + (if buffer-preexisted (push-mark))) (funcall goto-func tag-info) ;; Return the buffer where the tag was found. @@ -1183,6 +1185,7 @@ "Find the right line in the specified FILE." ;; If interested in compressed-files, search files with extensions. ;; Otherwise, search only the real file. + (defvar buffer-preexisted) ; Tell byte compiler about free dynamic var (let* ((buffer-search-extensions (if auto-compression-mode tags-compression-info-list '(""))) @@ -1198,6 +1201,8 @@ (while (and (not the-buffer) buffer-search-extensions) (setq the-buffer (find-buffer-visiting (concat file (car buffer-search-extensions)))) (setq buffer-search-extensions (cdr buffer-search-extensions))) + (if (and the-buffer (boundp 'buffer-preexisted)) + (setq buffer-preexisted t)) ;; if found a buffer but file modified, ensure we re-read ! (if (and the-buffer (not (verify-visited-file-modtime the-buffer))) (find-file-noselect (buffer-file-name the-buffer))) --=-=-=--