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

bm-buffer-save: Wrong type argument: listp, \.\.\. #39

Open
breadncup opened this issue Dec 12, 2021 · 9 comments
Open

bm-buffer-save: Wrong type argument: listp, \.\.\. #39

breadncup opened this issue Dec 12, 2021 · 9 comments

Comments

@breadncup
Copy link

When bm-buffer-persistence is t, it looks like it's trying to save all bm information.

However, it doesn't seem to work.

I got

bm-buffer-save: Wrong type argument: listp, \.\.\.

The Backtrace shows

  bm-repository-remove("/Users/ysong/org/personal/data/98/7982F8-57F5-4B57-8959-697522A860FE/MonkeyIsland2-Walkthrough-Daniel2.txt")
  bm-buffer-save()
  #f(compiled-function (buffer) #<bytecode 0x1fef21e97305>)(#<buffer MonkeyIsland2-Walkthrough-Daniel2.txt<7982F8-57F5-4B57-8959-697522A860FE>>)
  mapc(#f(compiled-function (buffer) #<bytecode 0x1fef21e97305>) (#<buffer *Messages*> #<buffer bm.el> #<buffer  *Minibuf-1*> #<buffer *Help*> #<buffer todo_personal_project.org> #<buffer note_emacs.org> #<buffer *scratch*> #<buffer MonkeyIsland2-Walkthrough-Daniel2.txt<7982F8-57F5-4B57-8959-697522A860FE>> #<buffer *spacemacs*> #<buffer  *Minibuf-0*> #<buffer  *code-conversion-work*> #<buffer  *Echo Area 0*> #<buffer  *Echo Area 1*> #<buffer MonkeyIsland2-Walkthrough-Daniel.txt> #<buffer MonkeyIsland2-Walkthrough-Daniel2.txt<MonkeyIsland2>> #<buffer MonkeyIsland2-Walkthrough.txt> #<buffer magit: .spacemacs.d<4>> ...))
  bm-buffer-save-all()

Could you figure out why it happens?

@breadncup
Copy link
Author

breadncup commented Dec 12, 2021

Looks like this patch make the fix?

$ cat bm-updated3.diff.patch
--- bm-original.el	2021-12-12 14:51:34.000000000 -0800
+++ bm-updated3.el	2021-12-17 15:04:52.000000000 -0800
@@ -1604,6 +1604,12 @@
     (when (assoc key bm-repository)
       ;; remove all occurances
       (while bm-repository
+        (if (equal "..." (cdr bm-repository))
+            (progn
+              (setq bm-repository (cdr bm-repository))
+              (loop-continue)
+              )
+          )
         (if (not (equal key (car (car bm-repository))))
             (setq repository (append repository (list (car bm-repository)))))
         (setq bm-repository (cdr bm-repository)))

Would you check the above, please?

@tavisrudd
Copy link

I ran into this also. The patch isn't sufficient as I had a key \.\.\. inside my bm-repository file. I had to manually remove it.

(car (car bm-repository)) is what triggers the error when it encounters that corrupt key.

@breadncup
Copy link
Author

Yes, the patch I put in previously is not working. I wish the contributor for this would fix this issue as soon as possible.

@joodland
Copy link
Owner

Sorry for the extremely late reply. I have been away from Emacs developing bm.el for a long time due to work and other priorities.

I am unable to reproduce this. When testing persistent bookmarks locally, everything works as it should. I suspect that the error is due to a corrupt repository file.

@joodland joodland closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2022
@bdevel
Copy link

bdevel commented Nov 14, 2023

This patch fixes it for me. It filters anything that's not a list from bm-repository. Not sure if this introduces any bugs, or why \.\.\. is in the list in the first place.

(while bm-repository
+	(if (not (listp (car bm-repository)))
+	    (setq bm-repository (cdr bm-repository))
          (if (not (equal key (car (car bm-repository))))

Full function:

(defun bm-repository-remove (key)
  "Remove data for a buffer from the repository identified by KEY."
  (let ((repository nil))
    (when (assoc key bm-repository)
      ;; remove all occurances
      (while bm-repository
	(if (not (listp (car bm-repository)))
	    (setq bm-repository (cdr bm-repository))
          (if (not (equal key (car (car bm-repository))))
              (setq repository (append repository (list (car bm-repository))))))
        (setq bm-repository (cdr bm-repository)))
      (setq bm-repository repository))))

@joodland
Copy link
Owner

Thanks, but I want to verify this with a "corrupt" data file. Do you (or anybody else) have a repository file that cannot load without this patch?

@joodland joodland reopened this Nov 20, 2023
@rdiez
Copy link

rdiez commented Feb 14, 2024

I have hit a similar issue often during the past years, and it happened again today.

I was editing my usual text file with various notes, and then I wanted to open another text file with find-file, and I got this error at the bottom:

Wrong type argument: number-or-marker-p, nil

The other text file did not open then. That is, the error stopped find-file from working.

I think that bm is not mentioned in that error message there, I am not sure now.

This happens only sporadically. I have got used to switching to another buffer, and then using find-file again, which then works. Afterwards, everything works fine.

Today I looked at buffer *Messages*, and I saw the full error message there:

bm-buffer-restore: Wrong type argument: number-or-marker-p, nil

I think that my bm-repository file becomes corrupt sometimes. I have seen such an error in other circumstances, so I learnt to delete it to recover. It happens sometimes on Emacs start-up, so I lose the persistent bookmarks.

Most of the times, I haven't done anything dangerous with the PC, or within Emacs, which could corrupt bm-buffer-restore. And usually, I do not need to do anything special to fix it, I guess that by changing some bookmarks then next time the file is saved properly and is not corrupt anymore.

I am sorry that I didn't capture an example of a corrupted file. I'll pay more attention the next time around.

I would bet that bm is writing an invalid bm-repository file under some circumstances. This is an excerpt of this file:

((position . 70176)
(time . 1707490293.2360196)
(temporary-bookmark)
(annotation)
(before-context-string . "blah blah

- blah blah")
(after-context-string . "blah blah"))

The first thing I noticed is that the new-line characters are not escaped/quoted in any way, so that is why the strings look weird, as we are not normally accustomed to such multiline strings. I am guessing that bm is not quoting some characters properly, and then the bm-repository becomes "corrupt", that is, it is generated with an invalid format.

I wonder when bm decides to regenerate the bm-repository file. It doesn't do it straight away when I add a bookmark, or when I save the text file. It doesn't either when I use find-file to open another file. It does when I exit Emacs, but that would normally be too late, as you do not want to lose the bookmarks for the past hours. I wonder also why the error happened when I tried to use find-file. My guess is that bm is trying to read the existing bm-repository when I open a new file, in case there are bookmarks for that file stored.

I do not know much about bm, but that's not going to stop me from making wild suggestions. ;-) The following would help prevent file corruption as much as possible, make it easier to collect a corrupt file:

  1. Regenerate bm-repository as bm-repository.new
  2. Move bm-repository to bm-repository.old
  3. Move bm-repository.new to bm-repository

Moving a file is an atomic operation which makes sure you do not corrupt an existing valid file. Keeping at least one backup file would increase the chances of recovering a corrupt file for analysis.

When bm encounters an error, it should not let it bubble all the way up, interrupting an unrelated operation like find-file . bm should improve its error-handling logic with a try/catch which does the following:

  1. Prepend the filename to the error message, so that you get an error like this

bm: Error parsing "~/.emacs.d-or-wherever-configured/bm-repository": blah blah

The user will then immediately realise where the error is coming from, and what file may be corrupt.

  1. The error should not be raised further, but only output it with display-warning.

That will catch the user's attention without interrupting the current operation.

@rdiez
Copy link

rdiez commented Feb 15, 2024

This problem happened again today, and I think this time I found the potential problem.

I cannot post the suspect bm-repository file here because it has confidential information. But I hope you can reproduce the issue anyway.

I normally set the coding system in my text files like this:

(set-buffer-file-coding-system 'utf-8-with-signature-dos)

This way, even the old Notepad in Windows XP can open text files with international characters without any trouble.

I had set a bookmark near a German 'Ü' character, so the corresponding excerpt in bm-repository looked like this:

(after-context-string . "\"\303\234blahblah")

303 and 234 are the octal numbers for hex 0xC3 0x9C, which is the UTF-8 encoding for 'Ü'.

That bm-repository file failed to parse, but a new, similar one saved later worked fine.

I am not quite certain that the 'Ü' above is the cause, as there were some other changes in bm-repository which I didn't remove, but nevertheless I found an interesting and suspect behaviour.

When I compared the suspect bm-repository to the later one, Emacs' ediff marked these lines as different, although the looked the same:

(after-context-string . "\"\303\234blahblah")
(after-context-string . "\"\303\234blahblah")

In the suspect bm-repository, Emacs is displaying \303 and \234 for characters 0xC3 and 0x9C, so that those numbers are red and the cursor jumps around them at once.

However, in the later bm-repository, the \303 and \234 were normal strings of 4 characters, and I could place the cursor for example between '\' and '3'.

I suspect bm is getting confused with character encoding. I would expect that the encoding of such UTF-8 characters remains the same when bm re-saves bm-repository.

@bdevel
Copy link

bdevel commented Mar 28, 2024

Here's a sample file. I'm not sure what added the ...

;; bm.el -- persistent bookmarks. Do not edit this file.
(("file1.clj" (version . 2) (size . 16971) (timestamp 25956 62043 922338 0) (bookmarks ((position . 3190) (time . 1700683141.869202) (temporary-bookmark) (annotation) (before-context-string . "
  \"\"
  [links]
") (after-context-string . "  (let [suspect-")) ((position . 4395) (time . 1700679725.50607) (temporary-bookmark) (annotation) (before-context-string . "(loop [out   {}
") (after-context-string . "                ")) ((position . 5946) (time . 1700007634.545421) (temporary-bookmark) (annotation) (before-context-string . "content link)))
") (after-context-string . "             (le")) ((position . 6344) (time . 1700007634.545434) (temporary-bookmark) (annotation) (before-context-string . "link
           ") (after-context-string . "      (not (empt")) ((position . 6812) (time . 1700683167.019963) (temporary-bookmark) (annotation) (before-context-string . "     (->> links
") (after-context-string . "                ")) ((position . 8800) (time . 1700007634.545437) (temporary-bookmark) (annotation) (before-context-string . " )
      )
  )

") (after-context-string . "(defn add-analys")))) ("file2.clj" (version . 2) (size . 10580) (timestamp 25956 62044 898623 0) (bookmarks ((position . 8618) (time . 1700675668.744828) (temporary-bookmark) (annotation) (before-context-string . "             []
") (after-context-string . "                ")))) ("file3.clj" (version . 2) (size . 11246) (timestamp 25956 62046 161409 0) (bookmarks ((position . 8634) (time . 1700260380.557019) (temporary-bookmark) (annotation) (before-context-string . "             })
") (after-context-string . "                ")) ((position . 9453) (time . 1700674855.157526) (temporary-bookmark) (annotation) (before-context-string . " 
    (if error
") (after-context-string . "      (track-hos")) ((position . 10878) (time . 1700681273.85643) (temporary-bookmark) (annotation) (before-context-string . "r?url=yes\")

  
") (after-context-string . "  (expand-url \"h")))) ("file4.rb" (version . 2) (size . 16497) (timestamp 25995 14513 391384 0) (bookmarks ((position . 295) (time . 1703186480.700471) (temporary-bookmark) (annotation) (before-context-string . "clear_database

") (after-context-string . "    assert_equal")))) ("file5.el" (version . 2) (size . 1483) (timestamp 26008 34494 627617 0) (bookmarks ((position . 303) (time . 1704494666.940916) (temporary-bookmark) (annotation) (before-context-string . "a popup toolbar
") (after-context-string . "  (setq tabbar-r")))) ("file6.rb" (version . 2) (size . 13607) (timestamp 26013 45361 731150 0) (bookmarks ((position . 9737) (time . 1704826265.22963) (temporary-bookmark) (annotation) (before-context-string . "the right one..
") (after-context-string . "        $logger.")))) ("/file7.rb" (version . 2) (size . 8497) (timestamp 26033 28974 812028 0) (bookmarks ((position . 3469) (time . 1706123322.472216) (temporary-bookmark) (annotation) (before-context-string . " 30.0).round(),
") (after-context-string . "        \"Channel")))) ("file8.py" (version . 2) (size . 33117) (timestamp 26043 55985 437729 0) (bookmarks ((position . 23121) (time . 1706643617.444897) (temporary-bookmark) (annotation) (before-context-string . "s per directory
") (after-context-string . "        subdir =")) ((position . 21963) (time . 1706643617.444875) (temporary-bookmark) (annotation) (before-context-string . "===============
") (after-context-string . "    # Thumbnails")) ((position . 4112) (time . 1706643617.444891) (temporary-bookmark) (annotation) (before-context-string . "th_state = \"ok\"
") (after-context-string . "            log.")) ((position . 25741) (time . 1706643617.444899) (temporary-bookmark) (annotation) (before-context-string . "         break

") (after-context-string . "        #today_d")))) ("file9.rb" (version . 2) (size . 19643) (timestamp 26049 16158 354231 0) (bookmarks ((position . 3124) (time . 1707163066.495678) (temporary-bookmark) (annotation) (before-context-string . "s_a?(Exception)
") (after-context-string . "            $log")) ((position . 15637) (time . 1707163044.458348) (temporary-bookmark) (annotation) (before-context-string . "el_search_queue
") (after-context-string . "    $logger.info")))) ("file10.rb" (version . 2) (size . 17967) (timestamp 26049 16159 891441 0) (bookmarks ((position . 2591) (time . 1707162963.900269) (temporary-bookmark) (annotation) (before-context-string . "ger.reset_jobs\"
") (after-context-string . "    @unsorted_jo")) ((position . 3007) (time . 1707163025.994837) (temporary-bookmark) (annotation) (before-context-string . "d of their work
") (after-context-string . "        t.join(2")) ((position . 3339) (time . 1707162963.900254) (temporary-bookmark) (annotation) (before-context-string . "_devices.blank?
") (after-context-string . "      @device_qu")))) ...)

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

No branches or pull requests

5 participants