search

bastibe / annotate.el

Annotate.el
Other
388 stars 20 forks source link

Added signalling errors where appropriate. #77

Closed cage2 closed 4 years ago

cage2 commented 4 years ago

Hi @bastibe!

I am starting to working on what we discussed in #76, adding errors to developers oriented functions. To me the first candidate was the function that load the annotation database from a file.

I am going to add errors one function for commit because i think would be simpler to evaluate the changes.

Bye! C.

cage2 commented 4 years ago

Hi @bastibe!

I am not able to find any other function where adding errors signalling could be useful. So looking forward for your comments! :)

Bye! C.

cage2 commented 4 years ago

On Mon, Aug 10, 2020 at 11:49:43PM -0700, Bastian Bechtold wrote:

Hi @bastibe!!

Looks great!

I wonder if it might be better to add an optional ignore-errors argument to annotate-load-annotation-data instead of creating a new function. This might be a bit more concise.

To be honest as the function 'annotate-load-annotation-data-ignore-errors' is used here and there in the code, replacing each occurrence of said function with:

(ignore-errors (annotate-load-annotation-data ...

would be not so elegant.

Anyway i left the decision to you, as usual! :)

Bye! C.

bastibe commented 4 years ago

Sorry for not being precise (again). What I meant was to change the function to

(defun annotate-load-annotation-data (&optional ignore-errors) ...)

and call it variously as (annotate-load-annotation-data) or (annotate-load-annotation-data t).

But I am entirely OK with the current solution as well.

cage2 commented 4 years ago

On Tue, Aug 11, 2020 at 08:45:36AM -0700, Bastian Bechtold wrote:

Hi Bastibe!!!

Sorry for not being precise (again).

No, it is my fault, your message was very clear, it is me that did not pay the necessary attention reading it. :(

What I meant was to change the function to

(defun annotate-load-annotation-data (&optional ignore-errors) ...)

and call it variously as (annotate-load-annotation-data) or (annotate-load-annotation-data t).

This is a much more elegant solution! :)

Bye! C.

cage2 commented 4 years ago

Hi bastibe!

OK, merging! Thak you for your support! :)

Bye! C.

bastibe commented 4 years ago

My pleasure! Thank you for all the hard work!