nobiot / org-transclusion

Emacs package to enable transclusion with Org Mode
https://nobiot.github.io/org-transclusion/
GNU General Public License v3.0
902 stars 43 forks source link

org-transclusion-before-kill is unable to clean transclusion content from copy on disk. #244

Closed akashpal-21 closed 2 months ago

akashpal-21 commented 2 months ago

If user saves buffer while live-sync is in progress - the actual content saved to disk will not be cleaned of transclusion content. Combination of buffer-modified states prevent user from seeing what content is actually on disk and what is being presented in buffer.

One solution that I have implemented -- a slight modification to the org-transclusion-kill-buffer-hook

(defun org-transclusion-before-kill ()
  (when (and (org-transclusion-remove-all)
             (buffer-file-name))
    (set-buffer-modified-p t)
    (org-transclusion-remove-all)
    (save-buffer)
    ))

original implementation

 (defun org-transclusion-before-kill ()
  (when (and (org-transclusion-remove-all)
             (buffer-file-name))
    (org-transclusion-remove-all)))

Documentation removed for brevity.

Thanks, Akash.

nobiot commented 2 months ago

If user saves buffer while live-sync is in progress - the actual content saved to disk will not be cleaned of transclusion content

The manual has this note:

Note: During live-sync edit, file’s content gets saved to the file system as is – i.e. the transcluded text will be saved instead of the ‘#+transclude:’ keyword. If you kill buffer or quit Emacs, other hooks will still remove the transclusion to keep the file clear of the transcluded copy, leaving only the keyword in the file system.

Is this issue saying that the hooks do not remove the transclusion as described in the manual?

I am a little confused because the title of this issue is about save during live-sync and yet your suggested solution is about kill-buffer. Could you please elaborate a little more?

akashpal-21 commented 2 months ago

Yes - the kill-buffer-hook doesn't work as intended. The kill-buffer-hook should remove the transclusions even if during live-sync the file is saved as is -- since the save-buffer hooks are disabled inbetween.

I have also found that the kill-emacs-hooks dont function in my client- daemon setup -- in fact no buffer local kill-emacs hook function in the client-daemon setup -- but we shouldnt make any change there - a user side improvement is needed there too.

nobiot commented 2 months ago

I have also found that the kill-emacs-hooks dont function in my client- daemon setup -- in fact no buffer local kill-emacs hook function in the client-daemon setup

server-done-hook might work: https://stackoverflow.com/questions/3737207/emacsclient-hook-on-kill/3738178#3738178

nobiot commented 2 months ago

Instead of pressing the "X" button on your window manager, if I use server-edit (C-x #) or save-buffers-kill-terminal (C-x C-c) on emacsclient, transclusions correctly seem to get removed correctly. Is this not the case on your end?

nobiot commented 2 months ago

In addition... Is this issue only about Emacs running as a server and daemon? The title seems to suggest it is a generic issue and is a problem ALSO outside server and daemon. Are we talking about two different issues (one is about live-sync and the other, about server and daemon)? If so, let's have a separate issues...

If you are talking about live-sync being a generic issue, I cannot reproduce it.

akashpal-21 commented 2 months ago

I am sorry - I got lost midway, what I want to say is that there is one function org-transclusion-before-kill and this function is supposed to run at two occasions to cover the case that an user has saved the buffer while midway a live sync.

If nothing else is done now - when the time comes for the function to run - it would first detect if there are transclusions then before getting rid of the buffer clear the transclusions -

But the problem is this function then doesn't write the transclusion-free buffer anywhere.

So if buffer is saved midway live sync - the content on disk is not free of the transclusion content -- the function simply clears the buffer and sends it to the void -- the copy on disk is not cleaned at all.

The second part of the problem I was discussing was the running of this function itself and how the kill-emacs-hook has some quirks when set as buffer local -- I will move to a different issue with that as you suggested -- but it is related to this overarching problem itself -- to clean the transclusion content of a file which has slipped past our nose.

akashpal-21 commented 2 months ago

The documentation for the function states

 "Remove transclusions before `kill-buffer' or `kill-emacs'.
Intended to be used with `kill-buffer-hook' and `kill-emacs-hook'
to clear the file of the transcluded text regions.  This function
also flags the buffer modified and `save-buffer'.  Calling the
second `org-transclusion-remove-all' ensures the clearing process
to occur.  This is required because during live-sync, some hooks
that manage the clearing process are temporarily turned
off (removed)."
  ;; Remove transclusions first. To deal with an edge case where transclusions
  ;; were added for a capture buffer -- e.g. `org-capture' or `org-roam-catpure'
  ;; --, check is done for `buffer-file-name' to see if there is a file visited
  ;; by the buffer. If a "temp" buffer, there is no file being visited.
  (when (and (org-transclusion-remove-all)
             (buffer-file-name))
    (org-transclusion-remove-all)))

Although the documentation states that it changes the buffer-modified state and saves it - the actual function doesn't do this in my end.. the writing to the disk part is missing.

akashpal-21 commented 2 months ago

If you are talking about live-sync being a generic issue, I cannot reproduce it.

I saw this just now - I don't understand sorry - the transclusion-before-kill is not writing to disk here -- how is the cleaned up buffer being pushed to disk - org-transclusion-remove-all doesn't do any writing to disk...

How is it working for you but not for me? Can be elaborate on the schematic a little and correct me where I am wrong please.

nobiot commented 2 months ago

I am still trying to understand the issue you report here.

Let us take a step back.

The title of this issue is this:

Saving in between live-sync prevents transclusion cleanup on kill-buffer and friends

The documentation says this:

Note: During live-sync edit, file’s content gets saved to the file system as is

So... the behavior you are reporting as an issue seems to be the way live-sync currently works.

And then... I am sorry that I am lost here. I cannot tell if

  1. You observe this same behavior on your end. You do not like it so are suggesting a fix

  2. This documented behavior does not occur on your end.

Which case is it? Or either?

Sorry my lack of understanding of this issue is as basic as this.

nobiot commented 2 months ago

Please give me a basic step-by-step instruction how to reproduce the problem.

akashpal-21 commented 2 months ago

If you kill buffer or quit Emacs, other hooks will still remove the transclusion to keep the file clear of the transcluded copy, leaving only the keyword in the file system.

What I want to pull your attention is to this feature that org-transclusion-before-kill is supposed to fulfill but cannot because it is missing the very important step of writing to disk.

Create a source.org and target.org Give basic structure to source.org

  * h1
    Some text

Go to target.org Insert: #+transclude: [[file:source.org]] eval: (org-transclusion-add)

eval: (org-transclusion-live-sync-start) eval: (save-buffer)

Result on disk ;; target.org

* h1
   Some text

As expected so far

Kill buffer or kill-emacs

Expected Result on disk ;; target.org

#+transclude: [[file:source.org]]

Actual Result on disk ;; target.org

* h1
   Some text

Why?

Because org-transclusion-before-kill did not do its job properly.

akashpal-21 commented 2 months ago

-- Close issue --

Suggestion introduces regression -- user choice to discard changes made is not kept -- discarded changes will be forced to disk by the function if it writes to disk

If you kill buffer or quit Emacs, other hooks will still remove the transclusion to keep the file clear of the transcluded copy, leaving only the keyword in the file system.

Should be removed from the manual as no provisions are in place to do this. org-transclusion-before-kill nevertheless should be run to clear overlays from source buffers

akashpal-21 commented 2 months ago

My personal fix which works in all conditions -- uses the vundo library to process the case that user wants to discard change and yet transclusions from disk need to be cleaned. Good enough for me to ensure that transclusions are replaced with their keyword if I mistakenly save during live sync.

(defun patch/org-transclusion-before-kill (fn &rest args)
  (cond
   ;; Case 1 : User has chosen to discard changes since last save
   ;; Revert to last save position -- clean transclusions -- push to disk
   ((and (buffer-modified-p) (functionp 'vundo))
    (let ((vundo-buf (vundo-1 (current-buffer))))
      (with-current-buffer vundo-buf
        (vundo-goto-last-saved 1)
    (ignore-errors
          (vundo-confirm))))
    (apply fn args)
    (write-file (buffer-file-name)))
   ;; Case 2 : Same as Case 1 -- but Vundo is not installed
   ;; Do not process file on disk
   ((buffer-modified-p)
    (apply fn args))
   ;; Case 3 : Buffer is unmodified - clean transclusions from disk if exists any
   ;; esp those that might have crept if the last save was during a live sync
   (t
    (apply fn args)
    (write-file (buffer-file-name)))))

  (advice-add 'org-transclusion-before-kill :around #'patch/org-transclusion-before-kill)