all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] Improve find-sibling-rules option type
@ 2023-09-24  1:03 Paul W. Rankin via Emacs development discussions.
  2023-09-24  5:33 ` Eli Zaretskii
  2023-09-24  8:34 ` Philip Kaludercic
  0 siblings, 2 replies; 6+ messages in thread
From: Paul W. Rankin via Emacs development discussions. @ 2023-09-24  1:03 UTC (permalink / raw)
  To: emacs-devel; +Cc: Paul W. Rankin

* lisp/files.el (find-sibling-rules): use alist with tags for custom
  type
---
This is preferable than having to enter a sexp as a user option.


 lisp/files.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 9d76668..e7adccd 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7572,7 +7572,8 @@ files, you could say something like:
 In this example, if you're in \"src/emacs/emacs-27/lisp/abbrev.el\",
 and a \"src/emacs/emacs-28/lisp/abbrev.el\" file exists, it's now
 defined as a sibling."
-  :type 'sexp
+  :type '(alist :key-type (string :tag "Match")
+                :value-type (string :tag "Expansion"))
   :version "29.1")
 
 (defun find-sibling-file (file)
-- 
2.42.0




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve find-sibling-rules option type
  2023-09-24  1:03 [PATCH] Improve find-sibling-rules option type Paul W. Rankin via Emacs development discussions.
@ 2023-09-24  5:33 ` Eli Zaretskii
  2023-09-24  9:44   ` Mauro Aranda
  2023-09-24  8:34 ` Philip Kaludercic
  1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2023-09-24  5:33 UTC (permalink / raw)
  To: Paul W. Rankin, Mauro Aranda; +Cc: emacs-devel

> Cc: "Paul W. Rankin" <hello@paulwrankin.com>
> Date: Sun, 24 Sep 2023 11:03:27 +1000
> From:  "Paul W. Rankin" via "Emacs development discussions." <emacs-devel@gnu.org>
> 
> * lisp/files.el (find-sibling-rules): use alist with tags for custom
>   type
> ---
> This is preferable than having to enter a sexp as a user option.

Thanks.

Mauro, any comments?



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve find-sibling-rules option type
  2023-09-24  1:03 [PATCH] Improve find-sibling-rules option type Paul W. Rankin via Emacs development discussions.
  2023-09-24  5:33 ` Eli Zaretskii
@ 2023-09-24  8:34 ` Philip Kaludercic
  1 sibling, 0 replies; 6+ messages in thread
From: Philip Kaludercic @ 2023-09-24  8:34 UTC (permalink / raw)
  To: Paul W. Rankin via Emacs development discussions.; +Cc: Paul W. Rankin

"Paul W. Rankin" via "Emacs development discussions."
<emacs-devel@gnu.org> writes:

> * lisp/files.el (find-sibling-rules): use alist with tags for custom
>   type
> ---
> This is preferable than having to enter a sexp as a user option.
>
>
>  lisp/files.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 9d76668..e7adccd 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -7572,7 +7572,8 @@ files, you could say something like:
>  In this example, if you're in \"src/emacs/emacs-27/lisp/abbrev.el\",
>  and a \"src/emacs/emacs-28/lisp/abbrev.el\" file exists, it's now
>  defined as a sibling."
> -  :type 'sexp
> +  :type '(alist :key-type (string :tag "Match")
                              ^
                              (elisp) Simple Types lists a type `regexp'
                              that you could use here as well.

> +                :value-type (string :tag "Expansion"))
>    :version "29.1")
>  
>  (defun find-sibling-file (file)

-- 
Philip Kaludercic



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve find-sibling-rules option type
  2023-09-24  5:33 ` Eli Zaretskii
@ 2023-09-24  9:44   ` Mauro Aranda
  2023-09-25  1:17     ` Paul W. Rankin via Emacs development discussions.
  0 siblings, 1 reply; 6+ messages in thread
From: Mauro Aranda @ 2023-09-24  9:44 UTC (permalink / raw)
  To: Eli Zaretskii, Paul W. Rankin; +Cc: emacs-devel

On 24/9/23 02:33, Eli Zaretskii wrote:
 >> Cc: "Paul W. Rankin" <hello@paulwrankin.com>
 >> Date: Sun, 24 Sep 2023 11:03:27 +1000
 >> From:  "Paul W. Rankin" via "Emacs development discussions." 
<emacs-devel@gnu.org>
 >>
 >> * lisp/files.el (find-sibling-rules): use alist with tags for custom
 >>   type
 >> ---
 >> This is preferable than having to enter a sexp as a user option.
 >
 > Thanks.
 >
 > Mauro, any comments?

In particular, AFAICT there could be more than one EXPANSION.  In that
case, :value-type should be a repeat of strings, not just a string.  In
addition, regexp should be used as the :key-type type.  (I see that
Philip already spotted this)

In general, when converting from sexp to a more specific type, I think
we have to take extra precaution and check the code for how the
variable it's used (i.e., the docstring might not tell the whole story).
If ever in doubt, then it might be better and safer to offer a choice
with the more specific type first, and keep sexp as a catch-all
alternative.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve find-sibling-rules option type
  2023-09-24  9:44   ` Mauro Aranda
@ 2023-09-25  1:17     ` Paul W. Rankin via Emacs development discussions.
  2023-09-25  9:52       ` Mauro Aranda
  0 siblings, 1 reply; 6+ messages in thread
From: Paul W. Rankin via Emacs development discussions. @ 2023-09-25  1:17 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: Eli Zaretskii, emacs-devel


On 2023-09-24 19:44, Mauro Aranda wrote:
> In particular, AFAICT there could be more than one EXPANSION.  In that
> case, :value-type should be a repeat of strings, not just a string.  In
> addition, regexp should be used as the :key-type type.  (I see that
> Philip already spotted this)

Just looking at `find-sibling-file-search' and it looks like maybe? But 
if this is the case I think the docstring should be rewritten. Looking 
at it from a user's perspective if the docstring says "a string" but 
there's a repeat of strings, that would be confusing.

> In general, when converting from sexp to a more specific type, I think
> we have to take extra precaution and check the code for how the
> variable it's used (i.e., the docstring might not tell the whole 
> story).
> If ever in doubt, then it might be better and safer to offer a choice
> with the more specific type first, and keep sexp as a catch-all
> alternative.

Customize already provides for this! The user can click [State] > Show 
Saved Lisp Expression to edit/override the value with their own sexp.

I think the main thing is not to have user options that demand sexps.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve find-sibling-rules option type
  2023-09-25  1:17     ` Paul W. Rankin via Emacs development discussions.
@ 2023-09-25  9:52       ` Mauro Aranda
  0 siblings, 0 replies; 6+ messages in thread
From: Mauro Aranda @ 2023-09-25  9:52 UTC (permalink / raw)
  To: Paul W. Rankin; +Cc: Eli Zaretskii, emacs-devel

On 24/9/23 22:17, Paul W. Rankin wrote:
 >
 > On 2023-09-24 19:44, Mauro Aranda wrote:
 >> In particular, AFAICT there could be more than one EXPANSION.  In that
 >> case, :value-type should be a repeat of strings, not just a string.  In
 >> addition, regexp should be used as the :key-type type. (I see that
 >> Philip already spotted this)
 >
 > Just looking at `find-sibling-file-search' and it looks like maybe? 
But if this is the case I think the docstring should be rewritten. 
Looking at it from a user's perspective if the docstring says "a string" 
but there's a repeat of strings, that would be confusing.

My clue was (MATCH EXPANSION...) and then it says EXPANSIONS.  And each
EXPANSION is a string, hence we have a repeat of strings.

 >> In general, when converting from sexp to a more specific type, I think
 >> we have to take extra precaution and check the code for how the
 >> variable it's used (i.e., the docstring might not tell the whole story).
 >> If ever in doubt, then it might be better and safer to offer a choice
 >> with the more specific type first, and keep sexp as a catch-all
 >> alternative.
 >
 > Customize already provides for this! The user can click [State] > 
Show Saved Lisp Expression to edit/override the value with their own sexp.

That was not my point.  My point was that in a "sexp to
more-specific-type" change, there's always a risk to introduce a minor
regression: that the user starts seeing a MISMATCH instead of SET/SAVED
as a message in Customize.  IMO, that needs to be avoided as much as
possible.

But AFAICS, there will be no trouble here, provided that :value-type is
a repeat of strings.

 > I think the main thing is not to have user options that demand sexps.

I agree.  I'm in favor of avoiding sexp as much as possible, be it by
providing a more specific type, or a not-that-complicated user option.




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-09-25  9:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-24  1:03 [PATCH] Improve find-sibling-rules option type Paul W. Rankin via Emacs development discussions.
2023-09-24  5:33 ` Eli Zaretskii
2023-09-24  9:44   ` Mauro Aranda
2023-09-25  1:17     ` Paul W. Rankin via Emacs development discussions.
2023-09-25  9:52       ` Mauro Aranda
2023-09-24  8:34 ` Philip Kaludercic

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.