unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Gabriel Santos <gabrielsantosdesouza@disroot.org>
Cc: 74461@debbugs.gnu.org
Subject: bug#74461: [PATCH] Add go-work-ts-mode
Date: Thu, 21 Nov 2024 21:14:21 +0200	[thread overview]
Message-ID: <865xogpfg2.fsf@gnu.org> (raw)
In-Reply-To: <F23F09F1-D3C4-4133-AC46-67E7E1C35BDA@disroot.org> (message from Gabriel Santos on Thu, 21 Nov 2024 14:25:14 -0300)

[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 <gabrielsantosdesouza@disroot.org>
> 
> >> 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" <bug-gnu-emacs@gnu.org>
> >> 
> >> 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.






      parent reply	other threads:[~2024-11-21 19:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-21 10:49 bug#74461: [PATCH] Add go-work-ts-mode Gabriel Santos via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-21 16:02 ` Eli Zaretskii
     [not found]   ` <F23F09F1-D3C4-4133-AC46-67E7E1C35BDA@disroot.org>
2024-11-21 19:14     ` Eli Zaretskii [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=865xogpfg2.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=74461@debbugs.gnu.org \
    --cc=gabrielsantosdesouza@disroot.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).