Saving a capture buffer gives wrong behaviour, because the wrong node is found

[andersjohansson]

Hi, after my report in #10 I’ve still not gotten org-roam-timestamp to always do the right thing when capturing.
As reported there, I often save the buffer before finalizing the capture (C-c C-c) and this is about what happens when invoking save-buffer in an org-roam capture buffer (although I don’t think I really get the right behaviour when doing org-capture-finalize either, will continue debugging that).

It seemed that org-roam-timestamps--on-save had trouble getting the right node. After doing some debugging with edebug (and lots of (message "pN %d" (point))) I realized that the problem is that when you do a save in an indirect buffer (which a capture buffer is) the buffer is switched to its base buffer (this happens in basic-save-buffer) and then the before-save-hooks (including org-roam-timestamps--on-save) but now (point) is wrong! This is because we are in the base buffer, which has a separate point, usually at some other node, so org-roam-timestamps--on-save gets it all wrong when invoking (org-roam-node-at-point) here.

The good thing is that I found what the problem was. The bad thing is that I don’t really have a good solution for this.

[andersjohansson]

Oh, I forgot to mention, this is in a file with multiple nodes (a datetree) this won‘t be a problem if the file only contains a single node (one id for the file).

[stites]

I can reproduce this even with top-level nodes. The funny thing is that org-roam-timestamps--on-save still adds the mtime and ctime properties, it just also throws this error at me.

[stites]

I think I fixed it with stites@bd995ff (both referenced commits are the same, I just amended the message and force-pushed) I am going to take it for a test drive before I submit a PR.

[andersjohansson]

Hi @stites, unfortunately, I don’t think your changes solves the problem at all for my case of saving while capturing to a new node in an already existing tree in a larger file with notes. The problem there is that the wrong node (the one point is on in the "direct" buffer, not the capture buffer) can be fetched already on the second line of org-roam-timestamps--on-save, and from there it all goes downhill 😄.

So, in the subsequent calls to org-roam-timestamps--get-mtime and org-roam-timestamps--add-mtime we do have a node, but it is the wrong one.

Additionally, when using my current multiple nodes per file-workflow (perfectly reasonable in org-roam from v. 2), I do think that your assumption that adding properties at point-min (so we get properties for the full file) is the what the user would expect.

[stites]

I used to observe your issue and will try to reproduce it again later tonight. Perhaps I have been ignoring it through selection bias, but I don't recall seeing this since my patch.

…

------- Original Message -------

On Wednesday, February 9th, 2022 at 11:37 AM, Anders Johansson ***@***.***> wrote: Hi ***@***.***(https://github.com/stites), unfortunately, I don’t think your changes solves the problem at all for my case of saving while capturing to a new node in an already existing tree in a larger file with notes. The problem there is that the wrong node (the one point is on in the "direct" buffer, not the capture buffer) can be fetched already on the second line of org-roam-timestamps--on-save, and from there it all goes downhill 😄. So, in the subsequent calls to org-roam-timestamps--get-mtime and org-roam-timestamps--add-mtime we do have a node, but it is the wrong one. Additionally, when using my current multiple nodes per file-workflow (perfectly reasonable in org-roam from v. 2), I do think that your assumption that adding properties at point-min (so we get properties for the full file) is the what the user would expect. — Reply to this email directly, [view it on GitHub](#12 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAM5X4PPSZQNMP4P6OFMFOTU2KJ35ANCNFSM5MIGN3NQ). Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub). You are receiving this because you were mentioned.Message ID: ***@***.***>

[stites]

Huh, funny -- I only observe this behavior "sometimes."

I think my PR #14 fixes #10 which should have been reopened. This issue, I believe, is also mistitled because it does not depend on a capture buffer. My PR does fix some aspects you mentioned (saving the indirect buffer yields a nil point), but the assumptions are not exactly correct as you point out.

I'm not an elisp developer and, unfortunately, I can live with the current functionality. That said, if we can come up with something reproducible, I could carve out some time to fix it. At the moment this looks like a Heisenbug for me. Here is what I am doing, in the form of an org-mode file:

:PROPERTIES:
:ID:       965fb288-6030-4cb1-9f32-ad13df8ce72f
:mtime:    20220209225649
:ctime:    20220209225649
:END:
#+title: first I create a tester org-rom file, I let this get saved and ctime and mtime are created.

* I produce a non-roam node
(1) I type things here. save. No additional properties seem to be added here. Let's see if
I can capture this by producing a new node in roam... (skip to the next header before returning to (2)).

(2) saving again. ctime and mtime??? okay... weird. Let me delete this and save
again... nothing.

I'll delete file-level ctime and mtime to retrigger the library. save.
save again.

OKAY! I think I found it. nope. I'll explain the rest in the issue.

** let's make a node with some text.
:PROPERTIES:
:ID:       62150661-672f-498b-93d4-a126213ff251
:mtime:    20220209225541
:ctime:    20220209225541
:END:
Add an id now. save.
saving creates the mtime and ctime as expected. I'll go back to the parent
header now in (2)...

I was able to produce a ctime and mtime /sometimes/ when I saved at the bottom of the parent header's text. I thought that it might be point related and tried saving when my cursor was in certain locations that might have been misconstrud as "on the child header" but actually I was in the parent header. I also deleted ctime/mtime to attempt to trigger some functions.

So, unfortunately, this still looks like a heisenbug to me. I still suspect that part of this bug is that the state of the cursor is being interpreted as "on a valid org-roam node" but there is some caching going on that suggests that the point/mark is on a parent/child header.

[andersjohansson]

I believe we may be talking about quite different situations that cause problems. For the case that I have tried to outline above, the problem comes with capture buffers, which are "indirect buffers" (a separate view into the same buffer) with a separate position for point. Saving a capture buffer will execute the save action and hooks in the normal (base) buffer, where point may be at an entirely different place (which is a big problem if there are multiple nodes in the file, and less of a problem if each node is in a separate file). So org-roam-timestamps will fetch and add times to whatever node is at point in the base buffer.

As I mentioned earlier, this is hard to workaround in a good way if we want to still use before-save-hook since this is how saving indirect buffers work (see the first few lines in basic-save-buffer). I may have to adapt my workflow a bit instead.