unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Adding 'compat' from ELPA as an optional dependency to ERC
@ 2022-07-19 14:43 F. Jason Park
  2022-07-19 16:17 ` Stefan Monnier
  0 siblings, 1 reply; 10+ messages in thread
From: F. Jason Park @ 2022-07-19 14:43 UTC (permalink / raw)
  To: emacs-devel; +Cc: Amin Bandali, Philip Kaludercic, emacs-erc

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

Hi Emacs,

As an ELPA ":core" package, ERC has a life outside of the emacs.git
tree, where it aims to support the two most recent major releases of
Emacs: currently 27 and 28. Part of that means tending to a menagerie of
adapters here on trunk. However, going forward, we'd like to offload the
bulk of this upkeep to the purpose-built library 'compat'.

The patch below details the changes we'd need, but the crux is really
this addition here:

  @@ -26,6 +26,7 @@
   ;; This mostly defines stuff that cannot be worked around easily.
   
   ;;; Code:
  +(require 'compat nil 'noerror)
   
   ;;;###autoload(autoload 'erc-define-minor-mode "erc-compat")
   (define-obsolete-function-alias 'erc-define-minor-mode

We bring this to your attention in hopes of addressing any concerns that
may arise over its (hopefully imminent) inclusion. FWIW, occasional
external dependencies crop up elsewhere within the Emacs tree. But most,
like `gnus-read-ephemeral-emacs-bug-group', appear to be supplemental
and integrations-focused rather than top-level affairs affecting decent
swaths of code (true in our case for Emacs 27).

I've Cc'd Philip, compat's author, who's far better equipped to field
any nontrivial questions.

Thanks,
J.P.


Related discussions:

  https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01546.html
  https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg00338.html
  https://lists.gnu.org/archive/html/emacs-erc/2022-07/msg00006.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-compat-from-GNU-ELPA-as-a-soft-dependency-in-ERC.patch --]
[-- Type: text/x-patch, Size: 11830 bytes --]

From f41d13548df11ba6d947e364fb893083ecf8d2f8 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 8 Jul 2022 04:58:26 -0700
Subject: [PATCH 1/1] Add compat from GNU ELPA as a soft dependency in ERC

* lisp/erc/erc-backend.el (erc-parse-server-response,
erc--parse-isupport-value): Remove sub-28 compat code involving
`string-search'.

* lisp/erc/erc-compat.el: Require compat package, but don't error
when absent.

* lisp/erc/erc-dcc.el (erc-dcc-member): Remove `string-search' compat
code.
(erc-dcc-unquote-filename): Remove `string-replace' compat code.

* lisp/erc/erc-speedbar.el (erc-speedbar-expand-server,
erc-speedbar-expand-channel, erc-speedbar-expand-user): Remove
`string-search' compat code.

* lisp/erc/erc.el: Add compat version 28.1.2.0 to Package-Requires
header and require `erc-compat' after other libraries.
(erc--valid-local-channel-p): Remove `string-search' compat code.
(erc-update-mode-line-buffer): Remove `string-replace' compat code.
(erc-message-english-PART): Remove `string-replace' compat code.
---
 lisp/erc/erc-backend.el  | 24 ++++++------------------
 lisp/erc/erc-compat.el   |  1 +
 lisp/erc/erc-dcc.el      | 12 ++----------
 lisp/erc/erc-speedbar.el | 24 ++++++------------------
 lisp/erc/erc.el          | 33 +++++++++++----------------------
 5 files changed, 26 insertions(+), 68 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 8be4894ecb..15ecaab76d 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -1012,21 +1012,15 @@ erc-parse-server-response
     (save-match-data
       (let* ((tag-list (when (eq (aref string 0) ?@)
                          (substring string 1
-                                    (if (>= emacs-major-version 28)
-                                        (string-search " " string)
-                                      (string-match " " string)))))
+                                    (string-search " " string))))
              (msg (make-erc-response :unparsed string :tags (when tag-list
                                                               (erc-parse-tags
                                                                tag-list))))
              (string (if tag-list
-                         (substring string (+ 1 (if (>= emacs-major-version 28)
-                                                    (string-search " " string)
-                                                  (string-match " " string))))
+                         (substring string (+ 1 (string-search " " string)))
                        string))
              (posn (if (eq (aref string 0) ?:)
-                       (if (>= emacs-major-version 28)
-                           (string-search " " string)
-                         (string-match " " string))
+                       (string-search " " string)
                      0)))
 
         (setf (erc-response.sender msg)
@@ -1036,9 +1030,7 @@ erc-parse-server-response
 
         (setf (erc-response.command msg)
               (let* ((bposn (string-match "[^ \n]" string posn))
-                     (eposn (if (>= emacs-major-version 28)
-                                (string-search " " string bposn)
-                              (string-match " " string bposn))))
+                     (eposn (string-search " " string bposn)))
                 (setq posn (and eposn
                                 (string-match "[^ \n]" string eposn)))
                 (substring string bposn eposn)))
@@ -1046,9 +1038,7 @@ erc-parse-server-response
         (while (and posn
                     (not (eq (aref string posn) ?:)))
           (push (let* ((bposn posn)
-                       (eposn (if (>= emacs-major-version 28)
-                                  (string-search " " string bposn)
-                                (string-match " " string bposn))))
+                       (eposn (string-search " " string bposn)))
                   (setq posn (and eposn
                                   (string-match "[^ \n]" string eposn)))
                   (substring string bposn eposn))
@@ -1667,9 +1657,7 @@ erc--parse-isupport-value
                      start (- (match-end 0) 3))
              (setq start (match-end 0))))
          v))
-     (if (if (>= emacs-major-version 28)
-             (string-search "," value)
-           (string-match-p "," value))
+     (if (string-search "," value)
          (split-string value ",")
        (list value)))))
 
diff --git a/lisp/erc/erc-compat.el b/lisp/erc/erc-compat.el
index 16cfb15a5a..f24fe746ef 100644
--- a/lisp/erc/erc-compat.el
+++ b/lisp/erc/erc-compat.el
@@ -26,6 +26,7 @@
 ;; This mostly defines stuff that cannot be worked around easily.
 
 ;;; Code:
+(require 'compat nil 'noerror)
 
 ;;;###autoload(autoload 'erc-define-minor-mode "erc-compat")
 (define-obsolete-function-alias 'erc-define-minor-mode
diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index d0e1848e0e..7a24acc433 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -191,9 +191,7 @@ erc-dcc-member
                   test (cadr (plist-member elt prop)))
             ;; if the property exists and is equal, we continue, else, try the
             ;; next element of the list
-            (or (and (eq prop :nick) (if (>= emacs-major-version 28)
-                                         (string-search "!" val)
-                                       (string-match "!" val))
+            (or (and (eq prop :nick) (string-search "!" val)
                      test (string-equal test val))
                 (and (eq prop :nick)
                      test val
@@ -659,13 +657,7 @@ erc-dcc-ctcp-query-send-regexp
 
 (define-inline erc-dcc-unquote-filename (filename)
   (inline-quote
-   (if (>= emacs-major-version 28)
-       (string-replace
-        "\\\\" "\\"
-        (string-replace "\\\"" "\"" ,filename))
-     (replace-regexp-in-string
-      "\\\\\\\\" "\\"
-      (replace-regexp-in-string "\\\\\"" "\"" ,filename t t) t t))))
+   (string-replace "\\\\" "\\" (string-replace "\\\"" "\"" ,filename))))
 
 (defun erc-dcc-handle-ctcp-send (proc query nick login host to)
   "This is called if a CTCP DCC SEND subcommand is sent to the client.
diff --git a/lisp/erc/erc-speedbar.el b/lisp/erc/erc-speedbar.el
index 5b06c21612..19113c5aad 100644
--- a/lisp/erc/erc-speedbar.el
+++ b/lisp/erc/erc-speedbar.el
@@ -139,9 +139,7 @@ erc-speedbar-server-buttons
 	t))))
 
 (defun erc-speedbar-expand-server (text server indent)
-  (cond ((if (>= emacs-major-version 28)
-             (string-search "+" text)
-           (string-match "\\+" text))
+  (cond ((string-search "+" text)
 	 (speedbar-change-expand-button-char ?-)
 	 (if (speedbar-with-writable
 	       (save-excursion
@@ -150,9 +148,7 @@ erc-speedbar-expand-server
 	     (speedbar-change-expand-button-char ?-)
 	   (speedbar-change-expand-button-char ??)))
 	(;; we have to contract this node
-         (if (>= emacs-major-version 28)
-             (string-search "-" text)
-           (string-match "-" text))
+         (string-search "-" text)
 	 (speedbar-change-expand-button-char ?+)
 	 (speedbar-delete-subblock indent))
 	(t (error "Ooops... not sure what to do")))
@@ -189,9 +185,7 @@ erc-speedbar-expand-channel
   "For the line matching TEXT, in CHANNEL, expand or contract a line.
 INDENT is the current indentation level."
   (cond
-   ((if (>= emacs-major-version 28)
-        (string-search "+" text)
-      (string-match "\\+" text))
+   ((string-search "+" text)
     (speedbar-change-expand-button-char ?-)
     (speedbar-with-writable
      (save-excursion
@@ -240,9 +234,7 @@ erc-speedbar-expand-channel
 	     (speedbar-with-writable
 	      (dolist (entry names)
 		(erc-speedbar-insert-user entry ?+ (1+ indent))))))))))
-   ((if (>= emacs-major-version 28)
-        (string-search "-" text)
-      (string-match "-" text))
+   ((string-search "-" text)
     (speedbar-change-expand-button-char ?+)
     (speedbar-delete-subblock indent))
    (t (error "Ooops... not sure what to do")))
@@ -293,9 +285,7 @@ erc-speedbar-update-channel
 	(erc-speedbar-expand-channel "+" buffer 1)))))
 
 (defun erc-speedbar-expand-user (text token indent)
-  (cond ((if (>= emacs-major-version 28)
-             (string-search "+" text)
-           (string-match "\\+" text))
+  (cond ((string-search "+" text)
 	 (speedbar-change-expand-button-char ?-)
 	 (speedbar-with-writable
 	   (save-excursion
@@ -318,9 +308,7 @@ erc-speedbar-expand-user
 		  nil nil nil nil
 		  info nil nil nil
 		  (1+ indent)))))))
-	((if (>= emacs-major-version 28)
-             (string-search "-" text)
-           (string-match "-" text))
+	((string-search "-" text)
 	 (speedbar-change-expand-button-char ?+)
 	 (speedbar-delete-subblock indent))
 	(t (error "Ooops... not sure what to do")))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 0a16831fba..33044f7fef 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -13,7 +13,7 @@
 ;;               Michael Olson (mwolson@gnu.org)
 ;;               Kelvin White (kwhite@gnu.org)
 ;; Version: 5.4.1
-;; Package-Requires: ((emacs "27.1"))
+;; Package-Requires: ((emacs "27.1") (compat "28.1.2.0"))
 ;; Keywords: IRC, chat, client, Internet
 ;; URL: https://www.gnu.org/software/emacs/erc.html
 
@@ -69,6 +69,8 @@
 (require 'iso8601)
 (eval-when-compile (require 'subr-x))
 
+(require 'erc-compat)
+
 (defconst erc-version "5.4.1"
   "This version of ERC.")
 
@@ -3519,9 +3521,7 @@ erc--valid-local-channel-p
   "Non-nil when channel is server-local on a network that allows them."
   (and-let* (((eq ?& (aref channel 0)))
              (chan-types (erc--get-isupport-entry 'CHANTYPES 'single))
-             ((if (>= emacs-major-version 28)
-                  (string-search "&" chan-types)
-                (string-match-p "&" chan-types))))))
+             ((string-search "&" chan-types)))))
 
 (defun erc-cmd-JOIN (channel &optional key)
   "Join the channel given in CHANNEL, optionally with KEY.
@@ -7004,21 +7004,12 @@ erc-update-mode-line-buffer
                                   (fill-region (point-min) (point-max))
                                   (buffer-string))))
                  (setq header-line-format
-                       (if (>= emacs-major-version 28)
-                           (string-replace
-                            "%"
-                            "%%"
-                            (if face
-                                (propertize header 'help-echo help-echo
-                                            'face face)
-                              (propertize header 'help-echo help-echo)))
-                         (replace-regexp-in-string
-                          "%"
-                          "%%"
-                          (if face
-                              (propertize header 'help-echo help-echo
-                                          'face face)
-                            (propertize header 'help-echo help-echo)))))))
+                       (string-replace
+                        "%"
+                        "%%"
+                        (if face
+                            (propertize header 'help-echo help-echo 'face face)
+                          (propertize header 'help-echo help-echo))))))
               (t (setq header-line-format
                        (if face
                            (propertize header 'face face)
@@ -7303,9 +7294,7 @@ erc-message-english-PART
               nick user host channel
               (if (not (string= reason ""))
                   (format ": %s"
-                          (if (>= emacs-major-version 28)
-                              (string-replace "%" "%%" reason)
-                            (replace-regexp-in-string "%" "%%" reason)))
+                          (string-replace "%" "%%" reason))
                 "")))))
 
 
-- 
2.36.1


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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-19 14:43 Adding 'compat' from ELPA as an optional dependency to ERC F. Jason Park
@ 2022-07-19 16:17 ` Stefan Monnier
  2022-07-19 16:25   ` Philip Kaludercic
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2022-07-19 16:17 UTC (permalink / raw)
  To: F. Jason Park; +Cc: emacs-devel, Amin Bandali, Philip Kaludercic, emacs-erc

>    ;;; Code:
>   +(require 'compat nil 'noerror)

I thought the idea for `compat` is that the packages using it would
require a specific (minimum) version, as in

    (require 'compat-28 nil 'noerror)


-- Stefan




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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-19 16:17 ` Stefan Monnier
@ 2022-07-19 16:25   ` Philip Kaludercic
  2022-07-19 22:34     ` J.P.
  0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2022-07-19 16:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: F. Jason Park, emacs-devel, Amin Bandali, emacs-erc

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>    ;;; Code:
>>   +(require 'compat nil 'noerror)
>
> I thought the idea for `compat` is that the packages using it would
> require a specific (minimum) version, as in
>
>     (require 'compat-28 nil 'noerror)

No, requiring 'compat will load all missing definitions up until the
currently version the library supports.  By explicitly loading
compat-28, one would additionally load "prefixed" definitions for
functions that add new features to existing functions.

E.g. consider how assoc acquired a new optional argument "TESTFN" in
version 26.  One would either have to advise the existing function,
which was regarded to be too invasive and dangerous, or to define a
separate function underneath the `compat-' prefix -- `compat-assoc'.

This function is not made visible by loading `compat', and explicitly
ought not to be in this case, as someone using the in-tree version of
ERC would not have no such function defined.  In this sense the support
compat can provide for core packages is limited, but as it seems for
ERC's requirements sufficient.



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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-19 16:25   ` Philip Kaludercic
@ 2022-07-19 22:34     ` J.P.
  2022-07-25 18:56       ` Philip Kaludercic
  0 siblings, 1 reply; 10+ messages in thread
From: J.P. @ 2022-07-19 22:34 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Stefan Monnier, emacs-devel, Amin Bandali, emacs-erc

Hi Philip (and Stefan),

Philip Kaludercic <philipk@posteo.net> writes:

>                                             By explicitly loading
> compat-28, one would additionally load "prefixed" definitions for
> functions that add new features to existing functions.
>
> E.g. consider how assoc acquired a new optional argument "TESTFN" in
> version 26.  One would either have to advise the existing function,
> which was regarded to be too invasive and dangerous, or to define a
> separate function underneath the `compat-' prefix -- `compat-assoc'.
>
> This function is not made visible by loading `compat', and explicitly
> ought not to be in this case, as someone using the in-tree version of
> ERC would not have no such function defined.  In this sense the support
> compat can provide for core packages is limited, but as it seems for
> ERC's requirements sufficient.

I suppose we could always add prefixed definitions down the road if
really needed, perhaps to spare us from having to copy some complicated
backport definition in full. Although, I believe doing so would also
define compat- prefixed variants in all versions of Emacs, the idea
being to save third-party packages from having to do stuff like

  (defmacro erc-compat--json-parse-string (string &rest args)
    `(,(if (fboundp 'compat-json-parse-string)
           'compat-json-parse-string
         'json-parse-string)
      ,string
      ,@args))

But in ERC's case, there'd be no avoiding this kind of indirection.
(Please correct me if that's a flawed understanding.)

Thanks,
J.P.



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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-19 22:34     ` J.P.
@ 2022-07-25 18:56       ` Philip Kaludercic
  2022-07-26  3:04         ` J.P.
  2022-07-26 21:00         ` Copying autoloads (was: Adding 'compat' from ELPA as an optional dependency to ERC) Stefan Monnier
  0 siblings, 2 replies; 10+ messages in thread
From: Philip Kaludercic @ 2022-07-25 18:56 UTC (permalink / raw)
  To: J.P.; +Cc: Stefan Monnier, emacs-devel, Amin Bandali, emacs-erc

"J.P." <jp@neverwas.me> writes:

> Hi Philip (and Stefan),
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>>                                             By explicitly loading
>> compat-28, one would additionally load "prefixed" definitions for
>> functions that add new features to existing functions.
>>
>> E.g. consider how assoc acquired a new optional argument "TESTFN" in
>> version 26.  One would either have to advise the existing function,
>> which was regarded to be too invasive and dangerous, or to define a
>> separate function underneath the `compat-' prefix -- `compat-assoc'.
>>
>> This function is not made visible by loading `compat', and explicitly
>> ought not to be in this case, as someone using the in-tree version of
>> ERC would not have no such function defined.  In this sense the support
>> compat can provide for core packages is limited, but as it seems for
>> ERC's requirements sufficient.
>
> I suppose we could always add prefixed definitions down the road if
> really needed, perhaps to spare us from having to copy some complicated
> backport definition in full. 

There is always the possibility of changing compat to advise certain
functions instead of providing prefixed alternatives.  But that should
probably be decided in a case-by-case basis.

Depending on the interest of the Emacs maintainers in `compat' as a
project, it might also be possible to add compat-* definitions to the
core, thought I am not certain sure how fond I am of this idea.

>                              
>                              Although, I believe doing so would also
> define compat- prefixed variants in all versions of Emacs, the idea
> being to save third-party packages from having to do stuff like
>
>   (defmacro erc-compat--json-parse-string (string &rest args)
>     `(,(if (fboundp 'compat-json-parse-string)
>            'compat-json-parse-string
>          'json-parse-string)
>       ,string
>       ,@args))

If a function like this were to be definied upstream

        (defun get-compatibility-func (name)
          "Return the function NAME of a compatibility alias."
          (let* ((compat (intern-soft (format "compat-%s" name))))
            (or (symbol-function (if (fboundp name) name compat))
                (error "No definition for %S could be found" name))))

then all that ERC would have to do is

        (defalias 'erc-json-parse-string
          (get-compatibility-func 'json-parse-string))

> But in ERC's case, there'd be no avoiding this kind of indirection.
> (Please correct me if that's a flawed understanding.)

While prefixing is used, no, because we cannot assume that compat is
installed.



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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-25 18:56       ` Philip Kaludercic
@ 2022-07-26  3:04         ` J.P.
  2022-07-26 15:17           ` Philip Kaludercic
  2022-07-26 21:00         ` Copying autoloads (was: Adding 'compat' from ELPA as an optional dependency to ERC) Stefan Monnier
  1 sibling, 1 reply; 10+ messages in thread
From: J.P. @ 2022-07-26  3:04 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Stefan Monnier, emacs-devel, Amin Bandali, emacs-erc

Philip Kaludercic <philipk@posteo.net> writes:

> "J.P." <jp@neverwas.me> writes:
>
>>                              Although, I believe doing so would also
>> define compat- prefixed variants in all versions of Emacs, the idea
>> being to save third-party packages from having to do stuff like
>>
>>   (defmacro erc-compat--json-parse-string (string &rest args)
>>     `(,(if (fboundp 'compat-json-parse-string)
>>            'compat-json-parse-string
>>          'json-parse-string)
>>       ,string
>>       ,@args))
>
> If a function like this were to be definied upstream
>
>         (defun get-compatibility-func (name)
>           "Return the function NAME of a compatibility alias."
>           (let* ((compat (intern-soft (format "compat-%s" name))))
>             (or (symbol-function (if (fboundp name) name compat))
>                 (error "No definition for %S could be found" name))))
>
> then all that ERC would have to do is
>
>         (defalias 'erc-json-parse-string
>           (get-compatibility-func 'json-parse-string))
>

Nice. That looks really handy.

(BTW, wouldn't a real life `get-compatibility-func' need some extra
pizzazz to favor the prefixed form for certain versions? Because, as
shown, something like (erc-jason-parse-string "42") would still signal a
`json-parse-error' on Emacs 27, no?)

>> But in ERC's case, there'd be no avoiding this kind of indirection.
>> (Please correct me if that's a flawed understanding.)
>
> While prefixing is used, no, because we cannot assume that compat is
> installed.

Right. Makes sense.

So, getting back to the main thrust of this thread, I guess I'll go
ahead and add the proposed changes to Emacs 29, barring any last-minute
objections. Thanks.



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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-26  3:04         ` J.P.
@ 2022-07-26 15:17           ` Philip Kaludercic
  2022-07-27 12:55             ` J.P.
  0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2022-07-26 15:17 UTC (permalink / raw)
  To: J.P.; +Cc: Stefan Monnier, emacs-devel, Amin Bandali, emacs-erc

"J.P." <jp@neverwas.me> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> "J.P." <jp@neverwas.me> writes:
>>
>>>                              Although, I believe doing so would also
>>> define compat- prefixed variants in all versions of Emacs, the idea
>>> being to save third-party packages from having to do stuff like
>>>
>>>   (defmacro erc-compat--json-parse-string (string &rest args)
>>>     `(,(if (fboundp 'compat-json-parse-string)
>>>            'compat-json-parse-string
>>>          'json-parse-string)
>>>       ,string
>>>       ,@args))
>>
>> If a function like this were to be definied upstream
>>
>>         (defun get-compatibility-func (name)
>>           "Return the function NAME of a compatibility alias."
>>           (let* ((compat (intern-soft (format "compat-%s" name))))
>>             (or (symbol-function (if (fboundp name) name compat))
>>                 (error "No definition for %S could be found" name))))
>>
>> then all that ERC would have to do is
>>
>>         (defalias 'erc-json-parse-string
>>           (get-compatibility-func 'json-parse-string))
>>
>
> Nice. That looks really handy.
>
> (BTW, wouldn't a real life `get-compatibility-func' need some extra
> pizzazz to favor the prefixed form for certain versions? Because, as
> shown, something like (erc-jason-parse-string "42") would still signal a
> `json-parse-error' on Emacs 27, no?)

Yes, you are right, I could imagine using an optional argument to
indicate a minimal version.

>>> But in ERC's case, there'd be no avoiding this kind of indirection.
>>> (Please correct me if that's a flawed understanding.)
>>
>> While prefixing is used, no, because we cannot assume that compat is
>> installed.
>
> Right. Makes sense.
>
> So, getting back to the main thrust of this thread, I guess I'll go
> ahead and add the proposed changes to Emacs 29, barring any last-minute
> objections. Thanks.

If there are no technical objects, I think the only thing that could be
added would be a comment explaining why compat is used or linking to
this thread.?



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

* Copying autoloads (was: Adding 'compat' from ELPA as an optional dependency to ERC)
  2022-07-25 18:56       ` Philip Kaludercic
  2022-07-26  3:04         ` J.P.
@ 2022-07-26 21:00         ` Stefan Monnier
  2022-07-26 23:00           ` Copying autoloads Philip Kaludercic
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2022-07-26 21:00 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: J.P., emacs-devel, Amin Bandali, emacs-erc

Philip Kaludercic [2022-07-25 18:56:39] wrote:
>         (defun get-compatibility-func (name)
>           "Return the function NAME of a compatibility alias."
>           (let* ((compat (intern-soft (format "compat-%s" name))))
>             (or (symbol-function (if (fboundp name) name compat))
>                 (error "No definition for %S could be found" name))))

Side note: if `name` is autoloaded this will not return a "valid" value,
in the sense that the (autoload ...) value stored in `symbol-function`
is only usable when placed inside `name` and not elsewhere (e.g. you
can't `funcall` it, and if you store it in another symbol, calling the
function under that other symbol will probably fail).

Autoloads are somewhat hackish in this sense.


        Stefan




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

* Re: Copying autoloads
  2022-07-26 21:00         ` Copying autoloads (was: Adding 'compat' from ELPA as an optional dependency to ERC) Stefan Monnier
@ 2022-07-26 23:00           ` Philip Kaludercic
  0 siblings, 0 replies; 10+ messages in thread
From: Philip Kaludercic @ 2022-07-26 23:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: J.P., emacs-devel, Amin Bandali, emacs-erc

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Philip Kaludercic [2022-07-25 18:56:39] wrote:
>>         (defun get-compatibility-func (name)
>>           "Return the function NAME of a compatibility alias."
>>           (let* ((compat (intern-soft (format "compat-%s" name))))
>>             (or (symbol-function (if (fboundp name) name compat))
>>                 (error "No definition for %S could be found" name))))
>
> Side note: if `name` is autoloaded this will not return a "valid" value,
> in the sense that the (autoload ...) value stored in `symbol-function`
> is only usable when placed inside `name` and not elsewhere (e.g. you
> can't `funcall` it, and if you store it in another symbol, calling the
> function under that other symbol will probably fail).
>
> Autoloads are somewhat hackish in this sense.

Would getting rid of the `symbol-function' simplify the issue?

Here would be an updated proposal:

--8<---------------cut here---------------start------------->8---
(defun get-compatibility-func (name &optional version)
  "Return the function NAME of a compatibility alias.
If the optional argument VERSION is non-nil, it is interpreted as
a string designating from what version on the actual function may
be used, even if the function may have existed prior to that."
  ;; Compat is an ELPA package that provides compatibility code for
  ;; older versions of Emacs.  To avoid overriding or advising
  ;; existing functionality, updates to existing functions are
  ;; provided by defining a "prefixed" function, starting with
  ;; `compat-'.  The following code will attempt to return a symbol
  ;; designating NAME if it is defined (and allowed according to
  ;; VERSION), otherwise fall back to one provided by Compat.
  ;;
  ;; The assumption here is that either the user is tracking a recent
  ;; enough version of Emacs to have all the necessary definition, or
  ;; they are using a core package updated from ELPA, in which case
  ;; Compat has been added as a dependency.  Otherwise an error is
  ;; signalled as early as possible.
  ;;
  ;; See https://elpa.gnu.org/packages/compat.html for more details.
  (let ((compat (intern-soft (format "compat-%s" name)))
	(func (symbol-function name)))
    (cond ((and (or (null version) (version<= version emacs-version))
		(fboundp name))
	   name)
	  ((fboundp compat) compat)
	  ((error "No definition for %S could be found" name)))))
--8<---------------cut here---------------end--------------->8---

The function could be added to compat.el too, so that any core package
that depends on ELPA can make use of it.



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

* Re: Adding 'compat' from ELPA as an optional dependency to ERC
  2022-07-26 15:17           ` Philip Kaludercic
@ 2022-07-27 12:55             ` J.P.
  0 siblings, 0 replies; 10+ messages in thread
From: J.P. @ 2022-07-27 12:55 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Stefan Monnier, emacs-devel, Amin Bandali, emacs-erc

Philip Kaludercic <philipk@posteo.net> writes:

>> So, getting back to the main thrust of this thread, I guess I'll go
>> ahead and add the proposed changes to Emacs 29, barring any last-minute
>> objections. Thanks.
>
> If there are no technical objects, I think the only thing that could be
> added would be a comment explaining why compat is used or linking to
> this thread.?

I've added a comment along these lines and have gone ahead and installed
the changes. I've also subscribed to the compat-devel mailing list to
hopefully stay abreast of important announcements going forward. Thanks
for all the help and for making and maintaining this library.

J.P.



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

end of thread, other threads:[~2022-07-27 12:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 14:43 Adding 'compat' from ELPA as an optional dependency to ERC F. Jason Park
2022-07-19 16:17 ` Stefan Monnier
2022-07-19 16:25   ` Philip Kaludercic
2022-07-19 22:34     ` J.P.
2022-07-25 18:56       ` Philip Kaludercic
2022-07-26  3:04         ` J.P.
2022-07-26 15:17           ` Philip Kaludercic
2022-07-27 12:55             ` J.P.
2022-07-26 21:00         ` Copying autoloads (was: Adding 'compat' from ELPA as an optional dependency to ERC) Stefan Monnier
2022-07-26 23:00           ` Copying autoloads Philip Kaludercic

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).