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

Add undo-propose-marker-list #25

Merged
merged 4 commits into from
Oct 5, 2019
Merged

Add undo-propose-marker-list #25

merged 4 commits into from
Oct 5, 2019

Conversation

jackkamm
Copy link
Owner

This PR generalizes #21 and provides a workaround for #22, to correctly update markers after undo'ing. In particular, it creates a list undo-propose-marker-list, and undo-propose will update the positions of markers in this list after undo'ing.

In addition, it adds a unit testing framework to undo-propose and adds a test for org-clock-marker as the first unit test.

undo-propose.el Outdated
undo-propose-parent)))))

(with-eval-after-load 'org
(add-to-list 'undo-propose-marker-list org-clock-marker))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an additional marker regarding org-clock. Could you add org-clock-hd-marker?

undo-propose.el Outdated
"Copy markers registered in `undo-propose-marker-list'."
(setq-local undo-propose-marker-map
(cl-loop for orig-marker in undo-propose-marker-list
if (eq (marker-buffer orig-marker) undo-propose-parent)
Copy link
Contributor

@takaxp takaxp Sep 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, if we replace orig-marker with (symbol-value orig-marker) from line 196 to 200, then user can add any markers to undo-propose-marker-list like (add-to-list 'undo-propose-marker-list 'org-clock-hd-marker).

@takaxp
Copy link
Contributor

takaxp commented Oct 3, 2019

I'm using the proposed code for this week and it works well so far. Thanks.

@jackkamm jackkamm merged commit f80baee into master Oct 5, 2019
@jackkamm
Copy link
Owner Author

jackkamm commented Oct 5, 2019

@takaxp Thanks for the helpful feedback and sorry for the delay. I've added your suggestions (most importantly, turning undo-propose-list into a list of quoted symbols) and merged it in. Thanks!

@takaxp
Copy link
Contributor

takaxp commented Oct 5, 2019

Congrats! I also thanks to you for the precise analysis on markers and the implementation of the simplest but sufficient code for this issue. Thanks!

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