From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions Date: Fri, 06 Jan 2023 12:27:30 -0500 Message-ID: References: <87fscpifdw.fsf@localhost> <83wn61w7lc.fsf@gnu.org> <8dea9f3e0e7865d8f3c6@heytings.org> <83r0w9vzgy.fsf@gnu.org> <8dea9f3e0e8e6114ffc3@heytings.org> <83pmbtvwyr.fsf@gnu.org> <8dea9f3e0e160e17818f@heytings.org> <83mt6xvsct.fsf@gnu.org> <8dea9f3e0ea1de84c49a@heytings.org> <83cz7sw4q1.fsf@gnu.org> <371ba1d0beb0ed44a9a6@heytings.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36996"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 60568@debbugs.gnu.org, Eli Zaretskii , yantar92@posteo.net, mardani29@yahoo.es To: Gregory Heytings Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jan 06 18:28:13 2023 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 1pDqVp-0009RM-K8 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 06 Jan 2023 18:28:13 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pDqVj-00049Z-Bj; Fri, 06 Jan 2023 12:28:07 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pDqVe-000465-TJ for bug-gnu-emacs@gnu.org; Fri, 06 Jan 2023 12:28:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pDqVe-0000xq-E2 for bug-gnu-emacs@gnu.org; Fri, 06 Jan 2023 12:28:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pDqVd-0005hw-RB for bug-gnu-emacs@gnu.org; Fri, 06 Jan 2023 12:28:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 06 Jan 2023 17:28:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60568 X-GNU-PR-Package: emacs Original-Received: via spool by 60568-submit@debbugs.gnu.org id=B60568.167302606221912 (code B ref 60568); Fri, 06 Jan 2023 17:28:01 +0000 Original-Received: (at 60568) by debbugs.gnu.org; 6 Jan 2023 17:27:42 +0000 Original-Received: from localhost ([127.0.0.1]:55748 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pDqVK-0005hL-5w for submit@debbugs.gnu.org; Fri, 06 Jan 2023 12:27:42 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:40731) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pDqVI-0005h6-GO for 60568@debbugs.gnu.org; Fri, 06 Jan 2023 12:27:40 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id AF1F34414AE; Fri, 6 Jan 2023 12:27:34 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 879CB4410C4; Fri, 6 Jan 2023 12:27:32 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1673026052; bh=GyiYrOicC7G2j6DOGstV9xBMs+9KP3wHFEeCdTZlj8U=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=WwFdGSviyI+lKIn4EN0vpo8WSlvtoH741kvarQBJJ1yo/K2zTSiC2MS2451BFkde4 C4FC80EJcclbZg3rjB3Fd+aKFmZqYZNEaw5TK2uOOwqTyO/CAjRlK8PeAhjwsR5jOI Pib83D0gFnVQMRfYXU9OSf7fVRy+T0RGjD78g/aZO1dNJjN7jJAxx6SrwHSph8k5cq 15RYceY21vseWIGU6Gc8eFGziEywNxeHpp942i/poV66ZmrdEcbjctE10llf5dry5q cQvx6s7Zfl8Ei5QHS0S6vqOl6MGe3QcmFo8qJbDOgrPZSDDdQL7LkO8DeW8zKLMbll 5TzzSAVq/I+vQ== Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 617481203CF; Fri, 6 Jan 2023 12:27:32 -0500 (EST) In-Reply-To: <371ba1d0beb0ed44a9a6@heytings.org> (Gregory Heytings's message of "Fri, 06 Jan 2023 09:11:02 +0000") 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:252733 Archived-At: >>> Not always, I can imagine functions defined with fset for example. I'd consider that an error. `defalias` on the other hand is indeed a normal and common case. >>> What about the attached patch? It seems to work well. >> SGTM. With which cases did you test this? >> Adding Stefan, in case he has any comments. SGTM as well. I do have some comments, see below. > +(defun help-function-def--find-probable-definition-place (fun) Can we shorten the name using "guess" i.s.o "find-probable"? Also, this should be in `find-func.el` rather than in `help-mode.el`, no? > + (save-excursion > + ;; Build a list of strings with the symbols (and strings) of FUN. > + (let ((names > + (mapcar (lambda (el) > + (concat > + "\\_<" > + (regexp-quote (format "%s" el)) I think you want "%S" rather than "%s". > + "\\_>")) > + (flatten-tree (symbol-function fun)))) - We probably want to use `advice-cd*r` so as to flatten the actual function definition rather than the combination of its definition with the various pieces of advice that are currently applied. - The above works for non-compiled functions but for byte-compiled functions we need an ad-hoc version of `flatten-tree` which extracts the constants from the constant vector of bytecode objects (also recursively since bytecode objects tend to contain other bytecode objects). For native-compiled functions, we may be able to get some kind of "constant vector" as well, but I don't think that's currently accessible from ELisp. We should ask Andrea. > + ;; Exclude symbols that are 3 characters or less. > + (when (> (length el) 9) If we filter them out when constructing `names`, we can use a comment where the number matches the number used in the code :-) > + ;; Determine the beginning position of that > + ;; function. > + (goto-char (point-min)) > + (when (catch 'found > + (while (re-search-forward > + (concat "\\_<" > + (regexp-quote result) > + "\\_>")) > + (when (string= (lisp-current-defun-name) > + result) > + (throw 'found t))) > + (throw 'found nil)) > + (beginning-of-defun) > + (cons result (point))))))))))))) Can't we use `find-function-noselect` here? Or maybe just return the function name and let the caller then use `find-function-noselect`. > @@ -281,7 +366,31 @@ help-function-def--button-function > (unless (= (point) position) > (push-mark nil t)) > (goto-char position)) > - (message "Unable to find location in file"))))) > + (let ((probable-definition-place > + (help-function-def--find-probable-definition-place fun))) > + (when probable-definition-place > + (goto-char (cdr probable-definition-place))) > + (let ((help-buffer-under-preparation t)) > + (help-setup-xref (list #'help-function-def--button-function > + fun file) > + (called-interactively-p 'interactive)) > + (with-help-window (help-buffer) > + (insert (substitute-command-keys > + (format "Function %s could not be found in `%s'.\n\n" > + fun (file-name-nondirectory file)))) > + (setq help-mode--current-data (list :symbol fun > + :file file)) > + (save-excursion > + (re-search-backward > + (substitute-command-keys "`\\([^`']+\\)'") > + nil t) > + (help-xref-button 1 'help-function-def fun file)) > + (when probable-definition-place > + (insert (substitute-command-keys > + (format "It was probably defined by `%s'.\n\n" > + (car probable-definition-place))))) > + (insert "Function definition:\n\n") > + (insert (pp-to-string (symbol-function fun)))))))))) Please move this to a separate function. Also, I'm not completely convinced what you do with the *Help* buffer here is the UI we'll want, really. But I don't have a good idea for what to replace it with (yet), so we can start with that and tweak it later. Stefan