Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle org-clock-marker properly #21

Closed
wants to merge 3 commits into from
Closed

Handle org-clock-marker properly #21

wants to merge 3 commits into from

Conversation

takaxp
Copy link
Contributor

@takaxp takaxp commented Sep 6, 2019

Hi.

This PR will support remaining org-clock-marker properly. The original implementation will loose the marker position of org-clock-marker when copy-to-buffer in undo-propose-commit and undo-propose-squash-commit if the original buffer has an active clock. It causes a breaking of org-clock.

So I added a new private and buffer local variable undo-propose--org-clock-marker. The workflow to keep org-clock is built as follow:

  1. In undo-propose command, check the original buffer is org and having an active clock.
  2. If not, do nothing. It it is, then prepare a local marker and copy the position.
  3. Undos probably change the position of the active clock, but the local marker can follow it.
  4. In undo-propose-commit or undo-propose-squash-commit, move the position of org-clock-marker to the right position that is recorded in undo-propose--org-clock-marker.

Best,
Takaaki

@jackkamm
Copy link
Owner

jackkamm commented Sep 6, 2019

Thanks for reporting this problem and submitting this PR. I agree we should fix this problem, but I am hesitant to add such a specific accommodation for org-mode -- I would prefer a more general solution. It seems like the underlying issue is that all markers get destroyed by copy-buffer; I think the correct behavior would be to try and preserve all the markers if possible. However, I must admit my knowledge on this part of emacs is very shallow, and I need to do some research to better understand the issue.

If specific accommodations for org-mode are absolutely required, I think it would be better to create a second file, undo-propose-integrations.el, that include workarounds for various modes, and to keep the underlying undo-propose.el simple and general.

Could you comment on the specific parts of org-clock that break? E.g., are there errors in the *Messages* buffer, does the timer go away, what sorts of behavior do you visually see in emacs that indicate the breakage?

@takaxp
Copy link
Contributor Author

takaxp commented Sep 6, 2019

Hi!

Basically, I agree with your idea that keep the core module as simple as possible. So providing an additional file to fix some issues on specific package.

Here is a demo that shows the original implementation breaks org-clock-marker. No error message will show because this is not actually an error but just missing something.

Untitled

Explanation:

  • First, I demonstrated the clock-in and clock-out of a heading. org-clock-marker is originally #<marker in nobuffer> as default, but after starting clocking with org-clock-in, it changed to #<marker at 253 in test.org>. And after that, org-clock-out then it changed to #<marker in nobuffer> again.
  • I org-clock-in again and the marker is #<marker at 253 in test.org>.
  • Added some changes on the org buffer
  • Step into undo-propose-mode and some undoes
  • then call undo-propose-commit
  • check the org-clock-marker, and that reports #<marker at 1 in test.org> (I call this breaking), it should be 254.
  • After that, org-mode cannot find the correct location of the current clock and shows Clock start time is gone

According to my analysis, the breaking is happened, I mean org-clock-marker lose the marker position, right after the copy-to-buffer in undo-propose-commit.

@jackkamm
Copy link
Owner

jackkamm commented Sep 6, 2019

I've filed an issue for the more general problem of markers: #22

I think fixing the general problem of markers more broadly will require some major changes, possibly even a redesign. It may be some time before it's solved.

So, I think it is a good idea to start a second module for various workarounds/integrations, and refactor the core module as needed to make these workarounds possible (e.g. by adding more hooks).

@takaxp
Copy link
Contributor Author

takaxp commented Sep 6, 2019

That's sound good! I can help anything on upgrading this nice package, so feel free to assign any tasks to me :) I'll also retrieve a good and generic solution for handling markers.

It's up to you to close this PR or not. Until the integration of a second module, I can use my private repository for my daily base activities in Emacs with a combination of org and undo-propose.

@jackkamm
Copy link
Owner

As we discussed in #22, I've implemented a more general mechanism for updating markers, instead of handling org-clock-marker specially. So I am closing this PR in favor of the one at #25. If you have a chance to look at it I would be interested to hear your thoughts on it.

@jackkamm jackkamm closed this Sep 21, 2019
@takaxp takaxp deleted the org-clock branch November 20, 2019 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants