From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Evgeni Kolev Newsgroups: gmane.emacs.bugs Subject: bug#60407: [PATCH] Update go-ts-mode to use Imenu facility Date: Tue, 3 Jan 2023 11:01:25 +0200 Message-ID: References: <83y1qm39o3.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37101"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , Yuan Fu , 60407@debbugs.gnu.org To: Randy Taylor Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Jan 03 10:03:24 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 1pCdCd-0009RT-Qu for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 03 Jan 2023 10:03:23 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pCdBQ-0006Sr-MT; Tue, 03 Jan 2023 04:02:08 -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 1pCdBL-0006OI-9e for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 04:02: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 1pCdBK-0001Oo-Rb for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 04:02:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pCdBK-0002Aq-KY for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 04:02:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Evgeni Kolev Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 03 Jan 2023 09:02:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60407 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 60407-submit@debbugs.gnu.org id=B60407.16727365218342 (code B ref 60407); Tue, 03 Jan 2023 09:02:02 +0000 Original-Received: (at 60407) by debbugs.gnu.org; 3 Jan 2023 09:02:01 +0000 Original-Received: from localhost ([127.0.0.1]:44694 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pCdBJ-0002AM-07 for submit@debbugs.gnu.org; Tue, 03 Jan 2023 04:02:01 -0500 Original-Received: from mail-yb1-f169.google.com ([209.85.219.169]:41684) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pCdBF-00029k-8m for 60407@debbugs.gnu.org; Tue, 03 Jan 2023 04:01:58 -0500 Original-Received: by mail-yb1-f169.google.com with SMTP id 186so32361127ybe.8 for <60407@debbugs.gnu.org>; Tue, 03 Jan 2023 01:01:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=SclL3Q6I4BBPacj+EK9bInRICi6K2YFeeW21CcsUGFc=; b=VY+iCGAf4ktAgubv1cQDImRHfLYeALGS7Er3Q5JRMK+3oHdY9DZI4o8R9eCxeimBwo jjXWxsA0NFEPs3dvyrK21PWJW8SARMluerzhpRTx1EVBrjbRjNy0jHf1rvKc76FHEXIO ANrGSuLnNSPPQ/YZqAQWh6pJqiC9ybHfUTU/1S0zCGgStr6yoxgpQP6+9U0ay7HSxVGT a609GimU2UoYoayiLD03YxLolVhm5GvY+uCAW0owIMDFzemim+1tsq9ru5nPN9h2BYpp 196MLy24Gsb9WrjQAQY/A6h2IFwyVNtsMfhQ3mjQApbPvM8MgW2Qmm4W8lo9sq+btsRd f9Eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=SclL3Q6I4BBPacj+EK9bInRICi6K2YFeeW21CcsUGFc=; b=D+PQeSVx0Svjp+iFFXX6Z56iMdE9QvM7rVzXyOaixXb/EPEjUTa4ya63InCeBwhBcq NqPE7CUoHTM3UZ1LbKudeLgstEwzK+TRQARbGIyH/64BcNMeLYbUTXcpxIMkfyaGCOm9 yZSiGtbrPgglZJ/XrkXLorKmPhs84OWl5CrwgrdVaNb40ZsgYwNsSKjxZEfx3Dx6+zSp exUZNQ2+oOEHQMnWfsE4K0vnp8qetZviCbk7TUaTl6/gkfTFCTi/G0u6b4C4pC3Y4Jr9 KKnyTNVMj8EyqaRT7BQoR3fZCQmXJ0h375iqouLHqg8KmsgmhmKAm+wiq8KlC9ywQ+FH oNqg== X-Gm-Message-State: AFqh2krVEz2iV/NMkMg1x36mthDjr0LKNnL2ZHjK72NhstblbFjiyfZz 3zxM5X4op+REtU2whVdgeQX7Nx4syBnKLAY5l+w= X-Google-Smtp-Source: AMrXdXuocvFtvd1PXZxuL7fPR16otKOJYisM2K2pwkJIp4DTxbAl2E7aPa1L6EzlaGxajwHbrOiHGQGGTVdyTO9LkvM= X-Received: by 2002:a25:ce8c:0:b0:78f:12e9:7e83 with SMTP id x134-20020a25ce8c000000b0078f12e97e83mr1354698ybe.587.1672736511664; Tue, 03 Jan 2023 01:01:51 -0800 (PST) In-Reply-To: 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:252382 Archived-At: Hi Randy, thank you for your feedback! I'm providing an updated patch below. I tested with "type Quack int" and other cases such as: type MyInt = int type Option func(s string) type List[T any] struct { head, tail *element[T] } type Number interface { int64 | float64 } After experimenting, I decided to add additional Imenu categories: "Alias" and "Type". So the final list of newly added categories is "Method", "Struct", "Interface", "Alias" and "Type". Structs and interfaces are technically a private case for "Type", but are put in their own Imenu category. Hence the "Type" category now holds all types except structs, interfaces and aliases (aliases have their own tree sitter type defined in Go's grammar.js). For reference, eglot's Imenu uses category "Class" instead of "Type". But I decided to not replicate this behavior - "Class" is not a widely used Go concept. However, I decided to replicate another eglot behaviour - prefixing the method names with the type of the receiver (for example, "(rect).area" for "func (r rect) area() float64..."). I've also addressed the other comments. I'm a bit unsure how the git commit should be formatted - the part of the message which describes changed functions/files. Please let me know if the patch can be improved. The patch is below. commit a96e70052a79881ac666ab699ffd63ed916eaf83 Author: Evgeni Kolev Date: Thu Dec 29 17:49:40 2022 +0200 Improve go-ts-mode Imenu The Imenu items are extended to support "Method", "Struct", "Interface", "Alias" and "Type". go-ts-mode is updated to use the Imenu facility added in commit b39dc7ab27a696a8607ab859aeff3c71509231f5. * lisp/progmodes/go-ts-mode.el (go-ts-mode--imenu-1) (go-ts-mode--imenu): Remove functions. (go-ts-mode--defun-name) (go-ts-mode--interface-node-p) (go-ts-mode--struct-node-p) (go-ts-mode--other-type-node-p) (go-ts-mode--alias-node-p): New functions. (go-ts-mode): Improve Imenu settings. diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el index 1d6a8a30db5..d91b555e03e 100644 --- a/lisp/progmodes/go-ts-mode.el +++ b/lisp/progmodes/go-ts-mode.el @@ -173,44 +173,6 @@ go-ts-mode--font-lock-settings '((ERROR) @font-lock-warning-face)) "Tree-sitter font-lock settings for `go-ts-mode'.") -(defun go-ts-mode--imenu () - "Return Imenu alist for the current buffer." - (let* ((node (treesit-buffer-root-node)) - (func-tree (treesit-induce-sparse-tree - node "function_declaration" nil 1000)) - (type-tree (treesit-induce-sparse-tree - node "type_spec" nil 1000)) - (func-index (go-ts-mode--imenu-1 func-tree)) - (type-index (go-ts-mode--imenu-1 type-tree))) - (append - (when func-index `(("Function" . ,func-index))) - (when type-index `(("Type" . ,type-index)))))) - -(defun go-ts-mode--imenu-1 (node) - "Helper for `go-ts-mode--imenu'. -Find string representation for NODE and set marker, then recurse -the subtrees." - (let* ((ts-node (car node)) - (children (cdr node)) - (subtrees (mapcan #'go-ts-mode--imenu-1 - children)) - (name (when ts-node - (treesit-node-text - (pcase (treesit-node-type ts-node) - ("function_declaration" - (treesit-node-child-by-field-name ts-node "name")) - ("type_spec" - (treesit-node-child-by-field-name ts-node "name")))))) - (marker (when ts-node - (set-marker (make-marker) - (treesit-node-start ts-node))))) - (cond - ((or (null ts-node) (null name)) subtrees) - (subtrees - `((,name ,(cons name marker) ,@subtrees))) - (t - `((,name . ,marker)))))) - ;;;###autoload (add-to-list 'auto-mode-alist '("\\.go\\'" . go-ts-mode)) @@ -228,9 +190,21 @@ go-ts-mode (setq-local comment-end "") (setq-local comment-start-skip (rx "//" (* (syntax whitespace)))) + ;; Navigation. + (setq-local treesit-defun-type-regexp + (regexp-opt '("method_declaration" + "function_declaration" + "type_declaration"))) + (setq-local treesit-defun-name-function #'go-ts-mode--defun-name) + ;; Imenu. - (setq-local imenu-create-index-function #'go-ts-mode--imenu) - (setq-local which-func-functions nil) + (setq-local treesit-simple-imenu-settings + `(("Function" "\\`function_declaration\\'" nil nil) + ("Method" "\\`method_declaration\\'" nil nil) + ("Struct" "\\`type_declaration\\'" go-ts-mode--struct-node-p nil) + ("Interface" "\\`type_declaration\\'" go-ts-mode--interface-node-p nil) + ("Type" "\\`type_declaration\\'" go-ts-mode--other-type-node-p nil) + ("Alias" "\\`type_declaration\\'" go-ts-mode--alias-node-p nil))) ;; Indent. (setq-local indent-tabs-mode t @@ -247,6 +221,53 @@ go-ts-mode (treesit-major-mode-setup))) +(defun go-ts-mode--defun-name (node) + "Return the defun name of NODE. +Return nil if there is no name or if NODE is not a defun node." + (pcase (treesit-node-type node) + ("function_declaration" + (treesit-node-text + (treesit-node-child-by-field-name + node "name") + t)) + ("method_declaration" + (let* ((receiver-node (treesit-node-child-by-field-name node "receiver")) + (type-node (treesit-search-subtree receiver-node "type_identifier")) + (name-node (treesit-node-child-by-field-name node "name"))) + (concat + "(" (treesit-node-text type-node) ")." + (treesit-node-text name-node)))) + ("type_declaration" + (treesit-node-text + (treesit-node-child-by-field-name + (treesit-node-child node 0 t) "name") + t)))) + +(defun go-ts-mode--interface-node-p (node) + "Return t if NODE is an interface." + (and + (string-equal "type_declaration" (treesit-node-type node)) + (treesit-search-subtree node "interface_type" nil nil 2))) + +(defun go-ts-mode--struct-node-p (node) + "Return t if NODE is a struct." + (and + (string-equal "type_declaration" (treesit-node-type node)) + (treesit-search-subtree node "struct_type" nil nil 2))) + +(defun go-ts-mode--alias-node-p (node) + "Return t if NODE is a type alias." + (and + (string-equal "type_declaration" (treesit-node-type node)) + (treesit-search-subtree node "type_alias" nil nil 1))) + +(defun go-ts-mode--other-type-node-p (node) + "Return t if NODE is a type, other than interface or struct." + (and + (string-equal "type_declaration" (treesit-node-type node)) + (not (go-ts-mode--interface-node-p node)) + (not (go-ts-mode--struct-node-p node)))) + ;; go.mod support. (defvar go-mod-ts-mode--syntax-table On Mon, Jan 2, 2023 at 12:31 AM Randy Taylor wrote: > > On Sunday, January 1st, 2023 at 12:08, Evgeni Kolev wrote: > > > > As I mentioned in the start of the mail thread - go-ts-mode's Imenu > > puts Go interfaces and structs in the same "Type" bucket. This can be > > improved in go-ts-mode. > > > > I'm providing a second patch below which splits the interfaces and > > structs into their own Imenu categories. > > > > Please let me know if I should provide the second patch later, in a > > separate thread, after the first patch is finished. I'm assuming it's > > simpler to review the patches together. If it's not - I'll provide > > them in a way to make the review easier, just let me know. > > > > Hi Evgeni! > > Thanks for working on this, and apologies for the delay. > > The patches look good to me and work well, except for one issue (and a few minor nits) mentioned below. All in one patch is probably best. > > - Type definitions are no longer captured. For example: > type Quack int > Does not show up anymore. > - go-ts-mode--struct-node-p's docstring is missing a period at the end. > - In go-ts-mode--interface-node-p and go-ts-mode--struct-node-p's docstrings, I'm not sure it's worthwhile to mention "Go". Just interface and struct should be fine. > - Should the commit message mention changes to go-ts-mode (as in the actual define-derived-mode mode part)?