GNU bug report logs - #79390
31.0.50; oauth2.el; error in byte-compiling

Previous Next

Full log

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

From: Xiyue Deng <manphiz <at> gmail.com>
To: Kazuhiro Ito <kzhr <at> d1.dion.ne.jp>
Cc: 79390 <at> debbugs.gnu.org, Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#79390: 31.0.50; oauth2.el; error in byte-compiling
Date: Sat, 06 Sep 2025 02:22:50 -0700

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

Hi Ito-san,

Kazuhiro Ito <kzhr <at> d1.dion.ne.jp> writes:

>> >> When I byte-compile oauth2.el (0.18), below errors and warnings are
>> >> displayed and fail to byte-compiling.
> (snip)
>
>> Thanks for the report!  I have now pushed a few fixes which should
>> handle those warnings/errors plus a few other fixes.  I have also added
>> build checks in Gitlab CI.  Now tagged v0.18.1.  Please help check
>> whether those fixes work for you.
>
> I've tested on 0.18.2 and confirmed errors and warnings were fixed,
> thank you.
> But I have one question.  
>
> In oauth2--update-request-cache, 
>
>> (when host-name
>>   (let ((host-name (intern host-name)))
>>     (org-plist-delete request-cache host-name)
>>     (setq request-cache
>>         (plist-put request-cache host-name
>>                    `( :access-token ,access-token
>>                       :request-timestamp ,request-timestamp)))))
>> request-cache)
>
> org-plist-delete returns newly created plist and doesn't modify
> arguement PLIST, so it does nothing in the above code.

Hmm, when I look at the `org-plist-delete' doc in my 30.1 it seems to
suggest it does remove in-place:

,----
| (org-plist-delete PLIST PROPERTY)
| 
| Delete PROPERTY from PLIST.
| This is in contrast to merely setting it to 0.
`----

And on my test in a previous version of code it did seem to work (for my
kinda wrong usage.  See below.)

> Moreover,
> because next plist-put set new property value, it is no need to delete
> old property value explicitly.

I think you are correct here.

[ Well the full story is a bit long: previously I used a string as the
  key for plist, which partially works because you can use `plist-get'
  with `string=' so that you can get the value back, but when doing
  `plist-put' I forgot to add a predicate which ended up with many
  entries with the same string key.  Well I could have fixed it by
  adding the predicate, but it was probably quite late in the night and
  somehow I ended up trying to remove all previous entries before adding
  a new one using `org-plist-delete', which also worked. ]

Anyway, now I use interned account string as keys (which I think should
be faster than using strings with `string=') so `plist-put' should just
DTRT, and `org-plist-delete' is indeed not needed.  I have applied your
patch and initial testing shows it is working fine.  I'll see if there
are any other issues, and tag another release soon.

Thanks again!

>
> Please see attached patch.
>
> -- 
> Kazuhiro Ito
> From 4661b972491084d35c23351bd05438cec05e8440 Mon Sep 17 00:00:00 2001
> From: Kazuhiro Ito <kzhr <at> d1.dion.ne.jp>
> Date: Sat, 6 Sep 2025 15:22:01 +0900
> Subject: [PATCH] * oauth2.el (oauth2--update-request-cache): Remove unneeded
>  code.
>
> ---
>  oauth2.el | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/oauth2.el b/oauth2.el
> index c0aca41c66..612881a615 100644
> --- a/oauth2.el
> +++ b/oauth2.el
> @@ -39,7 +39,6 @@
>  ;;; Code:
>  
>  (eval-when-compile (require 'cl-lib))
> -(require 'org-macs)
>  (require 'plstore)
>  (require 'json)
>  (require 'url-http)
> @@ -122,7 +121,6 @@ HOST-NAME is nil, do nothing.
>  Returns the newly updated request-cache."
>    (when host-name
>      (let ((host-name (intern host-name)))
> -      (org-plist-delete request-cache host-name)
>        (setq request-cache
>            (plist-put request-cache host-name
>                       `( :access-token ,access-token
> -- 
> 2.45.1
>

-- 
Regards,
Xiyue Deng

[signature.asc (application/pgp-signature, inline)]

Previous Next

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