From: Tomi Ollila <tomi.ollila@iki.fi>
To: Aaron Ecay <aaronecay@gmail.com>, Robert Horn <rjhorn@alum.mit.edu>,
	notmuch@notmuchmail.org
Subject: Re: bug related to ical
In-Reply-To: <m2obkjlm3f.fsf@guru.guru-group.fi>
References: <m31ul3qo4w.fsf@quad.robs.office> <87d319b899.fsf@gmail.com>
	<m2obkjlm3f.fsf@guru.guru-group.fi>
User-Agent: Notmuch/0.14+51~g62cd13b (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 03 Oct 2012 17:06:47 +0300
Message-ID: <m2391vir9k.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list

On Wed, Oct 03 2012, Tomi Ollila wrote:

> On Wed, Sep 26 2012, Aaron Ecay wrote:
>
>> The problem is in the =E2=80=98notmuch-show-insert-part-text/calendar=E2=
=80=99
>> function.  The call to =E2=80=98icalendar--convert-ical-to-diary=E2=80=
=99 does not
>> create a buffer visiting the temp file, so the call to =E2=80=98set-buff=
er=E2=80=99
>> fails.  The following patch fixes the problem.
>>
>> The ical->diary conversion also doesn=E2=80=99t seem to work =E2=80=93 t=
he calendar
>> attachment shows up as an empty part =E2=80=93 but I guess that=E2=80=99=
s a separate
>> issue (and not addressed by the patch).
>>
>> I guess that part insertion handlers should be called inside a
>> =E2=80=98condition-case=E2=80=99, so that an error inside of one can be =
recovered from,
>> and doesn=E2=80=99t entirely derail the insertion of the messages in the=
 buffer.
>> (I actually made this patch because I was so annoyed that Olivier=E2=80=
=99s
>> buggy test attachment made it impossible for me to read Tomi=E2=80=99s r=
eply.)
>>
>> ----- cut here -----
>>
>> diff --git i/emacs/notmuch-show.el w/emacs/notmuch-show.el
>> index ce5ea6f..4c89d7e 100644
>> --- i/emacs/notmuch-show.el
>> +++ w/emacs/notmuch-show.el
>> @@ -746,7 +746,7 @@ message at DEPTH in the current thread."
>>  	      (icalendar--convert-ical-to-diary
>>  	       (icalendar--read-element nil nil)
>>  	       file t)
>> -	      (set-buffer (get-file-buffer file))
>> +	      (set-buffer (find-file-noselect file))
>>  	      (setq result (buffer-substring (point-min) (point-max)))
>>  	      (set-buffer-modified-p nil)
>>  	      (kill-buffer (current-buffer))
>>
>> ----- cut here -----
>
> The problem seems to be carriage returns in the attachment -- which goes
> unnotified when converting from octet-stream content.
>
> I've got the example in question to "work" with the following patch_
>
> --8<----8<----8<-- cut here  --8<----8<----8<--
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 86130ce..f7c08ee 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -747,17 +747,21 @@ message at DEPTH in the current thread."
>    (notmuch-show-insert-part-header nth declared-type content-type (plist=
-get part :filename))
>    (insert (with-temp-buffer
>  	    (insert (notmuch-get-bodypart-content msg part nth notmuch-show-pro=
cess-crypto))
> -	    (goto-char (point-min))
>  	    (let ((file (make-temp-file "notmuch-ical"))
>  		  result)
> +	      (set-buffer (icalendar--get-unfolded-buffer (current-buffer)))
> +	      (beginning-of-buffer)
> +	      (replace-string "" "")

OK, something mangled my email, there used to be ^M above (as one char, CR
.. maybe "\r" would have worked ?)

Tomi


> +	      (beginning-of-buffer)
>  	      (icalendar--convert-ical-to-diary
>  	       (icalendar--read-element nil nil)
>  	       file t)
> +	      (kill-buffer (current-buffer))
>  	      (set-buffer (get-file-buffer file))
>  	      (setq result (buffer-substring (point-min) (point-max)))
>  	      (set-buffer-modified-p nil)
>  	      (kill-buffer (current-buffer))
> -	      (delete-file file)
> +	      ;;;(delete-file file)
>  	      result)))
>    t)
>=20=20
> --8<----8<----8<-- cut here  --8<----8<----8<--
>
> In our case the icalendar--get-unfolded-buffer doesn't seem
> to do any conversions (the docstring mentions some LF/CR/BLANK
> replacements) -- I just copied that line from icalendar-import-buffer
> which I tried first.
>
> The interesting thing is that the notmuch-icalXXXXXX file is
> still empty ... hmm, the buffer is just not saved (would /dev/null work ;=
))
>
> To do:
> Check why icalendar--get-unfolded-buffer did not do what it advertised;
> the code seems to support the replacements mentioned -- or maybe I failed=
 ;/
>
> The problem with failing to icalendar--convert-ical-to-diary is that
> the buffer is not created. The find-file-noselect shields about that
> problem here but not elsewhere (and why that failure is so "fatal" ???)
> Some shielding in higher level should be implemented (i.e. the
> "condition-case" suggestion Aaron mentioned or something :)
>
>
> self to remember:  M-x debug-on-entry and keys 'd', 'u' and 'c' in debugg=
er :)
>
> Tomi
>
>>
>> --=20
>> Aaron Ecay