Subject: bug#74461: [PATCH] Add go-work-ts-mode

From: Eli Zaretskii <[email-masked]>
Date: Thu, 21 Nov 2024 21:14:21 +0200

[Please use Reply All to reply, to keep the bug tracker CC'ed.]

> Date: Thu, 21 Nov 2024 14:25:14 -0300
> From: Gabriel Santos <[email-masked]>
> 
> >> Date: Thu, 21 Nov 2024 07:49:35 -0300
> >> From: Gabriel Santos via "Bug reports for GNU Emacs,
> >> the Swiss army knife of text editors" <[email-masked]>
> >> 
> >> I wrote a tree-sitter mode after founding there wasn't one for Go
> >> Workspace files. I'd like to say thanks to Randy Taylor on his work to
> >> the Go tree-sitter mode, helped me a lot in this process.
> >> 
> >> Commit message:
> >> 
> >> * admin/notes/tree-sitter/build-module/batch.sh:
> >> * admin/notes/tree-sitter/build-module/build.sh: Add go-work support.
> >> * etc/NEWS: Mention go-work-ts-mode.
> >> * lisp/progmodes/eglot.el (eglot-server-programs): Add go-work-ts-mode.
> >> * lisp/progmodes/go-ts-mode: Add go-work-ts-mode for working with
> >> workspace files.
> >> 
> >> The parser can be found here:
> >> https://github.com/omertuc/tree-sitter-go-work
> >
> >I think the parser URL should be mentioned in the comments in
> >go-ts-mode.el.
> 
> Thanks, got it. I'll also add the links to the other parsers too. This is helpful information for users, as some of
> these aren't included in the tree-sitter organization (https://github.com/tree-sitter).
> 
> >> I will mail my copyright assignment to the FSF still today.
> >
> >Thanks.
> 
> On this, I missed the address of my house by a single number (astonishing how often I do this). To update
> the data in my request, can I just send another e-mail?

Yes, sure.

> >> +---
> >> +** New major mode 'go-work-ts-mode'.
> >> +A major mode based on the tree-sitter library for editing "go.work"
> >> +files. It is auto-enabled for files which are named "go.work".
> >
> >The last sentence is factually inaccurate: the user needs to load
> >go-ts-mode to have this mode auto-enabled, right?
> 
> I got it from the original commit for the Go tree-sitter mode (fee2efe1b03 by Randy Taylor). I was working
> quite late at night on this, so I didn't have time to reflect. You're right, this mode won't be auto-loaded since
> tree-sitter modes currently require some user set-up to get working.
> 
> >> +(defun go-work-ts-mode--in-directive-p ()
> >> + "Return non-nil if point is inside a directive.
> >
> >"inside a go-work directive", I presume? IOW, the doc string's first
> >sentence sounds too general.
> 
> Again, I took this from the original commit. I'm currently viewing the new version of the
> `go-mod-ts-mode--in-directive-p' and the docstring is better written. I'll rewrite based on it, and also make
> sure to check the current state of files instead.
> 
> >> +(if (treesit-ready-p 'gowork)
> >> + (add-to-list 'auto-mode-alist '("/go\\.work\\'" . go-work-ts-mode)))
> >
> >Wouldn't it be better to have this in the default value of
> >auto-mode-alist, just conditioned by (treesit-ready-p 'gowork) ? That
> >way, loading go-ts-mode will not change auto-mode-alist.
> >
> >Stefan, WDYT?
> 
> This change was also included in the file already, so I followed it. Looking at the current values of
> auto-mode-alist (lisp/files.el), I don't see any tree-sitter modes here. And other tree-sitter modes (see
> list/progmodes/rust-ts-mode) also include the same behaviour. I don't have much knowledge on this matter,
> so I'll let you decide on this.

Tree-sitter modes are not listed where we have other modes for the
same files, to avoid breaking past behavior.  I don't think this
danger is relevant to go.work files, but let's see what Stefan thinks
about that.

> Also, this is my first time on an e-mail workflow like this. Updating the patch based on your requests would
> only require a rebase + new e-mail, or a rebase + reply to this thread?

The latter.  Using the same thread keeps all the discussions recorded
as part of the same issue number, which is good for future reviewing
of the discussion.  For the same reason, please don't edit the Subject
of the responses, so that they all keep the same Subject.

Thanks.