Added dependency on info and removed duplicated operation

[cage2]

Hi Bastian!

This PR should address #73 and part of #74.

The bug was setting twice the buffer as modified, it was actually harmless but i can see no reason to keeps it anyway! :)

Bye! C.

[bastibe]

I don't think I understand the bug. It seems to me that the proposed change would leave the buffer-modified-bit unchanged when an annotation is deleted, which doesn't seem to make sense. Could you explain the reasoning behind that change?

Otherwise, this looks good.

[cage2]

On Thu, Jul 02, 2020 at 04:47:48AM -0700, Bastian Bechtold wrote:

Hello Bastian!

I don't think I understand the bug. It seems to me that the proposed change would leave the buffer-modified-bit unchanged when an annotation is deleted, which doesn't seem to make sense. Could you explain the reasoning behind that change?

Sure! I think explaining my change can also help me realize if this patch was a good idea or not! :)

I noticed that in the code we explicitly set the buffer as modified in just two points:

when all the annotations are removed from buffer:

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

and when a single annotation is created/modified/erased:

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

But, for example, the code does not set the buffer as modified when load annotation from the database, in this case it just restore the value of the modified status before the loading of annotations occurs.

This behaviour does makes sense to me (explicit marks the buffer only when specific changes occurs regarding the annotations) and is what i would expect from the package.

But there is a single (as far as i know) point where the modified status is set to true in an implicit way, when a single annotation is deleted.

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

This introduce an implicit side effect that could be difficult to track when a function calls 'annotate-change-annotation' and, moreover, the only function that calls 'annotate-change-annotation' is 'annotate-annotate' where the buffer's status is set as modified anyway (at the end of the function's body).

I think removing the side effect hidden into 'annotate-change-annotation' effect from 'annotate-change-annotation' could prevents some bugs in the future where (and if ;-) 'annotate-change-annotation' is called by some other functions.

Moreover we could even implement what articuluxe asked in #74, for example using a customizable variable that prevent marking the buffer as modified.

Looking forward for your opinion and, if i was not able to explain my idea feel free to ask, of course! :)

Bye! C.

[bastibe]

I see! Thank you for the explanation, I had missed the fact that annotate-annotate already marked the buffer as modified. It all makes sense now, and I agree with your assessment. Good catch on that bug!

[cage2]

Hi Bastian!

I see! Thank you for the explanation, I had missed the fact that annotate-annotate already marked the buffer as modified. It all makes sense now, and I agree with your assessment. Good catch on that bug!

I have to thanks @articuluxe because otherwise i think i never spotted it :)

OK, so going to merge in a second

Bye! C.