bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

Files with empty annotations #111

Closed mmauger closed 3 years ago

mmauger commented 3 years ago

I know there is protection against saving annotations about a file when there are no actual annotations in the file, but I am seeing it happen. Looking at the function annotate-save-annotations, I think the sequence to filter out empty annotations is incorrect:

    ;; skip files with no annotations
    (annotate-dump-annotation-data (cl-remove-if (lambda (entry)
                                                   (null (cdr entry)))
                                                 all-annotations))

should be:

    ;; skip files with no annotations
    (annotate-dump-annotation-data (cl-remove-if (lambda (entry)
                                                   (null (cadr entry)))
                                                 all-annotations))

I've added the change locally and it seems to work, but I don't have enough time to properly debug right now.

cage2 commented 3 years ago

Hi @mmauger !

Thanks for using this software and also using your time to figuring out how to fix the bugs!

Yes, the code you patched (edit: i meant "the code you shown, before your patch") did not worked as intended but i guess you likely are using an old version of this package as the code in this repository at line 1412:

https://github.com/bastibe/annotate.el/blob/master/annotate.el#L1412

changed the test from: (null (cdr entry)) to: (null (annotate-annotations-from-dump entry)) where annotate-annotations-from-dump is just (nth 1 entry)) and i guess it is semantically equivalent to you patched code.

If not already did, i suggest you to update your code using Melpa unstable or the code you can find here to get rid of this bug, and also many others i would say!

@bastibe , do you think would be a good idea to tag the latest version for inclusion in Melpa stable?

Bye! C.

mmauger commented 3 years ago

I appreciate the confirmation that senility has not completely overtaken me.

I generally stick to melpa.stable at work--bizarre behavior during the middle of the day is not appreciated so I use stable.

My one fear of melpa is that it offers both repo/feeds which isolates developers from the meaning and reliance upon release stability unless they have lots of production/business users. The "techie" in us wants to us nightly melpa; the "grunt" in us wants to not have to explain to our boss why we are broken so we use stable melpa.

cage2 commented 3 years ago

Hi!

I appreciate the confirmation that senility has not completely overtaken me.

No problem i think i can understand you very well! ;-)

I generally stick to melpa.stable at work--bizarre behavior during the middle of the day is not appreciated so I use stable. My one fear of melpa is that it offers both repo/feeds which isolates developers from the meaning and reliance upon release stability unless they have lots of production/business users. The "techie" in us wants to us nightly melpa; the "grunt" in us wants to not have to explain to our boss why we are broken so we use stable melpa.

I can see the point, this is more or less the reason i try to explain to other people for using Debian stable on production servers. :)

But anyway, i think the new version of annotate is going to melpa stable, soon.

Thanks again for using this package! C.