bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

Fixed a regression and some bugs related to incorrect calculation of multiline annotations. #84

Closed cage2 closed 3 years ago

cage2 commented 3 years ago

Hi @bastibe !

I introduced some regression and i failed to manage correctly the multi line annotations. :( The bugs are too difficult for me to explain so i preferred to add use cases to reproduce the issues and proposed fix. Hope this is OK.

There are some thing still missing in this PR (probably just a couple of docstrings) but the bulk of the fix is here, i submitted this PR now so that maybe some user can test this branch and help with testing and i am going to check later what is missing.

Fortunately seems to me that some of the code here should be useful for #83!

Bye!! C.

To reproduce the bugs:

legend:

a = annotated text
* = non annotated text

- First bug

Create a multiline annotation using region.

aaaa
aaaa
aaaa    ####

Place the cursor as below.

aaaa
 ^ cursor
aaaa
aaaa    ####

type a character

a****
aaaa
aaaa    ####

The annotated text has a "gap"

Fix proposed: revert to the old (correct behaviour)

Second bug

aaaa
aaaa
aaaa    ####

Place the cursor as below.

aaaa
^ cursor on the first column
aaaa
aaaa    ####

type some text

***
aaa
aaa    ####

Save (C-x C-s)

you  get an  error  on  the echo  area:  "let*:  Wrong type  argument:
overlayp, nil" and the annotations are not correctly saved.

Fix proposed: remove the offending code.

Third bug

a multiline bug as before

aaaa
aaaa
aaaa    ####

place the cursor here:

aaaa
aaaa
^ cursor
aaaa    ####

type some text

aaaa
*****
aaaa    ####

Then annotate the same line (C-c C-a):

aaaa
aaaa    ####
aaaa    ####

we  introduced  a  annotation  in  the gap  of  the  already  existing
multiline annotation.

Fix proposed: prevents annotating text inside an annotation.
bastibe commented 3 years ago

I currently don't have the time to truly look into this, but I trust you completely to do the right thing!

cage2 commented 3 years ago

On Mon, Nov 09, 2020 at 01:51:04AM -0800, Bastian Bechtold wrote:

Hi Bastian!

I currently don't have the time to truly look into this,

No problem! Just hope everything is fine.

but I trust you completely to do the right thing!

Thank you! I promise i will do my best! :)

Bye! C.

bastibe commented 3 years ago

Hi Cage,

I currently don't have the time to truly look into this,

No problem! Just hope everything is fine.

We are doing very well, thank you. But we had a new baby a few weeks ago, which leave little room for side projects at the moment. I hope you are doing well also!

cage2 commented 3 years ago

On Wed, Nov 11, 2020 at 12:01:15AM -0800, Bastian Bechtold wrote:

Hi Cage,

Hi!!

I currently don't have the time to truly look into this,

No problem! Just hope everything is fine.

We are doing very well, thank you. But we had a new baby a few weeks ago, which leave little room for side projects at the moment.

My congratulations! So you are starting a new adventure! ;-)

I hope you are doing well also!

I am quite OK, things are getting complicated lately here where i live, because of the pandemic, but i am still alive in good shape and confident that everything is going to be alright soon! :)

Bye! C.

cage2 commented 3 years ago

Hi!

I think this PR is ready! I am going to merge it in the next hours.

Bye! C.