Re: [NonGNU ELPA] New package: Denote-Refs

[Philip Kaludercic <philipk@posteo.net>]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Akib Azmain Turja <akib@disroot.org> writes:
>>
>>> Denote-Refs shows links and backlinks inline in a Denote note buffer,
>>> just below the front matter of the note.
>>>
>>> Here is the patch:
>>>
>>> From f4145634df9eb45df7d526871c55326bfafb3785 Mon Sep 17 00:00:00 2001
>>> From: Akib Azmain Turja <akib@disroot.org>
>>> Date: Mon, 19 Dec 2022 22:22:46 +0600
>>> Subject: [PATCH] * elpa-packages (denote-refs): New package.
>>>
>>> ---
>>>  elpa-packages | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/elpa-packages b/elpa-packages
>>> index 9dde563..1d26a5a 100644
>>> --- a/elpa-packages
>>> +++ b/elpa-packages
>>> @@ -108,6 +108,8 @@
>>>    :ignored-files ("LICENSE" "test" "Cask" "Makefile")
>>>    :news "CHANGELOG.md")
>>>
>>> + (denote-refs           :url "https://codeberg.org/akib/emacs-denote-refs")
>>> +
>>>   (devhelp               :url "https://codeberg.org/akib/emacs-devhelp")
>>>
>>>   (diff-ansi		:url "https://codeberg.org/ideasman42/emacs-diff-ansi"
>>> --
>>> 2.37.1
>>
>> Here are a few comments:
>>
>> diff -u /tmp/denote-refs.el.1 /tmp/denote-refs.el
>> --- /tmp/denote-refs.el.1	2022-12-19 19:38:55.000000000 +0100
>> +++ /tmp/denote-refs.el	2022-12-19 17:57:48.712068577 +0100
>
> Wow, nanosecond precision.  :O

I made the mistake of downloading the file via wget, so I had to
download it once more (.1) to create the diff.

>>
>>  ;; This file is not part of GNU Emacs.
>> @@ -36,12 +36,11 @@
>>  (require 'denote)
>>  (require 'subr-x)
>>
>> -(defgroup denote-refs nil
>> +(defgroup denote-refs '()
>
> Why?

It is just a personal preference of mine, since the argument MEMBER is
an alist.  I think that '() makes it more explicit that this is a list,
as opposed to nil which might also be taken to be a truth value.  I
guess () would do as well...

>> @@ -62,7 +61,8 @@
>>
>>  Available sections are `links' and `backlinks', which shows the list
>>  of linked file and the list of backlinks respectively."
>> -  :type '(set (const :tag "Links" links)
>> +  :type '(set :greedy t
>> +	      (const :tag "Links" links)
>>                (const :tag "Backlinks" backlinks)))
>
> What does ':greedy t' mean?

It ensures that the type matches even if the order is changed (which
arguably should always be the case for a set).

> And anyway, I'm going to change that with 'repeat', since
> '(links backlinks)', '(backlinks links)' and '(links backlinks links)'
> all have meanings now.
>
> New type: '(repeat (choice (const :tag "Links" links)
>                            (const :tag "Backlinks" backlinks)))

In that case this is preferable.

> Again the space-tab problem.  Added .dir-locals.el.

1+

>>  (defun denote-refs-update ()
>>    "Update Denote references shown."
>>
>> Diff finished.  Mon Dec 19 17:57:57 2022
>
> Thanks for the information.  I guess you live somewhere at UTC+1, based
> on the email timestamp and diff timestamp.

Right, Germany :)

>>
>
> FYI, you missed the missing ;;;###autoload.  I just added it.

Sorry about that ^^

I'll go ahead and add this to NonGNU ELPA.