Subject: bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options

From: john muhl <[email-masked]>
Date: Thu, 07 Nov 2024 09:04:42 -0600

Eli Zaretskii <[email-masked]> writes:

>> From: john muhl <[email-masked]>
>> Date: Wed, 06 Nov 2024 15:22:34 -0600
>> 
>> * lisp/progmodes/lua-ts-mode.el (lua-ts-luacheck-program):
>> (lua-ts-inferior-program): Remove 'nil' as it is never a valid
>> choice.  (Bug#74235)
>
> IMO, the use of these options in the code is not clean: if these
> programs are not installed, the commands which reference these options
> will signal errors whose diagnostic might not clearly indicate the
> root cause.  I would like to see a cleaner handling of such
> contingencies by this mode.  Does this make sense, and if so, would
> you like to suggest changes to that effect?

Good point. I’ll work on that very soon.

>>  (defcustom lua-ts-luacheck-program "luacheck"
>>    "Location of the Luacheck program."
>> -  :type '(choice (const :tag "None" nil) string)
>> +  :type 'string
>>    :version "30.1")
>
> I think if we require this to be the name of a program, 'file' is a
> better value for :type, because it allows completion if the user types
> an absolute file name.

Makes sense.

>>  (defcustom lua-ts-inferior-buffer "*Lua*"
>> @@ -83,7 +83,7 @@ lua-ts-inferior-buffer
>>  
>>  (defcustom lua-ts-inferior-program "lua"
>>    "Program to run in the inferior Lua process."
>> -  :type '(choice (const :tag "None" nil) string)
>> +  :type 'string
>>    :version "30.1")
>>  
>>  (defcustom lua-ts-inferior-options '("-i")
>> -- 
>> 2.47.0
>> 
>> john muhl <[email-masked]> writes:
>> 
>> > Tags: patch
>> >
>> > For some silly reason the lua-ts-{inferior,luacheck}-program
>> > options ended up with an option for “None”. Obviously setting your
>> > Lua interpreter or linter program to nil is not going to work.
>> >
>> > Any chance it could make it into emacs-30?
>
> lua-ts-mode is new in Emacs 30, so we can install these changes on the
> release branch, as a stopgap.  But I'd like to see a cleaner solution
> installed later, even if we decide to install that on master due to
> its complexity.
>
> Thanks.

Then let’s put this one on hold and I’ll work on the improvements
you suggest. Hopefully it doesn’t require significant changes and
we can skip the stopgap and get it all into 30.

Thanks for the help.