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

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

Merged
merged 6 commits into from
Nov 23, 2020

Conversation

cage2
Copy link
Collaborator

@cage2 cage2 commented Nov 6, 2020

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.

of multiline annotations.

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
Copy link
Owner

bastibe commented Nov 9, 2020

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

@cage2
Copy link
Collaborator Author

cage2 commented Nov 9, 2020 via email

@bastibe
Copy link
Owner

bastibe commented Nov 11, 2020

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!

  'annotate--position-inside-annotated-text-p';
- fixed some typos.
@cage2
Copy link
Collaborator Author

cage2 commented Nov 11, 2020 via email

@cage2
Copy link
Collaborator Author

cage2 commented Nov 22, 2020

Hi!

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

Bye!
C.

@cage2 cage2 merged commit 9320918 into bastibe:master Nov 23, 2020
@cage2 cage2 deleted the fix-regression-multiline-annotations branch July 16, 2023 10:08
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

Successfully merging this pull request may close these issues.

2 participants