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

data loss on delete-trailing-whitespace #13

Closed
ghost opened this issue Oct 2, 2014 · 18 comments
Closed

data loss on delete-trailing-whitespace #13

ghost opened this issue Oct 2, 2014 · 18 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 2, 2014

Hi! AWESOME plugin!

But,
when I open relatively large text file in text mode and then
fold some parts of it. And then execute delete-trailing-whitespace
(I had it actually on before-save-hook) then most of the folded text
disappears, leaving only one or 2 first lines of the folded text intact.

I use GNU Emacs 24.4.50.1 on Debian Wheezy 64bit.
Do you need more detailed configuration / example files to reproduce ?

Best regards,
Svjatoslav

@zenozeng
Copy link
Owner

zenozeng commented Oct 3, 2014

Sorry for my late response, but I failed to reproduce this bug. Can you provide some more detailed steps
to reproduce it?

@ghost
Copy link
Author

ghost commented Oct 6, 2014

Hello!

Here I tried to compile more detailed bug report:

(See attached ZIP file)

OS version:

cat /etc/issue
Debian GNU/Linux 7 \n \l

uname -a
Linux sws 3.16-0.bpo.2-amd64 #1 SMP Debian 3.16.3-2~bpo70+1 (2014-09-21) x86_64 GNU/Linux

GNU Emacs version: 24.4.50.1

Bug might be reproducable on other emacs versions too, I haven't tried but I can try If it helps.

Also if you like I can provide VirtualBox or VMWare virtual machine with OS + Emacs setup. 

Zip file contains .emacs.d stripped to bare minimum with yafolding.el installed where
I managed to reproduce the bug.

Steps to reproduce the bug:

* Open "sample text.txt" (included in ZIP file) in emacs.

* Load yafolding plugin: M-x yafolding-mode

* Fold block starting with: Üks implementatsioon on:
  Using Ctrl + Enter

* Execute delete-trailing-whitespace using:
  M-x delete-trailing-whitespace

After these actions most of the content of folded text block is gone.
I suspect this might be because content of the folded block
could be recursively folded again.

Let me know if there is any way I can help further.

Best regards,

Svjatoslav

I suspect GIThub has bug and does not show attached ZIP file. So I sent it by email to
you just in case.

@zenozeng
Copy link
Owner

zenozeng commented Oct 6, 2014

Yeah, I succeed to reproduce the bug with your sample text. I will dig into this issue after finishing my homework. Thanks!

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

Tried using (put-text-property beg end 'read-only t), but causes a lot of problems.

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

modification-hooks
This property's value is a list of functions to be called if any character within the overlay is changed or if text is inserted strictly within the overlay.
The hook functions are called both before and after each change. If the functions save the information they receive, and compare notes between calls, they can determine exactly what change has been made in the buffer text.

When called before a change, each function receives four arguments: the overlay, nil, and the beginning and end of the text range to be modified.

When called after a change, each function receives five arguments: the overlay, t, the beginning and end of the text range just modified, and the length of the pre-change text replaced by that range. (For an insertion, the pre-change length is zero; for a deletion, that length is the number of characters deleted, and the post-change beginning and end are equal.)

If these functions modify the buffer, they should bind inhibit-modification-hooks to t around doing so, to avoid confusing the internal mechanism that calls these hooks.

Text properties also support the modification-hooks property, but the details are somewhat different (see Special Properties).

https://www.gnu.org/software/emacs/manual/html_node/elisp/Overlay-Properties.html#Overlay-Properties

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

but the modification-hooks were not fired. It's fired.

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

but modification-hooks are called when it's already going to change.

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

Did not find a element way to do this. So maybe advising delete-trailing-whitespace?

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

Or maybe:

(add-hook 'before-save-hook (lambda ()
    (yafolding-show-all) 
    (cleanup))

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

in emacs-24.3/lisp/simple.el

(defun delete-trailing-whitespace (&optional start end)
  "Delete trailing whitespace between START and END.
If called interactively, START and END are the start/end of the
region if the mark is active, or of the buffer's accessible
portion if the mark is inactive.

This command deletes whitespace characters after the last
non-whitespace character in each line between START and END.  It
does not consider formfeed characters to be whitespace.

If this command acts on the entire buffer (i.e. if called
interactively with the mark inactive, or called from Lisp with
END nil), it also deletes all trailing lines at the end of the
buffer if the variable `delete-trailing-lines' is non-nil."
  (interactive (progn
                 (barf-if-buffer-read-only)
                 (if (use-region-p)
                     (list (region-beginning) (region-end))
                   (list nil nil))))
  (save-match-data
    (save-excursion
      (let ((end-marker (copy-marker (or end (point-max))))
            (start (or start (point-min))))
        (goto-char start)
        (while (re-search-forward "\\s-$" end-marker t)
          (skip-syntax-backward "-" (line-beginning-position))
          ;; Don't delete formfeeds, even if they are considered whitespace.
          (if (looking-at-p ".*\f")
              (goto-char (match-end 0)))
          (delete-region (point) (match-end 0)))
        ;; Delete trailing empty lines.
        (goto-char end-marker)
        (when (and (not end)
           delete-trailing-lines
                   ;; Really the end of buffer.
           (= (point-max) (1+ (buffer-size)))
                   (<= (skip-chars-backward "\n") -2))
          (delete-region (1+ (point)) end-marker))
        (set-marker end-marker nil))))
  ;; Return nil for the benefit of `write-file-functions'.
  nil)

http://stackoverflow.com/questions/17170793/what-does-regex-f-mean

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

It's already 6:00 am now and I have a class at 8:00 am. I guess I have to go to sleep now.

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

seems that re-search-forward \s-$ will move to the end of line.

@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

did not find anything like re-search-forward-hook and advising is dirty.

@zenozeng zenozeng closed this as completed Oct 7, 2014
zenozeng added a commit that referenced this issue Oct 7, 2014
@zenozeng
Copy link
Owner

zenozeng commented Oct 7, 2014

@svjatoslavagejenko The content of the folded block won't be recursively folded again. (yafolding-show-region beg end) is called before hiding a region.

@ghost
Copy link
Author

ghost commented Oct 25, 2014

I'm very sorry. I can't help any way when it comes to lisp because currently I'm complete newbie to Emacs and totally alien to elisp.

Meanwhile I found out that emacs org-mode does folding very well for structured notes and doesn't have this data loss issue. Maybe it's source code holds clues how to better handle whitespace trimming. Org-mode drawback is: It relies on special heading markers so it cannot fold arbitrary indented text file as yafolding can.

Thank you very much for a workaround!

I inserted this:
(add-hook 'before-save-hook (lambda ()
(yafolding-show-all)
(cleanup)))

into yafolding.el and now data is preserved on save with delete-trailing-whitespace also in the before-save-hook.

Best regards,
Svjatoslav

@zenozeng zenozeng reopened this Oct 26, 2014
@dieggsy
Copy link

dieggsy commented Jul 13, 2016

Hey there, I saw this was re-opened. Has this been fixed? I haven't experienced any issues so far, but I wouldn't want to lose data... I'm also not sure where to put the fix on the front page. Should that just be in my init file?

@zenozeng
Copy link
Owner

@therockmandolinist Sorry for my late response. Sorry this issue won't be fixed. The data loss will happen if you call delete-trailing-whitespace when something in this file is folded. If you have to call delete-trailing-whitespace, wrap it with:

(lambda ()
    (yafolding-show-all)
    (delete-trailing-whitespace))

@dieggsy
Copy link

dieggsy commented Aug 8, 2016

As a suggestion, it might be better to leave this as an open as a known issue and tag it with 'help wanted' or something like that. It would make the issue more visible to people to know about as well as anyone who might know how to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants