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

Push modifications to buffer-undo-list, even if unwinding #1149

Merged
merged 1 commit into from
Oct 13, 2024

Conversation

pestctrl
Copy link
Contributor

I've written a couple of yasnippets for ledger, that involve user input through org-read-date. Something like this:

# -*- mode: snippet -*-
# name: Credit Card
# key: cred                                                                       
# --
`(format-time-string "%Y/%m/%d" (org-read-date nil t))` ${0:Charge to Credit Card}
    ${2:Exp}  ${1:\$100}
    Lia:CreditCard

However, occasionally I change my mind after expanding the yasnippet, and hit C-g. This completely ruins undo-tree, it's basically unusable.

The patch I've proposed fixes this, though IDK if there's a more elegant way to fix this. I noticed that if the block of code that modifies the buffer-undo-list still executes after hitting C-g, everything is fine. So, I'm wrapping the whole block that is "inserting" the snippet with an unwind-protect, to ensure that the proper values still get pushed to buffer-undo-list.

Is this good?

@pestctrl
Copy link
Contributor Author

ping

@monnier
Copy link
Collaborator

monnier commented Oct 10, 2024

Really? Ping already after waiting what... not even 2 years? Come on, give us a break!

More seriously: We should use undo-amalgamate-change-group instead, but in the mean time your change looks OK (it's going to do the wrong thing if we get an error before we narrow-to-region, but that seems much less likely than your case).

But I can't just merge your change because it has conflict with the current code. Can you fix the conflict?

@pestctrl
Copy link
Contributor Author

Branch has been rebased.

@monnier monnier merged commit fe1f4e0 into joaotavora:master Oct 13, 2024
@monnier
Copy link
Collaborator

monnier commented Oct 13, 2024

Thank you!
And sorry it took so long.

@pestctrl
Copy link
Contributor Author

Yay!

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