ledger / ledger

Double-entry accounting system with a command-line reporting interface
https://www.ledger-cli.org
Other
5.36k stars 504 forks source link

Using undo/redo can break ledger-mode (BZ#966) #966

Closed tbm closed 8 years ago

tbm commented 11 years ago

Note: the issue was created automatically with bugzilla2github

Bugzilla bug ID: BZ#966 From: unknown user CC: @thdox

tbm commented 11 years ago

Comment author: Craig Earls (@enderw88)

I thought about that, but what happens if the undo follows immediately after an auto-indent operation? I tried to work through that by turning off undo tracking while auto aligning, but then emacs got lost.

Thanks for looking into this, if it works for you I would be happy to add it back in.

tbm commented 11 years ago

Comment author: unknown user

It's a shame to lose a neat feature, but I guess it's the only safe decision with the 3.0 release so imminent!

However, reading up on it a little more, it looks as though I might actually have been close to correct with my attempt at a patch. I'm pretty sure that the problem is purely that auto-indentation running during an undo causes the undo list to be desynchronized from the buffer state. If it is suppressed during undos, Emacs should restore the buffer to its previous state correctly, including the effect of any previous auto-indentation.

The reason it didn't work reliably is that I picked a bad way to do that suppression. Changing the condition to use (not undo-in-progress), rather than (not (eq last-command 'undo)), is looking quite promising so far.

I'll try using this version for a while and see if I run into any more problems; if it seems to be safe, I think it would be nice to restore the feature as an option at some future point.

tbm commented 11 years ago

Comment author: Craig Earls (@enderw88)

Fixing this the right way requires developing a way for emacs to understand what semantically you last changed. I have removed auto indent as it is too dangerous since it really screws with undo.

I have added C-TAB to indent the current xact.

tbm commented 11 years ago

Comment author: Craig Earls (@enderw88)

For now, turn off auto-indent (in customize ledger). When you are done with an editing session you can clean up the buffer by selecting the entire buffer (or some subset) then M-x indent-region

Auto-indent may be too aggressive, it doesn't really fit in with the emacs way. It might go away if I can't get it to play nice with undo.

tbm commented 11 years ago

Comment author: unknown user

Actually, scratch that. It fixes the specific test case I was using, but Emacs can still get confused over what to undo in ways that can corrupt data.

For example, given a posting

2013/05/01 foo Expenses:Foo $10.00 Assets:Bar

If I delete the zero after the decimal, and then immediately press undo, the amount becomes "$100.0".

tbm commented 11 years ago

Comment author: unknown user

Thanks!

It looks to me, from what I see happening when I perform the undos, as though the problem relates to auto-indent modifying the buffer at a point where undo assumes it will not be changed.

I have experimented with changing the condition in ledger-post-maybe-align to prevent auto-indent firing after an undo:

After making this change, I can no longer reproduce the problem: both auto-indent and undo appear to work properly. However, I cannot claim to be an expert on Emacs programming, so I have no idea whether this is actually a robust fix.

tbm commented 11 years ago

Comment author: Craig Earls (@enderw88)

Thanks. It looks like it is a problem with auto-indent. I will look into it.

tbm commented 11 years ago

Comment author: unknown user

I primarily use GNU Emacs from bzr (currently 24.3.50.1). However, I can also reproduce this in Debian's build of GNU Emacs 23.4.1.

I do have auto indent turned on. If I disable it then I can no longer reproduce the problem.

The other information you requested follows, though note that I get the same results even if I use emacs -q to bypass everything I've customized.

Enabled minor modes: Auto-Composition Auto-Compression Auto-Encryption Column-Number Delete-Selection File-Name-Shadow Font-Lock Global-Font-Lock Icomplete Iswitchb Line-Number Mouse-Wheel Savehist Shell-Dirtrack Tooltip Transient-Mark Winner

The result of executing (ledger-mode-dump-configuration) in an ielm buffer -- after loading 'cus-edit, which it seems to require -- is:

Group ledger-exec: ledger-binary-path: "ledger" ledger-init-file-name: "~/.ledgerrc" Group ledger-faces: ledger-occur-use-face-shown: t Group ledger-post: ledger-post-auto-adjust-postings: t ledger-post-account-alignment-column: 4 ledger-post-amount-alignment-column: 52 ledger-post-use-completion-engine: :built-in Group ledger-reconcile: ledger-reconcile-default-commodity: "£" ledger-recon-buffer-name: "Reconcile" ledger-narrow-on-reconcile: t ledger-buffer-tracks-reconcile-buffer: t ledger-reconcile-force-window-bottom: nil ledger-reconcile-toggle-to-pending: t ledger-reconcile-default-date-format: "%Y/%m/%d" ledger-reconcile-target-prompt-string: "Target amount for reconciliation " Group ledger-report: ledger-reports: (("bal" "ledger -f %(ledger-file) bal") ("reg" "ledger -f %(ledger-file) reg") ("payee" "ledger -f %(ledger-file) reg @ %(payee)") ("account" "ledger -f %(ledger-file) reg %(account)")) ledger-report-format-specifiers: (("ledger-file" . ledger-report-ledger-file-format-specifier) ("payee" . ledger-report-payee-format-specifier) ("account" . ledger-report-account-format-specifier) ("value" . ledger-report-value-format-specifier)) ledger-clear-whole-transactions: t Group ledger-test: ledger-source-directory: "~/ledger/" ledger-test-binary: "/Products/ledger/debug/ledger" Group ledger-texi: ledger-texi-sample-doc-path: "~/ledger/doc/sample.dat" ledger-texi-normalization-args: "--args-only --columns 80" ledger-highlight-xact-under-point: t ledger-use-iso-dates: nil Group ledger-schedule: ledger-schedule-buffer-name: "Ledger Schedule" ledger-schedule-look-backward: 7 ledger-schedule-look-forward: 14 ledger-schedule-file: "~/FinanceData/ledger-schedule.ledger"

tbm commented 11 years ago

Comment author: Craig Earls (@enderw88)

Cannot reproduce using your instructions, undo works as expected using Aquamacs 2.4.

What version of Emacs are you using?
What other modes are active when this happens?
Do you have auto indent turned on?

Please post results of running: (ledger-mode-dump-configuration) from an ielm buffer.

tbm commented 11 years ago

Comment author: unknown user

I have several times managed to break ledger-mode such that my ledger buffer was left in a corrupt state. If I had saved without noticing this, it would have led to data loss.

In each case, all I had done was to load the file, perform basic editing operations (e.g. deleting comments), and then use the standard undo and redo functions: Emacs would then either perform those operations in the wrong place (e.g. deleting text a few characters to the left of the text that should have been deleted), or refuse to perform them at all with the message "primitive-undo: Changes to be undone are outside visible portion of buffer".

I can reliably trigger the problem with the following steps:

  1. Create a new buffer.
  2. Type as follows, where quoted text is typed verbatim: M-x ledger-mode RET "2013/05/01 foo" RET SPACE "Expenses:Foo" SPACE SPACE "$10.00" RET
  3. Press an undo key (C-x u, C-/, etc) repeatedly until things stop happening.

Expected results: buffer is empty, since everything entered in it has been undone.

Actual results: buffer contains the text "o $10"