bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

[Replace] button does not edit annotations; throws abbreviate-file-name: Wrong type argument #108

Closed glvno closed 3 years ago

glvno commented 3 years ago

Hello again!

After reinstalling Spacemacs, I'm suddenly having trouble with the replace button. It fails to edit the annotation and instead throws abbreviate-file-name: Wrong type argument: stringp, nil. It seems possible that this has something to do with the latest merge, as abbreviate-file-name is only invoked once, and in a section of lisp that got heavily revised.

Another new and potentially related issue is that the *annotations buffer occasionally fails to display the correct annotated text. So the database has the correct region defined numerically as well as the correct string, the text in foo.org is correctly highlighted, but what should be displayed in *annotations is offset by an arbitrary number of characters, usually between 2 and 10.

So if foo.org contains the line

foo bar baz

with the section in bold denoting annotated text, annotate-show-annotation-summary will generate an annotations buffer that includes something like

** Annotated text: "oo bar b"

where the annotated text in quotes is offset by two or more characters.

Please let me know if you need further clarification or information. Also, let me know if I should split the second problem into a separate issue.

Thanks so much for your work on this great package!

cage2 commented 3 years ago

Hi @glvno !

Just a quick reply to write that the first issue is confirmed and reproducible on my system. I can not reproduce the second but just because i need some time to investigate it more to understand what is happening. In the meanwhile i would start working on the edit issue.

Thanks a lot! Bye! C.

glvno commented 3 years ago

Thank you! For future reference, here's a tidy screenshot of the offset issue:

image

cage2 commented 3 years ago

Hi @glvno !

Turned out that the "[replace]" button's callback was severely broken! I just pushed a patch (#109) that supposed to fix this issue.

May i ask to take a look at it?

Thanks! C.

glvno commented 3 years ago

Hi @cage2,

Sorry for the delay; I don't have a lot of experiences with quelpa recipes, so it took me a second to figure out how to test your patch.

The patch works wonderfully. Thanks so much for your quick work!

I use this package extensively, and I cannot overstate how grateful I am for your help.

I'm going to keep an eye out to see if i can figure out how to consistently replicate the offset issue.

Thanks again!

m

glvno commented 3 years ago

Hello again!

I can now reliably replicate the offset problem. See the screen recording below.

The version I was using that did not have this problem was 54ac759facadacbfea5c1e7c2975e2da6434cdda from March 22, which preceded the introduction of saving annotations before displaying the *annotations* buffer in d2841bad65d1d419ac22355241a847fc500a0d5f.

I'm wondering if perhaps calling annotate-show-annotation-summary is causing annotations to be saved too early, before the package accounts for any changes in the position of annotated regions too late, after the annotations buffer has already been created. One day I will be good enough with elisp to actually help figure out what's going on, but for now I just have wild guesses.

Thanks!

m

EDIT: The more I think about this, the more sense it makes.

The question to answer is this: How is it that two different renderings using the same database can be selecting different text?

The answer, it seems to me, could be that the *annotations* buffer is using region data from the second-to-last save (i.e., the save that directly preceded the save that accompanies annotate-show-annotation-summary, whereas the highlighting is using region data from the current db.

I hope this helps!

https://user-images.githubusercontent.com/25870908/119716664-f4ce5980-be2a-11eb-8a40-9918751e08d5.mov

cage2 commented 3 years ago

Hello again!

Hi @glvno !

I can now reliably replicate the offset problem. See the screen recording below.

Thank you, watching your video and reading your message i was quickly able to understand (i hope so!) where the problem is! :)

EDIT: The more I think about this, the more sense it makes.

In my opinion it makes a lot of sense, indeed! :)

I tried to fix the problem in the same branch (#109) in the commit message you could find an explanation of the issue (i think it is the same you give in your message) but i fear that my English is not good enough to express well what the problem was.

Thank you very much for your detailed report, i hope you'll learn some elisp soon, maybe you are going to want to became a collaborator for this project in the future! :)

Bye! C.

glvno commented 3 years ago

The new fix works great! Thanks so much.

I'm definitely hoping to be able to contribute to this project soon.

Thanks for your help!

m

cage2 commented 3 years ago

On Thu, May 27, 2021 at 07:46:09AM -0700, glvno wrote:

Hi!!

The new fix works great! Thanks so much.

That's wonderful! :)

I'm definitely hoping to be able to contribute to this project soon.

Looking forward for it! :)

Thanks for your help!

Thanks for your excellent report, i would like to mention you in the NEWS file for the next version of the package; are you OK with that? And, if yes, is "glvno" the right name to mention you in the file?

Bye! C.

glvno commented 3 years ago

i would like to mention you in the NEWS file for the next version of the package; are you OK with that? And, if yes, is "glvno" the right name to mention you in the file?

Absolutely! "glvno" is fine.

m

cage2 commented 3 years ago

Hi!

fixed by #109

Thanks! C.