bug#39965: [PATCH] Add support for multiple gravatar-like services

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip K]

Specifically, the non-proprietary services libravatar (now default)
and unicornify have been added. The behaviour is customised via the
new variable `gravatar-service'.
---
 lisp/image/gravatar.el            | 19 +++++++++++++++----
 test/lisp/image/gravatar-tests.el |  1 +
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index b8542bc3c3..c1ae0cefe3 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -118,9 +118,16 @@ gravatar-force-default
   :version "27.1"
   :group 'gravatar)
 
-(defconst gravatar-base-url
-  "https://www.gravatar.com/avatar"
-  "Base URL for getting gravatars.")
+(defcustom gravatar-service 'libravatar
+  "Symbol denoting gravatar-like service to use."
+  :type '(choice (const libravatar)
+                 (const gravatar)
+                 (const unicornify))
+  :version "27.1"
+  :link '(url-link "https://www.libravatar.org/")
+  :link '(url-link "https://unicornify.pictures/")
+  :link '(url-link "https://gravatar.com/")
+  :group 'gravatar)
 
 (defun gravatar-hash (mail-address)
   "Return the Gravatar hash for MAIL-ADDRESS."
@@ -142,7 +149,11 @@ gravatar-build-url
   "Return the URL of a gravatar for MAIL-ADDRESS."
   ;; https://gravatar.com/site/implement/images/
   (format "%s/%s?%s"
-          gravatar-base-url
+          (cond ((eq gravatar-service 'gravatar)
+                 "https://www.gravatar.com/avatar")
+                ((eq gravatar-service 'unicornify)
+                 "https://unicornify.pictures/avatar/")
+                (t "https://seccdn.libravatar.org/avatar"))
           (gravatar-hash mail-address)
           (gravatar--query-string)))
 
diff --git a/test/lisp/image/gravatar-tests.el b/test/lisp/image/gravatar-tests.el
index e66b5c6803..31a28293fa 100644
--- a/test/lisp/image/gravatar-tests.el
+++ b/test/lisp/image/gravatar-tests.el
@@ -65,6 +65,7 @@ gravatar-build-url
   "Test `gravatar-build-url'."
   (let ((gravatar-default-image nil)
         (gravatar-force-default nil)
+        (gravatar-service 'gravatar)
         (gravatar-size nil))
     (should (equal (gravatar-build-url "foo") "\
 https://www.gravatar.com/avatar/acbd18db4cc2f85cedef654fccc4a4d8?r=g"))))
-- 
2.20.1

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Eli Zaretskii]

> From: Philip K <philip@warpmail.net>
> Date: Sat,  7 Mar 2020 01:11:41 +0100
> Cc: philip.kaludercic@fau.de
> 
> Specifically, the non-proprietary services libravatar (now default)
> and unicornify have been added. The behaviour is customised via the
> new variable `gravatar-service'.

Thanks.  I will let the Gnus folks to review this, but I have one nit:

> +  :version "27.1"

This should say "28.1", as the emacs-27 branch doesn't accept new
features.

Also, we need a NEWS entry describing the new option(s), and possibly
also a corresponding change for the Gnus manual.

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Sat,  7 Mar 2020 01:11:41 +0100, Philip K <philip@warpmail.net> said:

    Philip> Specifically, the non-proprietary services libravatar (now default)
    Philip> and unicornify have been added. The behaviour is customised via the
    Philip> new variable `gravatar-service'.
    Philip> ---
    Philip>  lisp/image/gravatar.el            | 19 +++++++++++++++----
    Philip>  test/lisp/image/gravatar-tests.el |  1 +
    Philip>  2 files changed, 16 insertions(+), 4 deletions(-)

    Philip> diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
    Philip> index b8542bc3c3..c1ae0cefe3 100644
    Philip> --- a/lisp/image/gravatar.el
    Philip> +++ b/lisp/image/gravatar.el
    Philip> @@ -118,9 +118,16 @@ gravatar-force-default
    Philip>    :version "27.1"
    Philip>    :group 'gravatar)
 
    Philip> -(defconst gravatar-base-url
    Philip> -  "https://www.gravatar.com/avatar"
    Philip> -  "Base URL for getting gravatars.")
    Philip> +(defcustom gravatar-service 'libravatar
    Philip> +  "Symbol denoting gravatar-like service to use."
    Philip> +  :type '(choice (const libravatar)
    Philip> +                 (const gravatar)
    Philip> +                 (const unicornify))
    Philip> +  :version "27.1"
    Philip> +  :link '(url-link "https://www.libravatar.org/")
    Philip> +  :link '(url-link "https://unicornify.pictures/")
    Philip> +  :link '(url-link "https://gravatar.com/")
    Philip> +  :group 'gravatar)

If you make this an alist then your cond below becomes just
'alist-get', and you have the URLS in just one place.

    Philip>  (defun gravatar-hash (mail-address)
    Philip>    "Return the Gravatar hash for MAIL-ADDRESS."
    Philip> @@ -142,7 +149,11 @@ gravatar-build-url
    Philip>    "Return the URL of a gravatar for MAIL-ADDRESS."
    Philip>    ;; https://gravatar.com/site/implement/images/
    Philip>    (format "%s/%s?%s"
    Philip> -          gravatar-base-url
    Philip> +          (cond ((eq gravatar-service 'gravatar)
    Philip> +                 "https://www.gravatar.com/avatar")
    Philip> +                ((eq gravatar-service 'unicornify)
    Philip> +                 "https://unicornify.pictures/avatar/")
    Philip> +                (t "https://seccdn.libravatar.org/avatar"))
    Philip>            (gravatar-hash mail-address)
    Philip>            (gravatar--query-string)))

This is changing the default from gravatar->libravatar, won't that
break stuff?

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

(please keep 39965@debbugs.gnu.org in the CC list)

>>>>> On Mon, 09 Mar 2020 15:48:59 +0100, Philip Kaludercic <philip.kaludercic@fau.de> said:

    >> 
    >> If you make this an alist then your cond below becomes just
    >> 'alist-get', and you have the URLS in just one place.

    Philip> That's right, I had already "fixed" that in the next version of the
    Philip> patch, but primarily because libravatar requires a bit more to work
    Philip> correctly than just alist-get.

OK

    Philip> Sadly I cannot send the patch right now, because git-send-email doesn't
    Philip> work from where I am writing.

git format-patch HEAD~ + attaching the result will work as
well, assuming your MUA doesnʼt mangle patches :-)

    >> This is changing the default from gravatar->libravatar, won't that
    >> break stuff?

    Philip> All Gravatar avatars should still be resolved as they have been until
    Philip> now (just via Libravatar's servers). The only user-visible change is
    Philip> that people who just use Libravatar will also have avatar images.

    Philip> If there is too much Resistance towards changing this, it shouldn't be a
    Philip> problem to keep gravatar the default, but as mentioned in the commit
    Philip> message, I think that it is somewhat dubious for *Emacs* to default to a
    Philip> proprietary service.

If the avatars are still resolved, then I think thereʼs no issue with
changing the default (especially considering it wasnʼt configurable
before). Thanks for clarifying.

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip Kaludercic]

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

Robert Pluim <rpluim@gmail.com> writes:

> (please keep 39965@debbugs.gnu.org in the CC list)

Sorry for that!

>>>>>> On Mon, 09 Mar 2020 15:48:59 +0100, Philip Kaludercic <philip.kaludercic@fau.de> said:
>
>     >> 
>     >> If you make this an alist then your cond below becomes just
>     >> 'alist-get', and you have the URLS in just one place.
>
>     Philip> That's right, I had already "fixed" that in the next version of the
>     Philip> patch, but primarily because libravatar requires a bit more to work
>     Philip> correctly than just alist-get.
>
> OK
>
>     Philip> Sadly I cannot send the patch right now, because git-send-email doesn't
>     Philip> work from where I am writing.
>
> git format-patch HEAD~ + attaching the result will work as
> well, assuming your MUA doesnʼt mangle patches :-)

In that case I'm attaching the patch to this mail, I hope Gnus handels
it corretly. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-multiple-gravatar-like-services.patch --]
[-- Type: text/x-diff, Size: 3790 bytes --]

From 5c967bc9e2c3bc72e3803d84b23b8b121328c49c Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Fri, 6 Mar 2020 12:40:28 +0100
Subject: [PATCH] Add support for multiple gravatar-like services

Specifically, the non-proprietary services libravatar (now default)
and unicornify have been added. The behaviour is customised via the
new variable `gravatar-service'.
---
 etc/NEWS                          |  7 ++++++
 lisp/image/gravatar.el            | 37 +++++++++++++++++++++++++++----
 test/lisp/image/gravatar-tests.el |  1 +
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7f70d149d6..24edf2de6a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -171,6 +171,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** gravatar.el
+
+---
+*** New variable `gravatar-service' changes avatar service.
+Now supports Libravatar (as default) and Unicornify, next to the
+already implemented Gravatar.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index b8542bc3c3..4ac545730a 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -118,9 +118,37 @@ gravatar-force-default
   :version "27.1"
   :group 'gravatar)
 
-(defconst gravatar-base-url
-  "https://www.gravatar.com/avatar"
-  "Base URL for getting gravatars.")
+(defconst gravatar-service-alist
+  `((gravatar . ,(lambda (_addr) "https://www.gravatar.com/avatar"))
+    (unicornify . ,(lambda (_addr) "https://unicornify.pictures/avatar/"))
+    (libravatar . ,#'gravatar--service-libravatar))
+  "Alist of supported gravatar services.")
+
+(defcustom gravatar-service 'libravatar
+  "Symbol denoting gravatar-like service to use.
+Note that certain services might ignore other options, such as
+`gravatar-default-image' or certain values as with
+`gravatar-rating'."
+  :type `(choice ,@(mapcar (lambda (s) `(const ,(car s)))
+                           gravatar-service-alist))
+  :version "28.1"
+  :link '(url-link "https://www.libravatar.org/")
+  :link '(url-link "https://unicornify.pictures/")
+  :link '(url-link "https://gravatar.com/")
+  :group 'gravatar)
+
+(defun gravatar--service-libravatar (addr)
+  "Find domain that hosts avatars for email address ADDR."
+  ;; implements https://wiki.libravatar.org/api/
+  (require 'dns)
+  (let* ((domain (save-match-data
+                   (unless (string-match ".+@\\(.+\\)" addr)
+                     (error "%s is not an email address" addr))
+                   (match-string 1 addr)))
+         (result (dns-query (concat "_avatars._tcp." domain) 'SRV)))
+    (if result
+        (concat "http://" result "/address")
+      "https://seccdn.libravatar.org/avatar")))
 
 (defun gravatar-hash (mail-address)
   "Return the Gravatar hash for MAIL-ADDRESS."
@@ -142,7 +170,8 @@ gravatar-build-url
   "Return the URL of a gravatar for MAIL-ADDRESS."
   ;; https://gravatar.com/site/implement/images/
   (format "%s/%s?%s"
-          gravatar-base-url
+          (funcall (cdr (assq gravatar-service gravatar-service-alist))
+                   mail-address)
           (gravatar-hash mail-address)
           (gravatar--query-string)))
 
diff --git a/test/lisp/image/gravatar-tests.el b/test/lisp/image/gravatar-tests.el
index e66b5c6803..31a28293fa 100644
--- a/test/lisp/image/gravatar-tests.el
+++ b/test/lisp/image/gravatar-tests.el
@@ -65,6 +65,7 @@ gravatar-build-url
   "Test `gravatar-build-url'."
   (let ((gravatar-default-image nil)
         (gravatar-force-default nil)
+        (gravatar-service 'gravatar)
         (gravatar-size nil))
     (should (equal (gravatar-build-url "foo") "\
 https://www.gravatar.com/avatar/acbd18db4cc2f85cedef654fccc4a4d8?r=g"))))
-- 
2.20.1

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Mon, 09 Mar 2020 16:10:48 +0100, Philip Kaludercic <philip.kaludercic@fau.de> said:

    Philip> In that case I'm attaching the patch to this mail, I hope Gnus handels
    Philip> it corretly. 

I hand't realised you were using the One True Emacs Mail Agent. Of
course it works.

    Philip> From 5c967bc9e2c3bc72e3803d84b23b8b121328c49c Mon Sep 17 00:00:00 2001
    Philip> From: Philip K <philip@warpmail.net>
    Philip> Date: Fri, 6 Mar 2020 12:40:28 +0100
    Philip> Subject: [PATCH] Add support for multiple gravatar-like services

    Philip> Specifically, the non-proprietary services libravatar (now default)
    Philip> and unicornify have been added. The behaviour is customised via the
    Philip> new variable `gravatar-service'.

This is missing a ChangeLog style comment. There are various ways to
produce that, the easiest is probably:

C-x v d ; vc-dir
m ; mark the files you want to commit
v ; vc-next-action, which will put you in a commit message buffer
C-c C-w ; this will look through the diffs of the files youʼre
        ; commiting and pre-populate the commit message with
        ; file/function/variable names. You then fill in the details
        ; of what you changed.

Of course, a preamble like you had explaining the 'why' of the change
is also good.

See "** Commit messages" in CONTRIBUTE for a more detailed description
of the desired format for commit messages.

    Philip> +---
    Philip> +*** New variable `gravatar-service' changes avatar service.
    Philip> +Now supports Libravatar (as default) and Unicornify, next to the
    Philip> +already implemented Gravatar.
    Philip> +

We normally call these 'user options', and donʼt use back-quotes in
NEWS, and try to have the first line be a complete sentence, so:

** Gravatar

===
*** New user-option 'gravatar-service' for host to query for gravatars.
Defaults to Libravatar, with Unicornify and Gravatar as options.

Thanks for your patch.

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip K.]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 09 Mar 2020 16:10:48 +0100, Philip Kaludercic <philip.kaludercic@fau.de> said:
>
>     Philip> From 5c967bc9e2c3bc72e3803d84b23b8b121328c49c Mon Sep 17 00:00:00 2001
>     Philip> From: Philip K <philip@warpmail.net>
>     Philip> Date: Fri, 6 Mar 2020 12:40:28 +0100
>     Philip> Subject: [PATCH] Add support for multiple gravatar-like services
>
>     Philip> Specifically, the non-proprietary services libravatar (now default)
>     Philip> and unicornify have been added. The behaviour is customised via the
>     Philip> new variable `gravatar-service'.
>
> This is missing a ChangeLog style comment. There are various ways to
> produce that, the easiest is probably:
>
> C-x v d ; vc-dir
> m ; mark the files you want to commit
> v ; vc-next-action, which will put you in a commit message buffer
> C-c C-w ; this will look through the diffs of the files youʼre
>         ; commiting and pre-populate the commit message with
>         ; file/function/variable names. You then fill in the details
>         ; of what you changed.
>
> Of course, a preamble like you had explaining the 'why' of the change
> is also good.
>
> See "** Commit messages" in CONTRIBUTE for a more detailed description
> of the desired format for commit messages.

I have been trying what you mention and what the "Commit message"
section describes, but I just cannot execute C-c C-w, nor
can I find it anywhere. What command is it supposed to call? We are
talking about the "*vc-log*" buffer, aren't we?

> We normally call these 'user options', and donʼt use back-quotes in
> NEWS, and try to have the first line be a complete sentence, so:
>
> ** Gravatar
>
> ===
> *** New user-option 'gravatar-service' for host to query for gravatars.
> Defaults to Libravatar, with Unicornify and Gravatar as options.

I'll just use your log message, it sound a lot better.

-- 
	With kind regards,
	Philip K.

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Noam Postavsky]

philip@warpmail.net (Philip K.) writes:

> I have been trying what you mention and what the "Commit message"
> section describes, but I just cannot execute C-c C-w, nor
> can I find it anywhere. What command is it supposed to call? We are
> talking about the "*vc-log*" buffer, aren't we?

Assuming you are running Emacs 27, then yes, it should work in the
*vc-log* buffer.

    C-c C-w runs the command log-edit-generate-changelog-from-diff (found
    in vc-git-log-edit-mode-map), which is an interactive compiled Lisp
    function in ‘log-edit.el’.

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip K.]

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


Sorry for the delay, I hope the commit message is ok now.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-multiple-Gravatar-services.patch --]
[-- Type: text/x-diff, Size: 3442 bytes --]

From 45ef168f4c7b2c0195de040570fa613c89e41ae4 Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Tue, 10 Mar 2020 10:21:08 +0100
Subject: [PATCH] Add support for multiple Gravatar services

Now supports Libravatar and Unicornify, next to Gravatar.

* lisp/image/gravatar.el (gravatar-base-url): Remove constant.
(gravatar-service-alist): List supported services.
(gravatar-service): Add user option to specify service, defaults to
Libravatar.
(gravatar--service-libravatar): Add libravatar image host resolver
implementation.
(gravatar-build-url): Use alist gravatar-service-alist instead of
gravatar-base-url.
* etc/NEWS: Add note.
---
 etc/NEWS               |  6 ++++++
 lisp/image/gravatar.el | 37 +++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7f70d149d6..7d9f858555 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -171,6 +171,12 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** Gravatar
+
+===
+*** New user-option 'gravatar-service' for host to query for gravatars.
+Defaults to Libravatar, with Unicornify and Gravatar as options.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index b8542bc3c3..9b4e93275d 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -118,9 +118,37 @@ gravatar-force-default
   :version "27.1"
   :group 'gravatar)
 
-(defconst gravatar-base-url
-  "https://www.gravatar.com/avatar"
-  "Base URL for getting gravatars.")
+(defconst gravatar-service-alist
+  `((gravatar . ,(lambda (_addr) "https://www.gravatar.com/avatar"))
+    (unicornify . ,(lambda (_addr) "https://unicornify.pictures/avatar/"))
+    (libravatar . ,#'gravatar--service-libravatar))
+  "Alist of supported gravatar services.")
+
+(defcustom gravatar-service 'libravatar
+  "Symbol denoting gravatar-like service to use.
+Note that certain services might ignore other options, such as
+`gravatar-default-image' or certain values as with
+`gravatar-rating'."
+  :type `(choice ,@(mapcar (lambda (s) `(const ,(car s)))
+                           gravatar-service-alist))
+  :version "28.1"
+  :link '(url-link "https://www.libravatar.org/")
+  :link '(url-link "https://unicornify.pictures/")
+  :link '(url-link "https://gravatar.com/")
+  :group 'gravatar)
+
+(defun gravatar--service-libravatar (addr)
+  "Find domain that hosts avatars for email address ADDR."
+  ;; implements https://wiki.libravatar.org/api/
+  (require 'dns)
+  (let* ((domain (save-match-data
+                   (unless (string-match ".+@\\(.+\\)" addr)
+                     (error "%s is not an email address" addr))
+                   (match-string 1 addr)))
+         (result (dns-query (concat "_avatars._tcp." domain) 'SRV)))
+    (if result
+        (concat "http://" result "/address")
+      "https://seccdn.libravatar.org/avatar")))
 
 (defun gravatar-hash (mail-address)
   "Return the Gravatar hash for MAIL-ADDRESS."
@@ -142,7 +170,8 @@ gravatar-build-url
   "Return the URL of a gravatar for MAIL-ADDRESS."
   ;; https://gravatar.com/site/implement/images/
   (format "%s/%s?%s"
-          gravatar-base-url
+          (funcall (alist-get gravatar-service gravatar-service-alist)
+                   mail-address)
           (gravatar-hash mail-address)
           (gravatar--query-string)))
 
-- 
2.20.1

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Tue, 10 Mar 2020 10:31:27 +0100, Philip K. <philip.kaludercic@fau.de> said:

    Philip> Sorry for the delay, I hope the commit message is ok now.

Thanks, much better. Eli, is this a small enough change to commit as
'Copyright-paperwork-exempt'?

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip Kaludercic]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Tue, 10 Mar 2020 10:31:27 +0100, Philip K. <philip.kaludercic@fau.de> said:
>
>     Philip> Sorry for the delay, I hope the commit message is ok now.
>
> Thanks, much better. Eli, is this a small enough change to commit as
> 'Copyright-paperwork-exempt'?
>
> Robert

I actually signed the copyright assignment last year, for contributing
to an ELPA package. I thought that applied to the Emacs core too?

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 39965@debbugs.gnu.org, eliz@gnu.org
> Date: Wed, 11 Mar 2020 08:38:32 +0100
> 
> >>>>> On Tue, 10 Mar 2020 10:31:27 +0100, Philip K. <philip.kaludercic@fau.de> said:
> 
>     Philip> Sorry for the delay, I hope the commit message is ok now.
> 
> Thanks, much better. Eli, is this a small enough change to commit as
> 'Copyright-paperwork-exempt'?

There's no need, since Philip has an assignment on file.

But I didn't understand how was the issue of preferring Libravatar
resolved?  Didn't someone say it wasn't a free service?

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Eli Zaretskii]

> From: Philip Kaludercic <philip.kaludercic@fau.de>
> Cc: 39965@debbugs.gnu.org,  eliz@gnu.org
> Date: Wed, 11 Mar 2020 09:50:08 +0100
> 
> I actually signed the copyright assignment last year, for contributing
> to an ELPA package. I thought that applied to the Emacs core too?

The assignment is for Emacs, and it applies to ELPA as well.

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip K.]

Eli Zaretskii <eliz@gnu.org> writes:

> But I didn't understand how was the issue of preferring Libravatar
> resolved?  Didn't someone say it wasn't a free service?

No, I might have said that Gravatar is non-free, and as such shouldn't
be the default.

-- 
	With kind regards,
	Philip K.

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Wed, 11 Mar 2020 18:00:06 +0100, philip@warpmail.net (Philip K.) said:

    Philip> Eli Zaretskii <eliz@gnu.org> writes:
    >> But I didn't understand how was the issue of preferring Libravatar
    >> resolved?  Didn't someone say it wasn't a free service?

    Philip> No, I might have said that Gravatar is non-free, and as such shouldn't
    Philip> be the default.

Right, and the default will become Libravatar, which is a free
service.

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Tue, 10 Mar 2020 10:31:27 +0100, Philip K. <philip.kaludercic@fau.de> said:

    Philip> Sorry for the delay, I hope the commit message is ok now.

Sorry for my delay in getting back to you. I have one question:

    Philip> +(defun gravatar--service-libravatar (addr)
    Philip> +  "Find domain that hosts avatars for email address ADDR."
    Philip> +  ;; implements https://wiki.libravatar.org/api/
    Philip> +  (require 'dns)
    Philip> +  (let* ((domain (save-match-data
    Philip> +                   (unless (string-match ".+@\\(.+\\)" addr)
    Philip> +                     (error "%s is not an email address" addr))
    Philip> +                   (match-string 1 addr)))
    Philip> +         (result (dns-query (concat "_avatars._tcp." domain) 'SRV)))
    Philip> +    (if result
    Philip> +        (concat "http://" result "/address")
    Philip> +      "https://seccdn.libravatar.org/avatar")))

Here you do the SRV lookup for the TCP service, but I believe thereʼs
an HTTPS service type available as well. Could you look that up first,
then the TCP one, then fall back to the default libravatar URL?

Robert

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Philip K.]

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


Robert Pluim <rpluim@gmail.com> writes:

> Sorry for my delay in getting back to you. I have one question:
>
> [...]
>
> Here you do the SRV lookup for the TCP service, but I believe thereʼs
> an HTTPS service type available as well. Could you look that up first,
> then the TCP one, then fall back to the default libravatar URL?

You're right, I missed that. This patch checks the avatar-sec and avatar
record. I rewrote it a bit, to avoid duplication, so I hope the style is
ok.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-multiple-Gravatar-services.patch --]
[-- Type: text/x-diff, Size: 3442 bytes --]

From 45ef168f4c7b2c0195de040570fa613c89e41ae4 Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Tue, 10 Mar 2020 10:21:08 +0100
Subject: [PATCH] Add support for multiple Gravatar services

Now supports Libravatar and Unicornify, next to Gravatar.

* lisp/image/gravatar.el (gravatar-base-url): Remove constant.
(gravatar-service-alist): List supported services.
(gravatar-service): Add user option to specify service, defaults to
Libravatar.
(gravatar--service-libravatar): Add libravatar image host resolver
implementation.
(gravatar-build-url): Use alist gravatar-service-alist instead of
gravatar-base-url.
* etc/NEWS: Add note.
---
 etc/NEWS               |  6 ++++++
 lisp/image/gravatar.el | 37 +++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7f70d149d6..7d9f858555 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -171,6 +171,12 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** Gravatar
+
+===
+*** New user-option 'gravatar-service' for host to query for gravatars.
+Defaults to Libravatar, with Unicornify and Gravatar as options.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index b8542bc3c3..9b4e93275d 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -118,9 +118,37 @@ gravatar-force-default
   :version "27.1"
   :group 'gravatar)
 
-(defconst gravatar-base-url
-  "https://www.gravatar.com/avatar"
-  "Base URL for getting gravatars.")
+(defconst gravatar-service-alist
+  `((gravatar . ,(lambda (_addr) "https://www.gravatar.com/avatar"))
+    (unicornify . ,(lambda (_addr) "https://unicornify.pictures/avatar/"))
+    (libravatar . ,#'gravatar--service-libravatar))
+  "Alist of supported gravatar services.")
+
+(defcustom gravatar-service 'libravatar
+  "Symbol denoting gravatar-like service to use.
+Note that certain services might ignore other options, such as
+`gravatar-default-image' or certain values as with
+`gravatar-rating'."
+  :type `(choice ,@(mapcar (lambda (s) `(const ,(car s)))
+                           gravatar-service-alist))
+  :version "28.1"
+  :link '(url-link "https://www.libravatar.org/")
+  :link '(url-link "https://unicornify.pictures/")
+  :link '(url-link "https://gravatar.com/")
+  :group 'gravatar)
+
+(defun gravatar--service-libravatar (addr)
+  "Find domain that hosts avatars for email address ADDR."
+  ;; implements https://wiki.libravatar.org/api/
+  (require 'dns)
+  (let* ((domain (save-match-data
+                   (unless (string-match ".+@\\(.+\\)" addr)
+                     (error "%s is not an email address" addr))
+                   (match-string 1 addr)))
+         (result (dns-query (concat "_avatars._tcp." domain) 'SRV)))
+    (if result
+        (concat "http://" result "/address")
+      "https://seccdn.libravatar.org/avatar")))
 
 (defun gravatar-hash (mail-address)
   "Return the Gravatar hash for MAIL-ADDRESS."
@@ -142,7 +170,8 @@ gravatar-build-url
   "Return the URL of a gravatar for MAIL-ADDRESS."
   ;; https://gravatar.com/site/implement/images/
   (format "%s/%s?%s"
-          gravatar-base-url
+          (funcall (alist-get gravatar-service gravatar-service-alist)
+                   mail-address)
           (gravatar-hash mail-address)
           (gravatar--query-string)))
 
-- 
2.20.1

bug#39965: [PATCH] Add support for multiple gravatar-like services

[Robert Pluim]

>>>>> On Tue, 17 Mar 2020 19:06:19 +0100, philip@warpmail.net (Philip K.) said:

    Philip> Robert Pluim <rpluim@gmail.com> writes:
    >> This one still checks only the avatar record (and is it avatar-sec or
    >> avatar_sec?).

    Philip> Oops, I seem to have selected the wrong patch. I double checked this
    Philip> time. And regarding your question, according to [0] it has to be
    Philip> "avatar-sec.". Sorry for all the trouble!

Thanks. Pushed to master as 421eeff243 (with my typo in NEWS fixed).

Closing bug.

Robert