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

[Stefan Monnier]

> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -273,7 +273,7 @@ automatically)."
>      (elm-mode . ("elm-language-server"))
>      (mint-mode . ("mint" "ls"))
>      ((kotlin-mode kotlin-ts-mode) . ("kotlin-language-server"))
> -    ((go-mode go-dot-mod-mode go-dot-work-mode go-ts-mode go-mod-ts-mode)
> +    ((go-mode go-dot-mod-mode go-dot-work-mode go-ts-mode go-mod-ts-mode 
> go-work-ts-mode)
>       . ("gopls"))
>      ((R-mode ess-r-mode) . ("R" "--slave" "-e"
>                              "languageserver::run()"))

Do we have a way to tell Eglot which LSP server(s) to use via some
buffer-local var, instead of having to change this centralized
"database"?

> @@ -565,6 +568,102 @@ what the parent of the node would be if it were a node."
>  (if (treesit-ready-p 'gomod)
>      (add-to-list 'auto-mode-alist '("/go\\.mod\\'" . go-mod-ts-mode)))
>  
> +;; go.work support.

I'd use 3 or more semi-colons here, so it acts as a section separator.

> +(defvar go-work-ts-mode--syntax-table
> +  (let ((table (make-syntax-table)))
> +    (modify-syntax-entry ?/   ". 124b" table)
> +    (modify-syntax-entry ?\n  "> b"    table)
> +    table)
> +  "Syntax table for `go-work-ts-mode'.")

The "4" above is weird since there's no "3".  Similarly, the use of "b"
is a bit odd since the "normal" comment is unused.

> +(defun go-work-ts-mode--in-directive-p ()
> +  "Return non-nil if point is inside a Go workspace directive.

This docstring doesn't seem right: the function returns another
function, not a boolean.

> +        (pcase (treesit-node-type (treesit-node-at (point)))
> +          ("replace" t)
> +          ("use" t))))))

AKA (member (treesit-node-type (treesit-node-at (point))) '("replace" "use"))

> +;;;###autoload
> +(define-derived-mode go-work-ts-mode prog-mode "Go Work"
> +  "Major mode for editing go.work files, powered by tree-sitter."
> +  :group 'go
> +  :syntax-table go-work-ts-mode--syntax-table

Why not use the standard name for the syntax-table (in which case you
don't even need this `:syntax-table` argument)?

> +    ;; Indent.
> +    (setq-local indent-tabs-mode t
> +                treesit-simple-indent-rules go-work-ts-mode--indent-rules)

Is this `indent-tabs-mode` setting required by the definition of the
go.work syntax/language, or is it a personal preference?  If it's
a personal preference then it doesn't belong in the major mode, and if
it's required by the syntax, then say so in a comment (ideally with
a URL pointing to the relevant part of the language definition).

> +(if (treesit-ready-p 'gowork)
> +    (add-to-list 'auto-mode-alist '("/go\\.work\\'" . go-work-ts-mode)))

Since we don't have another (non-treesitter) mode for these files, I'd
recommend you go straight for:

    ;;;###autoload
    (add-to-list 'auto-mode-alist '("/go\\.work\\'" . go-work-ts-mode))


- Stefan