From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#55778: 29.0.50; [PATCH] M-. into a .gz; we've all been there. Date: Fri, 03 Jun 2022 14:41:52 +0300 Message-ID: <83mteuc6tb.fsf@gnu.org> References: <87k09yw9ao.fsf@dick> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38252"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 55778@debbugs.gnu.org To: dick.r.chiang@gmail.com Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jun 03 13:54:45 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nx5t6-0009mp-VR for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 03 Jun 2022 13:54:45 +0200 Original-Received: from localhost ([::1]:40518 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nx5t4-0004JK-QV for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 03 Jun 2022 07:54:43 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43910) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nx5gp-0004g1-Rj for bug-gnu-emacs@gnu.org; Fri, 03 Jun 2022 07:42:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:33320) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nx5go-0003xh-HK for bug-gnu-emacs@gnu.org; Fri, 03 Jun 2022 07:42:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nx5go-0002Jj-Fn for bug-gnu-emacs@gnu.org; Fri, 03 Jun 2022 07:42:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 03 Jun 2022 11:42:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55778 X-GNU-PR-Package: emacs Original-Received: via spool by 55778-submit@debbugs.gnu.org id=B55778.16542565198898 (code B ref 55778); Fri, 03 Jun 2022 11:42:02 +0000 Original-Received: (at 55778) by debbugs.gnu.org; 3 Jun 2022 11:41:59 +0000 Original-Received: from localhost ([127.0.0.1]:55450 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nx5gk-0002JR-GE for submit@debbugs.gnu.org; Fri, 03 Jun 2022 07:41:59 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:58912) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nx5gg-0002Iw-BI for 55778@debbugs.gnu.org; Fri, 03 Jun 2022 07:41:57 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:41394) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nx5ga-0003wm-NY; Fri, 03 Jun 2022 07:41:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=LrI8LYmAaz9b6CNHB5wym/Myvd+/heIOOFHqdQN7490=; b=cAmZO3SfVG+T nwwDsMBYLZ0YgHzdCNr75pjuIIbmp6Jgs+KqEt8xf6ZfPHylZsnkfyVGK1NhVHS5GuzLEpljyZ5O9 KHMpyR1UxeBdL1Hzt6arXjDoRANUa70DZkqGBS7CIYYUc2nlQvYSn+61N6P1AjFLMVmFJjOaQbMNZ 81npc3gmw7KVSUIrfPpJesIlsvonQnD3OLtIl7UVmnlrTBdLGB4LXEW11YeIAmN0t+Zf7PiXzCym7 KZOrYGPl9jYFc/qUrS2LdoJ0RLRn5NUZ3zEAV9sMIDT/VWxh3kN6rUsEpaIiOCOS4ql4OridkomN4 xEOAtTzUgzesZ7Zdk1Zl0w==; Original-Received: from [87.69.77.57] (port=3354 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nx5gS-0007xc-Bz; Fri, 03 Jun 2022 07:41:43 -0400 In-Reply-To: <87k09yw9ao.fsf@dick> (dick.r.chiang@gmail.com) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:233605 Archived-At: > From: dick.r.chiang@gmail.com > Date: Fri, 03 Jun 2022 02:27:59 -0400 > > * lisp/emacs-lisp/find-func.el (find-function-regexp-alist): > Was crashing on this missing entry in find-function-regexp-alist. > (find-function-C-source-directory): Style. > (find-function-C-source): Style. > (find-function-search-for-symbol): English. > * lisp/help-fns.el (help-C-file-name): English. > (find-lisp-object-file-name): Use it. > * lisp/help-mode.el (xref): Require. > (help-function-def--button-function): Use it. > (help-face-def): Use it. > * lisp/progmodes/elisp-mode.el (xref-backend-definitions): Style. > (elisp--xref-find-definitions): Style. > (xref-location-marker): Use it. > * lisp/progmodes/project.el (project-root): Snuff bytecomp warning. > * lisp/progmodes/xref.el (xref-prefer-source-directory): New defcustom. > (xref-preferred-message): Message user. > (xref-preferred-source): Act on new defcustom. > (xref-show-definitions-buffer): Style. > (xref--create-fetcher): Style. > * src/emacs.c (syms_of_emacs): English. > * src/lread.c (load_path_default): English. > (init_lread): Style. > (syms_of_lread): New defvar. > * test/lisp/progmodes/elisp-mode-tests.el (xref-tests-prefer-source): > Test it. > * test/lisp/progmodes/xref-tests.el > (xref-matches-in-directory-finds-none-for-some-regexp): Style. > (xref--buf-pairs-iterator-groups-markers-by-buffers-1): Style. > (xref--buf-pairs-iterator-groups-markers-by-buffers-2): Style. > (xref--buf-pairs-iterator-cleans-up-markers): Style. Please separate the real changes from refactoring of existing code and from documentation changes that are unrelated to code changes. They each should be reviewed and judged separately and with different criteria in mind. > @@ -257,7 +258,7 @@ find-library--from-load-history > > (defvar find-function-C-source-directory > (let ((dir (expand-file-name "src" source-directory))) > - (if (file-accessible-directory-p dir) dir)) > + (when (file-accessible-directory-p dir) dir)) > "Directory where the C source files of Emacs can be found. > If nil, do not try to find the source code of functions and variables > defined in C.") > @@ -283,8 +284,8 @@ find-function-C-source > (read-directory-name "Emacs C source dir: " nil nil t)))) > (setq file (expand-file-name file dir)) > (if (file-readable-p file) > - (if (null find-function-C-source-directory) > - (setq find-function-C-source-directory dir)) > + (unless find-function-C-source-directory > + (setq find-function-C-source-directory dir)) > (error "The C source file %s is not available" > (file-name-nondirectory file)))) This seems to be some effort to use 'when' and 'unless' instead of 'if'? If so, please don't: I see no reason for such changes. > +(defcustom xref-prefer-source-directory nil > + "If non-nil, jump to the file in `source-directory' that > +corresponds to the target." "if that exists", right? And what does "jump" mean? who is it that will "jump"? Also, the first line of a doc string should be a complete sentence, for the benefit of 'apropos' commands. > DEFVAR_LISP ("installation-directory", Vinstallation_directory, > - doc: /* A directory within which to look for the `lib-src' and `etc' directories. > -In an installed Emacs, this is normally nil. It is non-nil if > -both `lib-src' (on MS-DOS, `info') and `etc' directories are found > -within the variable `invocation-directory' or its parent. For example, > -this is the case when running an uninstalled Emacs executable from its > -build directory. */); > + doc: /* Should have been named build-directory. > +Counter-intuitively, this variable is nil when Emacs is invoked from > +its `make install` executable. It normally takes on the value of > +`source-directory' when Emacs is invoked from its within-repo `make` > +executable. Its primary use is locating the lib-src and etc > +subdirectories of the build. Not to be confused with > +`installed-directory'. */); Please keep your controversial opinions out of the Emacs sources, and definitely out of the doc strings. Your intuition is wrong here. > -/* Return the default load-path, to be used if EMACSLOADPATH is unset. > - This does not include the standard site-lisp directories > - under the installation prefix (i.e., PATH_SITELOADSEARCH), > - but it does (unless no_site_lisp is set) include site-lisp > - directories in the source/build directories if those exist and we > - are running uninstalled. > - > - Uses the following logic: > - If !will_dump: Use PATH_LOADSEARCH. > - The remainder is what happens when dumping is about to happen: > - If dumping, just use PATH_DUMPLOADSEARCH. > - Otherwise use PATH_LOADSEARCH. > - > - If !initialized, then just return PATH_DUMPLOADSEARCH. > - If initialized: > - If Vinstallation_directory is not nil (ie, running uninstalled): > - If installation-dir/lisp exists and not already a member, > - we must be running uninstalled. Reset the load-path > - to just installation-dir/lisp. (The default PATH_LOADSEARCH > - refers to the eventual installation directories. Since we > - are not yet installed, we should not use them, even if they exist.) > - If installation-dir/lisp does not exist, just add > - PATH_DUMPLOADSEARCH at the end instead. > - Add installation-dir/site-lisp (if !no_site_lisp, and exists > - and not already a member) at the front. > - If installation-dir != source-dir (ie running an uninstalled, > - out-of-tree build) AND install-dir/src/Makefile exists BUT > - install-dir/src/Makefile.in does NOT exist (this is a sanity > - check), then repeat the above steps for source-dir/lisp, site-lisp. */ > +/* Dig toplevel LOAD-PATH out of epaths.h. */ Fat chance we will ever accept such "changes". These details might bore you, but they are there for a reason: so that people who need to make changes in this tricky code could be aware of all the pitfalls that others in their good time fell into. > static Lisp_Object > load_path_default (void) > { > if (will_dump_p ()) > - /* PATH_DUMPLOADSEARCH is the lisp dir in the source directory. > - We used to add ../lisp (ie the lisp dir in the build > - directory) at the front here, but that should not be > - necessary, since in out of tree builds lisp/ is empty, save > - for Makefile. */ > + /* PATH_DUMPLOADSEARCH is the lisp dir in the source directory. */ > return decode_env_path (0, PATH_DUMPLOADSEARCH, 0); And this. > - Lisp_Object lpath = Qnil; > - > - lpath = decode_env_path (0, PATH_LOADSEARCH, 0); > + Lisp_Object lpath = decode_env_path (0, PATH_LOADSEARCH, 0); > > + /* Counter-intuitively Vinstallation_directory is nil for > + invocations of the `make install` executable, and is > + Vsource_directory for invocations of the within-repo `make` > + executable. > + */ And this. (The comment style is also not our style.) > + DEFVAR_LISP ("installed-directory", Vinstalled_directory, > + doc: /* Install path of built-in lisp libraries. > +This directory contains the `etc`, `lisp`, and `site-lisp` > +installables, and is determined at configure time in the epaths-force > +make target. Not to be confused with the legacy > +`installation-directory' nor `invocation-directory'. */); > + Vinstalled_directory > + = Fexpand_file_name (build_string ("../"), > + Fcar (decode_env_path (0, PATH_LOADSEARCH, 0))); This won't work reliably, because it assumes PATH_LOADSEARCH is determined at build time. That is only true for some builds, but not for others. It also disregards EMACSLOADPATH. To be reliable, the variable's value needs to be recalculated at startup, each time anew.