Re: [O] [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props

[Jan Malakhovski]

Aaron Ecay  writes:

> Checking the source, date-to-time can raise an error (invalid date).
> format-time-string is a C function, so it’s less easy for me to
> understand.  But it looks like if it raises any errors, these are bugs
> that we want to know about and not suppress.
>
> The ignore-error call was introduced to org-gnus in commit 0dfde2da.
> Based on the commit message, it looks like the problem being solved was
> invalid dates getting passed to date-to-time.
>
> So I think the ignore-error can just wrap the date-to-time call.

Thanks. Ok.

Re: [O] [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props

[Aaron Ecay]

Hi Jan,

2015ko azaroak 4an, Jan Malakhovski-ek idatzi zuen:

[...]

>> Also, what is ignore-errors protecting? The call to date-to-time, or
>> format-time-string? I think the scope of ignore-errors should be as
>> narrow as it can be.
> 
> I have no idea. I moved these lines from org-gnus, equivalent lines in
> org-*other-mail-reader*s don't use ignore-errors at all. I don't use
> gnus so went for the safest change.

Checking the source, date-to-time can raise an error (invalid date).
format-time-string is a C function, so it’s less easy for me to
understand.  But it looks like if it raises any errors, these are bugs
that we want to know about and not suppress.

The ignore-error call was introduced to org-gnus in commit 0dfde2da.
Based on the commit message, it looks like the problem being solved was
invalid dates getting passed to date-to-time.

So I think the ignore-error can just wrap the date-to-time call.

-- 
Aaron Ecay

Re: [O] [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props

[Jan Malakhovski]

Aaron Ecay  writes:

> Can you introduce a let binding so that (date-to-time x) is only called
> once?

Ok.

> Also, what is ignore-errors protecting? The call to date-to-time, or
> format-time-string? I think the scope of ignore-errors should be as
> narrow as it can be.

I have no idea. I moved these lines from org-gnus, equivalent lines in
org-*other-mail-reader*s don't use ignore-errors at all. I don't use
gnus so went for the safest change.

Re: [O] [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props

[Aaron Ecay]

Hi Jan,

2015ko azaroak 3an, Jan Malakhovski-ek idatzi zuen:

[...]

> diff --git a/lisp/org.el b/lisp/org.el
> index c466870..cf0ef1f 100755
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -9960,7 +9960,7 @@ active region."
>(car org-stored-links))
>  
>  (defun org-store-link-props ( plist)
> -  "Store link properties, extract names and addresses."
> +  "Store link properties, extract names, addresses and dates."
>(let (x adr)
>  (when (setq x (plist-get plist :from))
>(setq adr (mail-extract-address-components x))
> @@ -9969,7 +9969,18 @@ active region."
>  (when (setq x (plist-get plist :to))
>(setq adr (mail-extract-address-components x))
>(setq plist (plist-put plist :toname (car adr)))
> -  (setq plist (plist-put plist :toaddress (nth 1 adr)
> +  (setq plist (plist-put plist :toaddress (nth 1 adr
> +(when (setq x (plist-get plist :date))
> +  (setq plist (plist-put plist :date-timestamp
> + (ignore-errors
> +   (format-time-string
> + (org-time-stamp-format t)
> + (date-to-time x)
> +  (setq plist (plist-put plist :date-timestamp-inactive
> + (ignore-errors
> +   (format-time-string
> + (org-time-stamp-format t t)
> + (date-to-time x)))

Can you introduce a let binding so that (date-to-time x) is only called
once?  Also, what is ignore-errors protecting?  The call to date-to-time,
or format-time-string?  I think the scope of ignore-errors should be as
narrow as it can be.

-- 
Aaron Ecay