~protesilaos/denote

23 2

[PATCH] feat: project.el integration & backlinks refactoring with xref

Details
Message ID
<86a64ooxyi.fsf@nobiot.com>
DKIM signature
missing
Download raw message
Hi Prot,

Following from my previous patches, I have created another one. The
intent of this new patch is to deepen integration with the built-in Xref
for the backlinks buffer.

In addtion, I have also implemented integration with built-in
project.el.  I touched on this in passing in my last patch; many
functions of Xref rely on project.el.  The project integration was
necessary to move further with this line of changes.  Beyond necessity,
I believe it opens up some avenues for the benefits of users.

As with my other patches, please treat this one as a discussion material
with concrete implementations.  It is fine for me if the project decides
not to implement it -- it might be reasonably considred that it adds
complexity and dependencies, albeing these on built-in packages.

From my point of view, the changes in this patch are in line with the
"composability" principle of Denote, and enhances "simple-to-use" user
experiences without compromising the "hackability" and "flexibility"
principles.

The summary and details of the changes are in the patch attached.

Thank you.
nobiot
Details
Message ID
<878rk7exwo.fsf@protesilaos.com>
In-Reply-To
<86a64ooxyi.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Fri, 18 Nov 2022 20:37:57 +0100
>
> Hi Prot,

Hello nobiot and thank you for doing this!

This is just to confirm that I have read your message.  I will read the
patch tomorrow and report back to you.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<865yfabwwy.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
 > Hello nobiot and thank you for doing this!

 You are most welcome :)

 > This is just to confirm that I have read your message.  I will read the
 > patch tomorrow and report back to you.

 Please take your time.  If you like the direction, I have another set of
 patches: some fixes and another to integrate further with Xref.  I've
 been playing with Xref with Denote today and am really excited about how
 fluid navigation has become between notes via links with using Xref,
 just like you would jump between Elisp files for definitions and
 references with M-. and M-?.

 I will send them if you like the first one you have got now.  Let me
 know how you go.  Please do not worry about communicating the negative
 with me if you do not approve of the direction.  I have learned a ton
 about the built-in Xref and Project libraries; now I'm now a fan of
 them.  I hope you will be too :)

 nobiot
Details
Message ID
<87fseegsld.fsf@protesilaos.com>
In-Reply-To
<865yfabwwy.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Sat, 19 Nov 2022 19:51:09 +0100

Hello nobiot,

>  > Hello nobiot and thank you for doing this!
>
>  You are most welcome :)
>
>  > This is just to confirm that I have read your message.  I will read the
>  > patch tomorrow and report back to you.
>
>  Please take your time.  If you like the direction, I have another set of
>  patches: some fixes and another to integrate further with Xref.  I've
>  been playing with Xref with Denote today and am really excited about how
>  fluid navigation has become between notes via links with using Xref,
>  just like you would jump between Elisp files for definitions and
>  references with M-. and M-?.
>
>  I will send them if you like the first one you have got now.  Let me
>  know how you go.  Please do not worry about communicating the negative
>  with me if you do not approve of the direction.  I have learned a ton
>  about the built-in Xref and Project libraries; now I'm now a fan of
>  them.  I hope you will be too :)

I like this direction and the benefits it provides!  Please proceed.

Below is a comment on your current patch:

    - One minor annoyance may be that the prompt now requires a confirmation
      if the user enters text that does not match any of the candidate and
      tries to exit

    - 'denote--title-history' is directly updated from the minibuffer
      completion function.  This eliminates the need for
      'denote--push-extracted-title-to-history'.  The original used
      'file-name-history' as the intermediate storage of titles, which are
      not really file names, thus resulted in polluting the history for file
      names

I understand that the confirmation will affect the "do or create"
commands, right?  Perhaps we can find a way to circumvent that step.

As for the history extraction, the idea was to re-use the last input in
those "do or create" commands.  So the user would type terms for a file
that does not exist and could then retrieve those terms in the next
prompt for the creation of a new note.  Does your change retain this
behaviour or would we need to make another arrangement?  Below I copy
the relevant commit message:

    commit 17dc71f7b560fbd06c1f797a35ea85131902ed41
    Author: Protesilaos Stavrou <info@protesilaos.com>
    Date:   Mon Oct 24 08:38:17 2022 +0300

        Allow denote-link-or-create to leverage last input

        I am implementing this functionality in response to a request made by
        Guo Yong: <https://lists.sr.ht/~protesilaos/denote/%3CNF6pFBq--3-9%40tutanota.com%3E>.

        The idea is to enhance the "link or create+link" functionality so that
        when the user searches for a file that does not exit, they can re-use
        the search terms they used initially.

        As I explained on the mailing list:

            A non-intrusive way of addressing this issue is to leverage the
            minibuffer histories.  This allows us to type 'M-p' (the command is
            'previous-history-element') at Denote's title prompt and get the last
            input.  The diff further below makes this possible, but I am also
            copying the code for your convenience.

            Some notes before you check the code:

            1. We should not hardcode prompts, such as for file and keywords because
               that will contradict the user option 'denote-prompts'.  Concretely, a
               user may want to create+link to a note that has a specific date, is
               in a given subdirectory, and starts with a certain template.

            2. The input we provide at the standard file prompt should not be taken
               literally.  The reason is that it is common for a user to rely on the
               completion style to narrow the list of candidates.  They are not
               necessarily typing the exact title they would like.  For example, I
               use the 'orderless' package (among built-in options) and will often
               type searches out-of-order like "_journal test this" which would
               match a file that is "20221024T062254--this-is-a-test__journal.txt".

               By adding the last search to the minibuffer history, we let the user
               edit the title before they submit it.

        Thanks to Guo Yong for testing my prototype and confirming it does
        what we need.

With those granted, I think we are on the right path.  We will just need
to be sure that this all works with the required Emacs version of 28.1.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<867czq2ew7.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
My previous reply does not seem to go through to the mailing list
archive -- perhaps because of the video attachment.  For the benefit of
the community, I'm re-sending just the text.  I'd appreciate it it if
anyone has an explanation of this behaviour, and if the issue is indeed
the video), how I can share such video here.

It might well be that I should have just waited longer.

nobiot



 Protesilaos Stavrou <info@protesilaos.com> writes:

 > I like this direction and the benefits it provides!  Please proceed.

 Thanks for confirming.  I have done some testing on the patch (without
 the additional ones I have yet to send to you) with Emacs 28.1 with the
 "-q" option to respond to your comments.  To illustrate, I'm attaching a
 short video demonstration (.mp4 file).  Briefly, the sequence of the
 demo is as follows:

 1. Call 'denote-open-or-create' and show that notes in sub-directories of
    'denote-directory' are displayed

 2. Enter a new title name to create a new note.
    You will see a [Confirm] prompt briefly.

 3. In the next "File title" prompt, select the title from the title
    history

 > Below is a comment on your current patch:
 >     - One minor annoyance may be that the prompt now requires a confirmation
 [ 9 more citation lines. Click/Enter to show. ]
 >       if the user enters text that does not match any of the candidate and
 >       tries to exit
 >
 >     - 'denote--title-history' is directly updated from the minibuffer
 >       completion function.  This eliminates the need for
 >       'denote--push-extracted-title-to-history'.  The original used
 >       'file-name-history' as the intermediate storage of titles, which are
 >       not really file names, thus resulted in polluting the history for file
 >       names
 >
 > I understand that the confirmation will affect the "do or create"
 > commands, right?  Perhaps we can find a way to circumvent that step.

 Yes, do-or-create commands are affected. The "[Confirm]" prompt comes
 from the project's built-in function 'project--completing-read-strict'
 as it calls 'completing-read' passing the 'confirm argument.  There is a
 way, but it might not be worth the effort.  I don't find this prompt
 intrusive, but I thought I should this point out as it is a difference
 in the eye of the user.

 > As for the history extraction, the idea was to re-use the last input in
 > those "do or create" commands.  So the user would type terms for a file
 > that does not exist and could then retrieve those terms in the next
 > prompt for the creation of a new note.  Does your change retain this
 > behaviour or would we need to make another arrangement?

 No, no need to make another arrangement. The functionality described in
 the doc string in "do-or-create" commands are retained -- as far as I
 can test it, this should be the case.  I'd appreciate it if you could
 see the video and also test this on your end.  I don't really rely on
 this facility.

 In the current implementation, I'd type titles in the file name
 completion buffer, so it appears that 'file-name-history' variable seems
 to accumulate values that are not really file names.  I believe this
 patch does away from this "round-about" way and updates
 'denote--title-history' directly without changing the functionality for
 the user.  I may have misunderstood the intent of
 'denote--push-extracted-title-to-history'.  I'd appreciate your testing
 on your end here, too, as well as the rest of the changes.

 > With those granted, I think we are on the right path.  We will just need
 > to be sure that this all works with the required Emacs version of 28.1.

 I use 28.1 myself with the following versions that come with it.  I also
 briefly tested the patch with 'emacs -q' option as shown in the video
 demo (attached).  Looks good to me so far. 

 Package project is built-in.

      Status: Built-In.
     Version: 0.8.1

 Package xref is built-in.

      Status: Built-In.
     Version: 1.3.0

 The changes are rather extensive, so I'd appreciate you and others
 testing the latest once the commit is available in the respective
 repositories.

 I'll prepare the next batch of patches :)

 Thanks!
 nobiot
Details
Message ID
<86fsee106l.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
For the brief testing I did with Emacs 28.1 with 'emacs -q', I used the
following script.  Just for transparency, and also for those who wish to
conduct some tests in a vanilla condition.


;; -*- lexical-binding: t; -*-

(require 'package)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
(add-to-list 'package-archives '("gnu-devel" . "https://elpa.gnu.org/devel/") t)

(package-initialize)
(package-refresh-contents)

(package-install 'denote)

(fido-vertical-mode 1)
(cua-mode 1)

(setq backup-directory-alist
      '((".*" . "~/.emacs.d/backup")))

(define-key minibuffer-local-completion-map (kbd "<SPC>") #'self-insert-command)
Details
Message ID
<87v8nax8vu.fsf@protesilaos.com>
In-Reply-To
<867czq2ew7.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Sun, 20 Nov 2022 09:45:12 +0100
>
> My previous reply does not seem to go through to the mailing list
> archive -- perhaps because of the video attachment.  For the benefit
> of the community, I'm re-sending just the text.  I'd appreciate it it
> if anyone has an explanation of this behaviour, and if the issue is
> indeed the video), how I can share such video here.
>
> It might well be that I should have just waited longer.

The video file gets rejected by the mailing list system here.  What you
describe in your message is sufficient though.  I guess the only way to
share such files is to host them somewhere else and post a link.  Though
don't bother with it, as the point is clear.

> [... 80 lines elided]

>  The changes are rather extensive, so I'd appreciate you and others
>  testing the latest once the commit is available in the respective
>  repositories.

I will test everything.  I am okay with the additional confirmation
step.  It seems more natural to be asked about this.

>  I'll prepare the next batch of patches :)

Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<87sfiex8l8.fsf@protesilaos.com>
In-Reply-To
<86fsee106l.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Sun, 20 Nov 2022 09:48:18 +0100
>
> For the brief testing I did with Emacs 28.1 with 'emacs -q', I used the
> following script.  Just for transparency, and also for those who wish to
> conduct some tests in a vanilla condition.
>
>
> ;; -*- lexical-binding: t; -*-
>
> (require 'package)
> (add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
> (add-to-list 'package-archives '("gnu-devel" . "https://elpa.gnu.org/devel/") t)
>
> (package-initialize)
> (package-refresh-contents)
>
> (package-install 'denote)
>
> (fido-vertical-mode 1)
> (cua-mode 1)
>
> (setq backup-directory-alist
>       '((".*" . "~/.emacs.d/backup")))
>
> (define-key minibuffer-local-completion-map (kbd "<SPC>") #'self-insert-command)

What I normally do in such cases is to find the file in Dired either in
the "elpa" directory or in some git clone and type '0 w', which is the
same as: C-u 0 M-x dired-copy-filename-as-kill

This gives me the full path to the file.  Then I start 'emacs -Q'
(notice the capital Q) and evaluate:

(require 'denote "/home/prot/.emacs.d/elpa/denote-1.1.0.0.20221116.165007/denote.el")

This basically skips the step of loading package.el and also works with
files that may not be available on the package archives (e.g. a local
copy that I just patched).

* * *

About your patches, should I wait for the complete set or can I go ahead
and install the one you have already sent?

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<86tu2ugd8e.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Thank you for the technique; very useful.

Just quickly on this question:

> About your patches, should I wait for the complete set or can I go ahead
> and install the one you have already sent?

I suggest you go ahread with the one you have with you.  The rest is
incremental change to it.  It might take some time for me to come back
to it today/tomorrow.

Thank you!
nobiot
Details
Message ID
<87pmdix7o6.fsf@protesilaos.com>
In-Reply-To
<86tu2ugd8e.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Sun, 20 Nov 2022 10:57:21 +0100
>
> Thank you for the technique; very useful.

You are welcome!

> Just quickly on this question:
>
>> About your patches, should I wait for the complete set or can I go ahead
>> and install the one you have already sent?
>
> I suggest you go ahread with the one you have with you.  The rest is
> incremental change to it.  It might take some time for me to come back
> to it today/tomorrow.

I just pushed it to the branch 'xref-refinement-project-integration'.  I
will do the same with the other patches and then merge them into 'main'.
This makes it easier for me to have everything in one place in order to
make the relevant changes to the manual.

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<865yf9ip12.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
> I just pushed it to the branch 'xref-refinement-project-integration'.
> I will do the same with the other patches and then merge them into
> 'main'.  This makes it easier for me to have everything in one place
> in order to make the relevant changes to the manual.

Yes, this makes sense to me, too.  Here are the remainder of the
patches: 3 patches attached.  I believe the commit messages should be
sufficient as documentation for each.  Let me know if anything is
unclear.

Thank you.
nobiot
Details
Message ID
<87wn7phta8.fsf@protesilaos.com>
In-Reply-To
<865yf9ip12.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
Good day nobiot,

> From: Noboru Ota <me@nobiot.com>
> Date: Sun, 20 Nov 2022 17:11:53 +0100
>
>> I just pushed it to the branch 'xref-refinement-project-integration'.
>> I will do the same with the other patches and then merge them into
>> 'main'.  This makes it easier for me to have everything in one place
>> in order to make the relevant changes to the manual.
>
> Yes, this makes sense to me, too.  Here are the remainder of the
> patches: 3 patches attached.  I believe the commit messages should be
> sufficient as documentation for each.  Let me know if anything is
> unclear.

Thank you!  I just installed the patches and pushed them to the branch.

Before I proceed with the merge into 'main', what do you about putting
this functionality into a separate file, such as denote-project.el?
This gives the user an opt-in option, while it also allows us to add all
those hooks you mention in the commits.  Basically, we will set up a
"just works" environment if the user runs a 'require' for this new file.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<861qpwpy6n.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Hi Prot,

> Before I proceed with the merge into 'main', what do you about putting
> this functionality into a separate file, such as denote-project.el?
> This gives the user an opt-in option, while it also allows us to add
> all those hooks you mention in the commits.  Basically, we will set up
> a "just works" environment if the user runs a 'require' for this new
> file.

I was kind of expecting this :)  And it makes sense.  Just to clarify,
though, as I have sent multiple patches (in order new to old):

- 0a94f3e3 feat: xref-backend facility to enable xref integration ::

  I believe you refer to only this patch.  It can be standalone and be a
  separate file.  In this case, I suppose denote-xref.el might be more
  appropriate (?)


- 93971de0 fix: denote-get-path-by-id to send only denote notes file ::

  This is a fix so I suggest it to be part of denote.el.
  

- 3e074afb feat: follow-symlinks ::

  My patch "hard code" a symbol :follow-symlinks to be as an
  argment. Alternatively, it can be a customizing variable. It should
  probably be part of denote.el.


- 6bb6da08 feat: project.el integration & backlinks refactoring with
  xref ::

  This patch includes refactoring/removing of functions within
  denote.el. The new denote-specific project facilities(project-root,
  project-files, denote-project-find) are used in core functions, so
  they cannot be separate out, unfortunately:
  
  + denote-file-prompt has been refactored to use
    'project-read-file-name-function' to look for sub-directories

  + denote-mode major mode for backend needs to set the hook
    buffer-locally to the backend link buffer.  This is needed to enable
    revert-buffer to work correctly

       (add-hook 'project-find-function #'denote-project-find nil t)
Details
Message ID
<87a64j4pxn.fsf@protesilaos.com>
In-Reply-To
<861qpwpy6n.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Mon, 21 Nov 2022 08:24:32 +0100
>
> Hi Prot,

Hello nobiot,

>> Before I proceed with the merge into 'main', what do you about putting
>> this functionality into a separate file, such as denote-project.el?
>> This gives the user an opt-in option, while it also allows us to add
>> all those hooks you mention in the commits.  Basically, we will set up
>> a "just works" environment if the user runs a 'require' for this new
>> file.
>
> I was kind of expecting this :)  And it makes sense.

I wanted to have your thoughts on this.  Though the functionality does
fit naturally with what we were doing before.  I have merged the branch
into 'main'.

> Just to clarify, though, as I have sent multiple patches (in order new
> to old):
>
> - 0a94f3e3 feat: xref-backend facility to enable xref integration ::
>
>   I believe you refer to only this patch.  It can be standalone and be a
>   separate file.  In this case, I suppose denote-xref.el might be more
>   appropriate (?)

Indeed.  I was thinking specifically about how best to handle the
xref-backend-functions and the xref-prompt-for-identifier.  The user now
needs to opt-in, which is a bit like a 'require' call.  Having a
separate file makes this more discoverable.  In a separate file, we
would assume the 'require' to mean "yes, I want all this", so we would
set everything up ourselves.

But having a separate file is not my goal: I want to make it easy for
the user to benefit from this feature.  Unlike what we did with the Org
dynamic blocks, which have their own file, the xref extension works for
all file types, so it fits into denote.el.

Perhaps we can have a minor mode?  It has the upside of being easier to
find via M-x and the self-documentation functions.

> - 93971de0 fix: denote-get-path-by-id to send only denote notes file ::
>
>   This is a fix so I suggest it to be part of denote.el.

Okay.

> - 3e074afb feat: follow-symlinks ::
>
>   My patch "hard code" a symbol :follow-symlinks to be as an
>   argment. Alternatively, it can be a customizing variable. It should
>   probably be part of denote.el.

Yes, let's keep it in denote.el.  I am fine with a user option, though I
say we keep it as-is for now to get feedback from users.

> - 6bb6da08 feat: project.el integration & backlinks refactoring with
>   xref ::
>
>   This patch includes refactoring/removing of functions within
>   denote.el. The new denote-specific project facilities(project-root,
>   project-files, denote-project-find) are used in core functions, so
>   they cannot be separate out, unfortunately:
>   
>   + denote-file-prompt has been refactored to use
>     'project-read-file-name-function' to look for sub-directories
>
>   + denote-mode major mode for backend needs to set the hook
>     buffer-locally to the backend link buffer.  This is needed to enable
>     revert-buffer to work correctly
>
>        (add-hook 'project-find-function #'denote-project-find nil t)

That's fine and thank you for taking the time to explain all this!

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<864jurbhfn.fsf@nobiot.com>
In-Reply-To
<878rk7exwo.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Goodd morning Prot.

I will come back to the points about minor-mode, etc. but very quickly,
let me send you a one-line patch.  It's rather embarrassing to get that
mistake in the code I provided ;)
Details
Message ID
<878rk3jw3v.fsf@protesilaos.com>
In-Reply-To
<864jurbhfn.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Tue, 22 Nov 2022 08:02:20 +0100
>
> Goodd morning Prot.

Hello nobiot,

> I will come back to the points about minor-mode, etc. but very
> quickly,

Sure, whenever you can.  Assume ths is tagged as "nice to have".

> let me send you a one-line patch.  It's rather embarrassing to get that
> mistake in the code I provided ;)

I noticed the compiler's complaints earlier.  Thank you for the quick
fix!

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<86ilj72zxy.fsf@nobiot.com>
In-Reply-To
<878rk3jw3v.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Protesilaos Stavrou <info@protesilaos.com> writes:


> I noticed the compiler's complaints earlier.  Thank you for the quick
> fix!

Thank you!  What do you use to get this complier's complaint?  On my
end, flymake didn't say anything and the version with the error
compilied (from .el to .elc in Dired) with no error message...

nobiot
Details
Message ID
<8735aaqpm1.fsf@protesilaos.com>
In-Reply-To
<86ilj72zxy.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Tue, 22 Nov 2022 08:47:21 +0100
>
> Protesilaos Stavrou <info@protesilaos.com> writes:
>
>
>> I noticed the compiler's complaints earlier.  Thank you for the quick
>> fix!
>
> Thank you!  What do you use to get this complier's complaint?  On my
> end, flymake didn't say anything and the version with the error
> compilied (from .el to .elc in Dired) with no error message...

I use this:

    (require 'package-lint-flymake)
    (add-hook 'flymake-diagnostic-functions #'package-lint-flymake)

Then I simply review the issues with M-x flymake-show-buffer-diagnostics

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<868rk1wpan.fsf@nobiot.com>
In-Reply-To
<8735aaqpm1.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Protesilaos Stavrou <info@protesilaos.com> writes:

> I use this:
>
>     (require 'package-lint-flymake)
>     (add-hook 'flymake-diagnostic-functions #'package-lint-flymake)

Thank you, Prot.  I didn't know about 'package-lint' and
'package-lint-flymake'.  I now see the errors you mentioned :) This is
very useful.

> Indeed.  I was thinking specifically about how best to handle the
> xref-backend-functions and the xref-prompt-for-identifier.  The user now
> needs to opt-in, which is a bit like a 'require' call.  Having a
> separate file makes this more discoverable.  In a separate file, we
> would assume the 'require' to mean "yes, I want all this", so we would
> set everything up ourselves.
>
> But having a separate file is not my goal: I want to make it easy for
> the user to benefit from this feature.  Unlike what we did with the Org
> dynamic blocks, which have their own file, the xref extension works for
> all file types, so it fits into denote.el.
>
> Perhaps we can have a minor mode?  It has the upside of being easier to
> find via M-x and the self-documentation functions.

We have these two (I set them like this in my init):

    (add-hook 'project-find-functions #'denote-project-find)
    (add-hook 'xref-backend-functions #'denote--xref-backend)

They are independent of each other so we do not need both of them at the
same time but I find both useful.

A minor mode is a good idea (either global and/or local).  An
alternative might be something like Org's 'org-modules' via customizing.
It may not have the flexibility of local/global that minor mode would
provide, but it provides an easy-to-use checkbox interface via
'customize' (see my sample implementation below; lightly tested).

For those who do not use 'customize' like you, I believe 'use-package'
with the ':custom' directive should provide the same facility according
to the README of 'use-package'[1] -- Emacs should automatically call the
set function (#'denote-set-modules) in our case below.  In your very
personalized case, I believe you can directly call
'custom-set-variables' (?)  to take advantage of the set function.  Or
directly set the hooks -- a truly advanced usage.

By the way, potentially, requiring 'denote-org-dblock.el' can also be a
"module" in this way (you'd need to add a 'const' value for it and code
an additional 'cond' clause -- probably using
'unload-feature'/'load-library', rather than 'require' so that you can
unload).

Just an idea with a sample implementation. 


[1]: the first NOTE in README available at
https://github.com/jwiegley/use-package#package-customization


Thank you.
nobiot

---

(defun denote-set-modules (var value)
  "...doc string to be written."
  (set var value)
  (remove-hook 'project-find-functions #'denote-project-find)
  (remove-hook 'xref-backend-functions #'denote--xref-backend)
  (dolist (v value)
    (cond ((eq v 'project)
           (add-hook 'project-find-functions #'denote-project-find))
          ((eq v 'xref)
           (add-hook 'xref-backend-functions #'denote--xref-backend))
          (t (ignore)))))

(defcustom denote-modules '(project xref)
  "...doc string to be written."
  :group 'denote
  :package-version '(denote . "1.2.0")
  :set 'denote-set-modules
  :type
  '(set	(const :tag "   project:         Package integration" project)
	(const :tag "   xref:            Xref integration" xref)))
Details
Message ID
<87ilj4ibvs.fsf@protesilaos.com>
In-Reply-To
<868rk1wpan.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
Hello nobiot,

> From: Noboru Ota <me@nobiot.com>
> Date: Wed, 23 Nov 2022 18:31:28 +0100
>
> Protesilaos Stavrou <info@protesilaos.com> writes:
>
>> I use this:
>>
>>     (require 'package-lint-flymake)
>>     (add-hook 'flymake-diagnostic-functions #'package-lint-flymake)
>
> Thank you, Prot.  I didn't know about 'package-lint' and
> 'package-lint-flymake'.  I now see the errors you mentioned :) This is
> very useful.

You are welcome!

>> Indeed.  I was thinking specifically about how best to handle the
>> xref-backend-functions and the xref-prompt-for-identifier.  The user now
>> needs to opt-in, which is a bit like a 'require' call.  Having a
>> separate file makes this more discoverable.  In a separate file, we
>> would assume the 'require' to mean "yes, I want all this", so we would
>> set everything up ourselves.
>>
>> But having a separate file is not my goal: I want to make it easy for
>> the user to benefit from this feature.  Unlike what we did with the Org
>> dynamic blocks, which have their own file, the xref extension works for
>> all file types, so it fits into denote.el.
>>
>> Perhaps we can have a minor mode?  It has the upside of being easier to
>> find via M-x and the self-documentation functions.
>
> We have these two (I set them like this in my init):
>
>     (add-hook 'project-find-functions #'denote-project-find)
>     (add-hook 'xref-backend-functions #'denote--xref-backend)
>
> They are independent of each other so we do not need both of them at the
> same time but I find both useful.

Yes, they are independent though they both enrich the experience of
finding a note without worrying about the details of the file system and
any possible directory structure.

As an aside, in the predecossor of Denote ("USLS" aka the "Unassuming
Sidenotes of Little Significance" or "useless") links were just the
identifier without any surrounding special characters.  For example:

    In 20221121T143640 I wrote that ...

To visit a link you would place the point over it and type 'C-x C-f
M-n'.  Actually, this works with Denote as well, assuming a flat
directory.  It also works with Embark, where you invoke 'embark-act' on
the identifier and then type RET for 'find-file'.

> A minor mode is a good idea (either global and/or local).  An
> alternative might be something like Org's 'org-modules' via customizing.
> It may not have the flexibility of local/global that minor mode would
> provide, but it provides an easy-to-use checkbox interface via
> 'customize' (see my sample implementation below; lightly tested).

Interesting idea!  I never checked how those work internally.

> For those who do not use 'customize' like you, I believe 'use-package'
> with the ':custom' directive should provide the same facility according
> to the README of 'use-package'[1] -- Emacs should automatically call the
> set function (#'denote-set-modules) in our case below.  In your very
> personalized case, I believe you can directly call
> 'custom-set-variables' (?)  to take advantage of the set function.  Or
> directly set the hooks -- a truly advanced usage.

Just for your information, Emacs 29 provides the 'setopt' macro which is
meant to look like 'setq' but work with those Custom features.

I have some experience with Custom setters, having used them in 'pulsar'
and 'lin' (among others).  The former no longer has them, while the
latter uses them while also providing a minor mode (Lin's custom setter
basically does the same thing as the minor mode when it is enabled).  In
short, I think they are fine though not as flexible as local/global
minor modes.  If we can provide both, that is even better.

Your sample code is like what I am doing in 'lin':
<https://git.sr.ht/~protesilaos/lin>.  If you want to take a look, check
the definition of 'lin-mode-hooks' and then that of the
'lin-global-mode'.

> By the way, potentially, requiring 'denote-org-dblock.el' can also be a
> "module" in this way (you'd need to add a 'const' value for it and code
> an additional 'cond' clause -- probably using
> 'unload-feature'/'load-library', rather than 'require' so that you can
> unload).
>
> Just an idea with a sample implementation. 

In principle, I am fine with this.  The intent is to make the features
discoverable for the benefit of users.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<86v8n0qp8d.fsf@nobiot.com>
In-Reply-To
<87ilj4ibvs.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Protesilaos Stavrou <info@protesilaos.com> writes:

Hi Prot,

> As an aside, in the predecossor of Denote ("USLS" aka the "Unassuming
> Sidenotes of Little Significance" or "useless") links were just the
> identifier without any surrounding special characters.  For example:
>
>     In 20221121T143640 I wrote that ...
>
> To visit a link you would place the point over it and type 'C-x C-f
> M-n'.  Actually, this works with Denote as well, assuming a flat
> directory.  It also works with Embark, where you invoke 'embark-act' on
> the identifier and then type RET for 'find-file'.

Thank you for this!  I didn't know about this behaviour and I dug a
little deeper.  Now I know what "future history" is in the context of
Emacs completion and this for 'find-file' is achieved by the 'ffap'
package.  I knew nothing of 'ffap' but it's great hidden treasure for me
:)

> The intent is to make the features
> discoverable for the benefit of users.

I have looked at your 'lin' package, and how it handles issues similar
to 'org-modules'. As a result...

I have implemented a new module concept together with a global/minor
mode.  Briefly, they work this way:

1. Set 'denote-modules' by selecting the modules you wish to use
2. Activate the minor mode globally or locally
3. This then disables the modules previously enabled, and enable the
   ones newly set -- if it is the first time after Emacs launch, it will
   just enable the modules.

I have added the integration with project and xref as modules and added
a new one with ffap (find-file-at-point).

It's made easier for Customizing UI.  If you wish to use 'setq' (and
probably 'setopt has the same syntax?), it would be a list of cons cells
like this:

    (setq denote-modules
          '((project-find-functions . denote-project-find)
            (xref-backend-functions . denote--xref-backend)
            (denote-module-ffap-enable . denote-module-ffap-disable)))

With the ffap module, you can put your point over an identifier, and then
press 'C-x C-f M-n'.  The future history now should also display files
in sub-directories (of any depth).  It also allows us to use `ffap-menu'
(among other useful commands) to show a list of all the links of a note
buffer (URL and denote notes via the identifiers)in a similar view with
IMenu -- though the source code of ffap-menu says that it no longer uses
IMenu.  This one is very nice too.

I'm attaching the patches.

Thank you.
nobiot
Details
Message ID
<865yey6ev2.fsf@nobiot.com>
In-Reply-To
<87ilj4ibvs.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Protesilaos Stavrou <info@protesilaos.com> writes:

Sorry that I unintentionally created another thread.  I'm hoping this
email finds itself in the original thread.

>> Just very quickly: I will be sending a patch soon to make the user
>> option of 'denote-modules' easy for users -- a list of symbols,
>> rather than the compound type in the current patch.

> Yes, this is better: we can handle all the implementation details
> internally.

Here is the patch :)


> Thank you for working on this!

You are very welcome.


> By the way, I think this is relevant:
> <https://github.com/protesilaos/denote/issues/114>.

I took the liberty of responding directly in the issue.


> We may want to consider the default values so that we give users a
> good experience out-of-the-box.

My assessment is that the modules are not relevant for this particular
issue, but I agree that we can/should discuss what default value should
be appropriate.  Currently 'denote-modules' is nil as default to respect
the current behaviour.

Cheers, nobiot
Details
Message ID
<87y1rtufwf.fsf@protesilaos.com>
In-Reply-To
<865yey6ev2.fsf@nobiot.com> (view parent)
DKIM signature
pass
Download raw message
> From: Noboru Ota <me@nobiot.com>
> Date: Mon, 28 Nov 2022 20:47:13 +0100

Good day nobiot,

> Protesilaos Stavrou <info@protesilaos.com> writes:
>
> Sorry that I unintentionally created another thread.  I'm hoping this
> email finds itself in the original thread.

It does on my email client.  Though the web archive is not as reliable
in this regard.  Sometimes a direct reply becomes its own thread.

>>> Just very quickly: I will be sending a patch soon to make the user
>>> option of 'denote-modules' easy for users -- a list of symbols,
>>> rather than the compound type in the current patch.
>
>> Yes, this is better: we can handle all the implementation details
>> internally.
>
> Here is the patch :)

Thank you!  I will collect them all now and install them.

>> Thank you for working on this!
>
> You are very welcome.
>
>> By the way, I think this is relevant:
>> <https://github.com/protesilaos/denote/issues/114>.
>
> I took the liberty of responding directly in the issue.

You did well.  Thank you!

>> We may want to consider the default values so that we give users a
>> good experience out-of-the-box.
>
> My assessment is that the modules are not relevant for this particular
> issue, but I agree that we can/should discuss what default value should
> be appropriate.  Currently 'denote-modules' is nil as default to respect
> the current behaviour.

Indeed, it is not pertinent to this topic.  I just mentioned it as a
point that we want to keep things simple for users of all skill levels
and have a sense for helpful defaults.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com

[PATCH] feat: thing-at-point 'denote-id

Details
Message ID
<86pmcr144u.fsf@nobiot.com>
In-Reply-To
<878rk3jw3v.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Hi Prot

This patch is motivated by our recent GitHub discussion on an issue #115
,specially this comment by Prot:
https://github.com/protesilaos/denote/issues/115#issuecomment-1345153297

    "The obvious use-case is to add hyphens and colons to the date and
    time components of the identifier. Though we can build on that".

I am proposing to add a denote-specific 'thing-at-point' extension to
identify denote-id with using 'denote-id-regexp'.  This lets 'denote-id'
at point can be identified and retrieved even when 'denote-id-regepxp' is
customized.

The 'thing-at-point' can be used across other denote moduels and
packages to identify denote-id at point.  As an example, it is already
useful for the denote xref module.  I customize my denote-id to this
format: yyyy-mm-ddThhmmss.  This change will let me retire my method
override for 'xref-backend-identifier-at-point'.

Warm regards,
nobiot
Reply to thread Export thread (mbox)