GNU bug report logs - #79832
[PATCH] Add 'lua-base-mode'

Previous Next

To reply to this bug, email your comments to 79832 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 09:35:08 -0600

Tags: patch

This adds a lua-base-mode and makes it the parent mode for the
other Lua modes allowing them to share implementations where
possible.

Message #8 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 10:08:33 -0600

[Message part 1 (text/plain, inline)]

Should the obsolete lua-ts-* options or anything else be included
in NEWS?

[0001-Add-lua-base-mode-Bug-79832.patch (text/x-patch, attachment)]

Message #11 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: john muhl <jm <at> pub.pink>
Cc: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 18:31:14 +0200

> From: john muhl <jm <at> pub.pink>
> Date: Fri, 14 Nov 2025 10:08:33 -0600
> 
> Should the obsolete lua-ts-* options or anything else be included
> in NEWS?

Could you please explain what you did, so that the above question
could be answered without studying a 750-line patch?

TIA

Message #14 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 11:38:53 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: john muhl <jm <at> pub.pink>
>> Date: Fri, 14 Nov 2025 10:08:33 -0600
>> 
>> Should the obsolete lua-ts-* options or anything else be included
>> in NEWS?
>
> Could you please explain what you did, so that the above question
> could be answered without studying a 750-line patch?
>
> TIA

Sure.

The defcustoms are marked them with make-obsolete-variable but
left in place. That way users who already customized some lua-ts-*
options will have them continue to work and new users won’t be
confused about if they should use them.

The obsolete inferior Lua related commands are replaced with
define-obsolete-function-alias where the lua-* command is
equivalent; which is all of them except lua-ts-inferior-lua (the
command to start an inferior Lua session). The latter is marked
with make-obsolete and re-written to call lua-start-process
passing in the values from any lua-ts-inferior-* options; again so
existing users with lua-ts-inferior-* customizations will have
those continue to work.

The goal was to keep things working for existing lua-ts-mode users
and let lua-mode users be able to try out or switch to the ts mode
without needing duplicate options; e.g. lua-ts-mode now respects
both lua-indent-level & lua-ts-indent-offset. Then sometime down
the road we’ll remove the obsolete lua-ts-* stuff.

I see now that in squashing I missed a patch so if it would make
it easier to review I can clean those squashed commits and send
them individually rather than as one big patch. I guess it’d be
4-6 self-contained patches. If you prefer that should I submit
them each to new bugs and close this one?

Message #17 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: john muhl <jm <at> pub.pink>
Cc: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 20:43:50 +0200

> From: john muhl <jm <at> pub.pink>
> Cc: 79832 <at> debbugs.gnu.org
> Date: Fri, 14 Nov 2025 11:38:53 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: john muhl <jm <at> pub.pink>
> >> Date: Fri, 14 Nov 2025 10:08:33 -0600
> >> 
> >> Should the obsolete lua-ts-* options or anything else be included
> >> in NEWS?
> >
> > Could you please explain what you did, so that the above question
> > could be answered without studying a 750-line patch?
> >
> > TIA
> 
> Sure.
> 
> The defcustoms are marked them with make-obsolete-variable but
> left in place. That way users who already customized some lua-ts-*
> options will have them continue to work and new users won’t be
> confused about if they should use them.
> 
> The obsolete inferior Lua related commands are replaced with
> define-obsolete-function-alias where the lua-* command is
> equivalent; which is all of them except lua-ts-inferior-lua (the
> command to start an inferior Lua session). The latter is marked
> with make-obsolete and re-written to call lua-start-process
> passing in the values from any lua-ts-inferior-* options; again so
> existing users with lua-ts-inferior-* customizations will have
> those continue to work.
> 
> The goal was to keep things working for existing lua-ts-mode users
> and let lua-mode users be able to try out or switch to the ts mode
> without needing duplicate options; e.g. lua-ts-mode now respects
> both lua-indent-level & lua-ts-indent-offset. Then sometime down
> the road we’ll remove the obsolete lua-ts-* stuff.

Thanks, but I don't understand the motivation for obsoleting lua-ts-*
options.  You explain why you took care making the changes in
backward-compatible way, which is fine, but what was the motivation
for the changes to begin with?

> I see now that in squashing I missed a patch so if it would make
> it easier to review I can clean those squashed commits and send
> them individually rather than as one big patch. I guess it’d be
> 4-6 self-contained patches. If you prefer that should I submit
> them each to new bugs and close this one?

I think it's enough to post an updated patch here.

Message #20 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Fri, 14 Nov 2025 13:45:44 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: john muhl <jm <at> pub.pink>
>> Cc: 79832 <at> debbugs.gnu.org
>> Date: Fri, 14 Nov 2025 11:38:53 -0600
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: john muhl <jm <at> pub.pink>
>> >> Date: Fri, 14 Nov 2025 10:08:33 -0600
>> >> 
>> >> Should the obsolete lua-ts-* options or anything else be included
>> >> in NEWS?
>> >
>> > Could you please explain what you did, so that the above question
>> > could be answered without studying a 750-line patch?
>> >
>> > TIA
>> 
>> Sure.
>> 
>> The defcustoms are marked them with make-obsolete-variable but
>> left in place. That way users who already customized some lua-ts-*
>> options will have them continue to work and new users won’t be
>> confused about if they should use them.
>> 
>> The obsolete inferior Lua related commands are replaced with
>> define-obsolete-function-alias where the lua-* command is
>> equivalent; which is all of them except lua-ts-inferior-lua (the
>> command to start an inferior Lua session). The latter is marked
>> with make-obsolete and re-written to call lua-start-process
>> passing in the values from any lua-ts-inferior-* options; again so
>> existing users with lua-ts-inferior-* customizations will have
>> those continue to work.
>> 
>> The goal was to keep things working for existing lua-ts-mode users
>> and let lua-mode users be able to try out or switch to the ts mode
>> without needing duplicate options; e.g. lua-ts-mode now respects
>> both lua-indent-level & lua-ts-indent-offset. Then sometime down
>> the road we’ll remove the obsolete lua-ts-* stuff.
>
> Thanks, but I don't understand the motivation for obsoleting lua-ts-*
> options.  You explain why you took care making the changes in
> backward-compatible way, which is fine, but what was the motivation
> for the changes to begin with?

Something along the lines of: if lua-mode been part of Emacs when
lua-ts-mode was added those options & commands would never have
existed.

Having to define intermediate variables in lua-mode (to avoid
messing with user options) and do “(or lua-ts-option lua-option)”
in lua-ts-mode adds some clutter to the code but obsoletion would
at least make it temporary clutter.

>> I see now that in squashing I missed a patch so if it would make
>> it easier to review I can clean those squashed commits and send
>> them individually rather than as one big patch. I guess it’d be
>> 4-6 self-contained patches. If you prefer that should I submit
>> them each to new bugs and close this one?
>
> I think it's enough to post an updated patch here.

Sounds good.

Message #23 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: john muhl <jm <at> pub.pink>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79832 <at> debbugs.gnu.org
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 10:52:51 +0200

> From: john muhl <jm <at> pub.pink>
> Cc: 79832 <at> debbugs.gnu.org
> Date: Fri, 14 Nov 2025 13:45:44 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> The defcustoms are marked them with make-obsolete-variable but
> >> left in place. That way users who already customized some lua-ts-*
> >> options will have them continue to work and new users won’t be
> >> confused about if they should use them.
> >> 
> >> The obsolete inferior Lua related commands are replaced with
> >> define-obsolete-function-alias where the lua-* command is
> >> equivalent; which is all of them except lua-ts-inferior-lua (the
> >> command to start an inferior Lua session). The latter is marked
> >> with make-obsolete and re-written to call lua-start-process
> >> passing in the values from any lua-ts-inferior-* options; again so
> >> existing users with lua-ts-inferior-* customizations will have
> >> those continue to work.
> >> 
> >> The goal was to keep things working for existing lua-ts-mode users
> >> and let lua-mode users be able to try out or switch to the ts mode
> >> without needing duplicate options; e.g. lua-ts-mode now respects
> >> both lua-indent-level & lua-ts-indent-offset. Then sometime down
> >> the road we’ll remove the obsolete lua-ts-* stuff.
> >
> > Thanks, but I don't understand the motivation for obsoleting lua-ts-*
> > options.  You explain why you took care making the changes in
> > backward-compatible way, which is fine, but what was the motivation
> > for the changes to begin with?
> 
> Something along the lines of: if lua-mode been part of Emacs when
> lua-ts-mode was added those options & commands would never have
> existed.
> 
> Having to define intermediate variables in lua-mode (to avoid
> messing with user options) and do “(or lua-ts-option lua-option)”
> in lua-ts-mode adds some clutter to the code but obsoletion would
> at least make it temporary clutter.

Thanks.

It is not easy to review this patch because it seems like it has at
least two separate purposes: (1) extend/improve/fix existing features,
and (2) make it easier for users to customize both lua-mode and
lua-ts-mode with the same set of options and customizations.  It might
be better to split this into two separate patches (which sounds like a
contradiction to what I wrote previously, but that was under the
impression that the patch only did (2)).

In any case, there should be a NEWS entry for (2), and it should
describe the change in user-facing terms.  Something like

  Both Lua Mode and Lua Ts Mode can now be customized using the same
  set of user options and customizations.  The options specific to
  'lua-ts-mode', such as  'lua-ts-indent-offset' and
  'lua-ts-inferior-program', are now obsolete, and users should use
  the common options, like  'lua-indent-offset' and
  'lua-default-application', instead.

Btw, did you try to call lua-mode and lua-ts-mode one after the other,
and if you did, did that work as expected, wrt fontifications,
movement by defun, etc.?  Toggling these modes back and forth
sometimes causes problems due to implicit dependencies.

Other comments to the patch you posted:

> -(defun lua-start-process (&optional name program startfile &rest switches)
> +(defun lua-start-process (&optional name program startfile histfile &rest switches)
>    "Start a Lua process named NAME, running PROGRAM.
> -PROGRAM defaults to NAME, which defaults to `lua-default-application'.
> +PROGRAM defaults to `lua-default-application'.

This loses the information about the default for NAME.  Why is it a
good idea?

> -          (lua-send-string lua-process-init-code)))
> +          (process-send-string lua-process lua-process-init-code)))

Is this an equivalent replacement?  AFAICT, lua-process-init-code does
not end in a newline.

> -(defcustom lua-ts-mode-hook nil
> -  "Hook run after entering `lua-ts-mode'."
> -  :type 'hook
> -  :options '(eglot-ensure
> -             flymake-mode
> -             hs-minor-mode
> -             outline-minor-mode)
> -  :version "30.1")

Why is it a good idea to remove a mode hook specific to lua-ts-mode?
Isn't it possible that users would like to customize lua-ts-mode in
some special way, which is different from lua-mode?  In any case, this
is a backward-incompatible change, which should be avoided.

> -(defcustom lua-ts-indent-offset 4
> +(defcustom lua-ts-indent-offset nil
>    "Number of spaces for each indentation step in `lua-ts-mode'."
> -  :type 'natnum
> +  :type '(choice (const nil) natnum)
>    :safe 'natnump
>    :version "30.1")

This changes the default value of a defcustom, so the :version tag
should be updated.  Same with other defcustom's whose default values
you change.

> -(defcustom lua-ts-luacheck-program "luacheck"
> +(defcustom lua-ts-luacheck-program nil
>    "Location of the Luacheck program."
> -  :type 'file
> +  :type '(choice (const nil) file)
>    :version "30.1")

The doc string should explain the meaning of the nil value.  Same with
other variables where you made similar changes.

>  (easy-menu-define lua-ts-mode-menu lua-ts-mode-map
>    "Menu bar entry for `lua-ts-mode'."
>    `("Lua"
> -    ["Evaluate Buffer" lua-ts-send-buffer]
> -    ["Evaluate File" lua-ts-send-file]
> -    ["Evaluate Region" lua-ts-send-region]
> +    ["Evaluate Buffer" lua-send-buffer]
> +    ["Evaluate File" lua-send-file]
> +    ["Evaluate Region" lua-send-region]
>      "--"
> -    ["Start Process" lua-ts-inferior-lua]
> -    ["Show Process Buffer" lua-ts-show-process-buffer]
> -    ["Hide Process Buffer" lua-ts-hide-process-buffer]
> -    ["Kill Process" lua-ts-kill-process]
> +    ["Start Process" lua-start-process]
> +    ["Show Process Buffer" lua-show-process-buffer]
> +    ["Hide Process Buffer" lua-hide-process-buffer]
> +    ["Kill Process" lua-kill-process]
>      "--"
>      ["Customize" (lambda () (interactive) (customize-group "lua-ts"))]))

Why is this menu and the similar menu in lua-mode not unified into a
single menu?

> -(define-derived-mode lua-ts-mode prog-mode "Lua"
> +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
>    "Major mode for editing Lua files, powered by tree-sitter.

This makes lua-mode and lua-ts-mode two independent descendants of
lua-base-mode.  However, it seems like lua-ts-mode uses variables and
functions from lua-mode and not from lua-base-mode.  And loading
lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
descendant of lua-mode.  Doesn't that seem like a contradiction?
Would that make independent development of both modes harder in the
future?  Should we reconsider the descendancy and/or to which mode do
the common functions/variables belong?

Stefan, WDYT?

Message #26 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org, john muhl <jm <at> pub.pink>
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 08:19:09 -0500

>> -(defcustom lua-ts-mode-hook nil
>> -  "Hook run after entering `lua-ts-mode'."
>> -  :type 'hook
>> -  :options '(eglot-ensure
>> -             flymake-mode
>> -             hs-minor-mode
>> -             outline-minor-mode)
>> -  :version "30.1")
>
> Why is it a good idea to remove a mode hook specific to lua-ts-mode?

Note that `lua-ts-mode-hook` is still defined and used (thanks to
`define-derived-mode`), so the only thing this does is to remove the
`:options` thingy: there's no backward-incompatibility issue.

> -(define-derived-mode lua-ts-mode prog-mode "Lua"
> +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
>    "Major mode for editing Lua files, powered by tree-sitter.

> This makes lua-mode and lua-ts-mode two independent descendants of
> lua-base-mode.  However, it seems like lua-ts-mode uses variables and
> functions from lua-mode and not from lua-base-mode.  And loading
> lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
> descendant of lua-mode.  Doesn't that seem like a contradiction?

I think this is generally a good change: we should try and keep the
differences between TS and non-TS modes to a minimum.

What you describe as "variables and functions from lua-mode" are really
"variables and functions shared between lua-mode and lua-ts-mode".

> Would that make independent development of both modes harder in the
> future?

Yes and no.  It will make changes a bit more delicate because you need
to make sure they work right in both modes, but that also means that it
will save work by reducing duplication.  More importantly, it should
be more user-friendly by increasing consistency between these two modes.


        Stefan

Message #29 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79832 <at> debbugs.gnu.org, jm <at> pub.pink
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 16:36:33 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: john muhl <jm <at> pub.pink>,  79832 <at> debbugs.gnu.org
> Date: Sat, 15 Nov 2025 08:19:09 -0500
> 
> >> -(defcustom lua-ts-mode-hook nil
> >> -  "Hook run after entering `lua-ts-mode'."
> >> -  :type 'hook
> >> -  :options '(eglot-ensure
> >> -             flymake-mode
> >> -             hs-minor-mode
> >> -             outline-minor-mode)
> >> -  :version "30.1")
> >
> > Why is it a good idea to remove a mode hook specific to lua-ts-mode?
> 
> Note that `lua-ts-mode-hook` is still defined and used (thanks to
> `define-derived-mode`), so the only thing this does is to remove the
> `:options` thingy: there's no backward-incompatibility issue.

And removing :options is justified?

> > -(define-derived-mode lua-ts-mode prog-mode "Lua"
> > +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
> >    "Major mode for editing Lua files, powered by tree-sitter.
> 
> > This makes lua-mode and lua-ts-mode two independent descendants of
> > lua-base-mode.  However, it seems like lua-ts-mode uses variables and
> > functions from lua-mode and not from lua-base-mode.  And loading
> > lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
> > descendant of lua-mode.  Doesn't that seem like a contradiction?
> 
> I think this is generally a good change: we should try and keep the
> differences between TS and non-TS modes to a minimum.

I know.  My point was to ask whether it is a good idea to make them
two independent descendants of lua-base-mode, but in practice
implement almost everything in lua-mode and not in lua-base-mode.

> > Would that make independent development of both modes harder in the
> > future?
> 
> Yes and no.  It will make changes a bit more delicate because you need
> to make sure they work right in both modes, but that also means that it
> will save work by reducing duplication.  More importantly, it should
> be more user-friendly by increasing consistency between these two modes.

Making these functions and variables be in the base mode will not
hamper consistency; quite the opposite.  OTOH, doing that in the base
mode would allow independent tailoring of lua-mode and lua-ts-mode,
whereas using lua-mode functions in lua-ts-mode makes it harder.  I
fear we will see in lua-mode stuff like

  (if (eq major-mode 'lua-ts-mode)
     (do-something))

which I think is at least inelegant.

Message #32 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 09:19:01 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: john muhl <jm <at> pub.pink>
>> Cc: 79832 <at> debbugs.gnu.org
>> Date: Fri, 14 Nov 2025 13:45:44 -0600
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> The defcustoms are marked them with make-obsolete-variable but
>> >> left in place. That way users who already customized some lua-ts-*
>> >> options will have them continue to work and new users won’t be
>> >> confused about if they should use them.
>> >>
>> >> The obsolete inferior Lua related commands are replaced with
>> >> define-obsolete-function-alias where the lua-* command is
>> >> equivalent; which is all of them except lua-ts-inferior-lua (the
>> >> command to start an inferior Lua session). The latter is marked
>> >> with make-obsolete and re-written to call lua-start-process
>> >> passing in the values from any lua-ts-inferior-* options; again so
>> >> existing users with lua-ts-inferior-* customizations will have
>> >> those continue to work.
>> >>
>> >> The goal was to keep things working for existing lua-ts-mode users
>> >> and let lua-mode users be able to try out or switch to the ts mode
>> >> without needing duplicate options; e.g. lua-ts-mode now respects
>> >> both lua-indent-level & lua-ts-indent-offset. Then sometime down
>> >> the road we’ll remove the obsolete lua-ts-* stuff.
>> >
>> > Thanks, but I don't understand the motivation for obsoleting lua-ts-*
>> > options.  You explain why you took care making the changes in
>> > backward-compatible way, which is fine, but what was the motivation
>> > for the changes to begin with?
>>
>> Something along the lines of: if lua-mode been part of Emacs when
>> lua-ts-mode was added those options & commands would never have
>> existed.
>>
>> Having to define intermediate variables in lua-mode (to avoid
>> messing with user options) and do “(or lua-ts-option lua-option)”
>> in lua-ts-mode adds some clutter to the code but obsoletion would
>> at least make it temporary clutter.
>
> Thanks.
>
> It is not easy to review this patch because it seems like it has at
> least two separate purposes: (1) extend/improve/fix existing features,
> and (2) make it easier for users to customize both lua-mode and
> lua-ts-mode with the same set of options and customizations.  It might
> be better to split this into two separate patches (which sounds like a
> contradiction to what I wrote previously, but that was under the
> impression that the patch only did (2)).

No problem. I’ll split it up.

> In any case, there should be a NEWS entry for (2), and it should
> describe the change in user-facing terms.  Something like
>
>   Both Lua Mode and Lua Ts Mode can now be customized using the same
>   set of user options and customizations.  The options specific to
>   'lua-ts-mode', such as  'lua-ts-indent-offset' and
>   'lua-ts-inferior-program', are now obsolete, and users should use
>   the common options, like  'lua-indent-offset' and
>   'lua-default-application', instead.

Thanks.

> Btw, did you try to call lua-mode and lua-ts-mode one after the other,
> and if you did, did that work as expected, wrt fontifications,
> movement by defun, etc.?  Toggling these modes back and forth
> sometimes causes problems due to implicit dependencies.

Hmm. I tried lua->lua-ts but probably not cycling them
repeatedly. I’ll look into it.

> Other comments to the patch you posted:
>
>> -(defun lua-start-process (&optional name program startfile &rest switches)
>> +(defun lua-start-process (&optional name program startfile histfile &rest switches)
>>    "Start a Lua process named NAME, running PROGRAM.
>> -PROGRAM defaults to NAME, which defaults to `lua-default-application'.
>> +PROGRAM defaults to `lua-default-application'.
>
> This loses the information about the default for NAME.  Why is it a
> good idea?

The code didn’t actually do what the docstring said. Would you
rather change the code to match the docstring?

>> -          (lua-send-string lua-process-init-code)))
>> +          (process-send-string lua-process lua-process-init-code)))
>
> Is this an equivalent replacement?  AFAICT, lua-process-init-code does
> not end in a newline.

The patch should have added a newline to
lua-process-init-code...maybe it was in bit that got missed in the
squash. I’ll double check before sending the updated patches.

The problem with using lua-send-string there is that it eventually
calls lua-start-process without any arguments. So when you try to
(lua-start-process "my-name" ...) you end up with 2 inferior
processes.

>> -(defcustom lua-ts-mode-hook nil
>> -  "Hook run after entering `lua-ts-mode'."
>> -  :type 'hook
>> -  :options '(eglot-ensure
>> -             flymake-mode
>> -             hs-minor-mode
>> -             outline-minor-mode)
>> -  :version "30.1")
>
> Why is it a good idea to remove a mode hook specific to lua-ts-mode?
> Isn't it possible that users would like to customize lua-ts-mode in
> some special way, which is different from lua-mode?  In any case, this
> is a backward-incompatible change, which should be avoided.

It doesn’t remove the hook, only it’s visibilty in customize
buffers. Existing customizations keep working and there is an
equivalent defcustom for lua-mode-hook for those who prefer the
customize interface. If you think it important to have both then
it’s no problem to drop that part. Just let me know.

>> -(defcustom lua-ts-indent-offset 4
>> +(defcustom lua-ts-indent-offset nil
>>    "Number of spaces for each indentation step in `lua-ts-mode'."
>> -  :type 'natnum
>> +  :type '(choice (const nil) natnum)
>>    :safe 'natnump
>>    :version "30.1")
>
> This changes the default value of a defcustom, so the :version tag
> should be updated.  Same with other defcustom's whose default values
> you change.
>
>> -(defcustom lua-ts-luacheck-program "luacheck"
>> +(defcustom lua-ts-luacheck-program nil
>>    "Location of the Luacheck program."
>> -  :type 'file
>> +  :type '(choice (const nil) file)
>>    :version "30.1")
>
> The doc string should explain the meaning of the nil value.  Same with
> other variables where you made similar changes.

Will fix.

>>  (easy-menu-define lua-ts-mode-menu lua-ts-mode-map
>>    "Menu bar entry for `lua-ts-mode'."
>>    `("Lua"
>> -    ["Evaluate Buffer" lua-ts-send-buffer]
>> -    ["Evaluate File" lua-ts-send-file]
>> -    ["Evaluate Region" lua-ts-send-region]
>> +    ["Evaluate Buffer" lua-send-buffer]
>> +    ["Evaluate File" lua-send-file]
>> +    ["Evaluate Region" lua-send-region]
>>      "--"
>> -    ["Start Process" lua-ts-inferior-lua]
>> -    ["Show Process Buffer" lua-ts-show-process-buffer]
>> -    ["Hide Process Buffer" lua-ts-hide-process-buffer]
>> -    ["Kill Process" lua-ts-kill-process]
>> +    ["Start Process" lua-start-process]
>> +    ["Show Process Buffer" lua-show-process-buffer]
>> +    ["Hide Process Buffer" lua-hide-process-buffer]
>> +    ["Kill Process" lua-kill-process]
>>      "--"
>>      ["Customize" (lambda () (interactive) (customize-group "lua-ts"))]))
>
> Why is this menu and the similar menu in lua-mode not unified into a
> single menu?

Only because the idea didn’t occur to me. Should we try to unify
the keymaps too?

>> -(define-derived-mode lua-ts-mode prog-mode "Lua"
>> +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
>>    "Major mode for editing Lua files, powered by tree-sitter.
>
> This makes lua-mode and lua-ts-mode two independent descendants of
> lua-base-mode.  However, it seems like lua-ts-mode uses variables and
> functions from lua-mode and not from lua-base-mode.  And loading
> lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
> descendant of lua-mode.  Doesn't that seem like a contradiction?
> Would that make independent development of both modes harder in the
> future?  Should we reconsider the descendancy and/or to which mode do
> the common functions/variables belong?

I tried to follow the ruby/ruby-ts model but if you think there is
a better way that’s fine too. Would it be better to move the base
mode to a new file?

The only functions from lua-mode used by lua-ts-mode are related
to the inferior Lua stuff which is independent of other lua-mode
functionality.

Message #35 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org, jm <at> pub.pink
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 10:47:27 -0500

>> >> -(defcustom lua-ts-mode-hook nil
>> >> -  "Hook run after entering `lua-ts-mode'."
>> >> -  :type 'hook
>> >> -  :options '(eglot-ensure
>> >> -             flymake-mode
>> >> -             hs-minor-mode
>> >> -             outline-minor-mode)
>> >> -  :version "30.1")
>> >
>> > Why is it a good idea to remove a mode hook specific to lua-ts-mode?
>> 
>> Note that `lua-ts-mode-hook` is still defined and used (thanks to
>> `define-derived-mode`), so the only thing this does is to remove the
>> `:options` thingy: there's no backward-incompatibility issue.
> And removing :options is justified?

I don't know, but I don't see anything in those options that's very
Lua-specific so I don't think they bring very much.  When I see this
kind of code in a major mode, I just take it as a hint that we should
improve our infrastructure.

E.g., it would be better to arrange for those options to show up
"automatically", maybe for any major mode that derives from `prog-mode`.

> I know.  My point was to ask whether it is a good idea to make them
> two independent descendants of lua-base-mode, but in practice
> implement almost everything in lua-mode and not in lua-base-mode.

+1


        Stefan

Message #38 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: john muhl <jm <at> pub.pink>
Cc: 79832 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 18:07:31 +0200

> From: john muhl <jm <at> pub.pink>
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>,  79832 <at> debbugs.gnu.org
> Date: Sat, 15 Nov 2025 09:19:01 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> -(defun lua-start-process (&optional name program startfile &rest switches)
> >> +(defun lua-start-process (&optional name program startfile histfile &rest switches)
> >>    "Start a Lua process named NAME, running PROGRAM.
> >> -PROGRAM defaults to NAME, which defaults to `lua-default-application'.
> >> +PROGRAM defaults to `lua-default-application'.
> >
> > This loses the information about the default for NAME.  Why is it a
> > good idea?
> 
> The code didn’t actually do what the docstring said. Would you
> rather change the code to match the docstring?

Not necessarily.  The code does include handling of omitted NAME, so I
think the doc string should be adapted to describe what it does.

> >>  (easy-menu-define lua-ts-mode-menu lua-ts-mode-map
> >>    "Menu bar entry for `lua-ts-mode'."
> >>    `("Lua"
> >> -    ["Evaluate Buffer" lua-ts-send-buffer]
> >> -    ["Evaluate File" lua-ts-send-file]
> >> -    ["Evaluate Region" lua-ts-send-region]
> >> +    ["Evaluate Buffer" lua-send-buffer]
> >> +    ["Evaluate File" lua-send-file]
> >> +    ["Evaluate Region" lua-send-region]
> >>      "--"
> >> -    ["Start Process" lua-ts-inferior-lua]
> >> -    ["Show Process Buffer" lua-ts-show-process-buffer]
> >> -    ["Hide Process Buffer" lua-ts-hide-process-buffer]
> >> -    ["Kill Process" lua-ts-kill-process]
> >> +    ["Start Process" lua-start-process]
> >> +    ["Show Process Buffer" lua-show-process-buffer]
> >> +    ["Hide Process Buffer" lua-hide-process-buffer]
> >> +    ["Kill Process" lua-kill-process]
> >>      "--"
> >>      ["Customize" (lambda () (interactive) (customize-group "lua-ts"))]))
> >
> > Why is this menu and the similar menu in lua-mode not unified into a
> > single menu?
> 
> Only because the idea didn’t occur to me. Should we try to unify
> the keymaps too?

If it's possible, why not?  What I would do is define a single keymap
from which both modes would inherit their own.  Then the common stuff
can be defined only once, and OTOH users can customize each mode
differently, if they want.

> >> -(define-derived-mode lua-ts-mode prog-mode "Lua"
> >> +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
> >>    "Major mode for editing Lua files, powered by tree-sitter.
> >
> > This makes lua-mode and lua-ts-mode two independent descendants of
> > lua-base-mode.  However, it seems like lua-ts-mode uses variables and
> > functions from lua-mode and not from lua-base-mode.  And loading
> > lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
> > descendant of lua-mode.  Doesn't that seem like a contradiction?
> > Would that make independent development of both modes harder in the
> > future?  Should we reconsider the descendancy and/or to which mode do
> > the common functions/variables belong?
> 
> I tried to follow the ruby/ruby-ts model but if you think there is
> a better way that’s fine too. Would it be better to move the base
> mode to a new file?

Having the base mode on a separate file is not necessary.  What
bothered me more was that most of the stuff was implemented in
lua-mode (not lua-base-mode).  But Stefan thinks it isn't a problem,
so I think it's up to you.

Message #41 received at 79832 <at> debbugs.gnu.org (full text, mbox):

From: john muhl <jm <at> pub.pink>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79832 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#79832: [PATCH] Add 'lua-base-mode'
Date: Sat, 15 Nov 2025 17:47:16 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: john muhl <jm <at> pub.pink>
>> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>,  79832 <at> debbugs.gnu.org
>> Date: Sat, 15 Nov 2025 09:19:01 -0600
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >> -(define-derived-mode lua-ts-mode prog-mode "Lua"
>> >> +(define-derived-mode lua-ts-mode lua-base-mode "Lua"
>> >>    "Major mode for editing Lua files, powered by tree-sitter.
>> >
>> > This makes lua-mode and lua-ts-mode two independent descendants of
>> > lua-base-mode.  However, it seems like lua-ts-mode uses variables and
>> > functions from lua-mode and not from lua-base-mode.  And loading
>> > lua-ts-mode loads lua-mode, which seems to imply that lua-ts-mode is a
>> > descendant of lua-mode.  Doesn't that seem like a contradiction?
>> > Would that make independent development of both modes harder in the
>> > future?  Should we reconsider the descendancy and/or to which mode do
>> > the common functions/variables belong?
>>
>> I tried to follow the ruby/ruby-ts model but if you think there is
>> a better way that’s fine too. Would it be better to move the base
>> mode to a new file?
>
> Having the base mode on a separate file is not necessary.  What
> bothered me more was that most of the stuff was implemented in
> lua-mode (not lua-base-mode).  But Stefan thinks it isn't a problem,
> so I think it's up to you.

Alright. I think I see what you mean and it looks like Stefan
agreed that more stuff as part of the base mode would be better. I
thought it would be easiest to keep the changes in lua-mode
minimal but maybe that’s not best. Let me take a fresh look.

I’ll send patches for the bug fixes and other not strictly related
stuff in other bugs and come back here after trying some
alternative approachs to lua-base-mode.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.