Use find-buffer-visiting instead of get-file-buffer

Use find-buffer-visiting instead of get-file-buffer

[Al Haji-Ali]

In the function `notmuch-show-insert-part-text/calendar`, get-file-buffer is used which works most of the time except the file is visited under a different name (for example when a directory in `temporary-file-directory` in symlinked).

This can be easily corrected by using `find-buffer-visiting` instead.  

-- Al

notmuch-show-insert-part-text/calendar

[David Bremner]

Dear Al;

I'm not able to reproduce the problem you mention. Can you try (or
look at) the tests in this series and see if the second one matches
the problem you are trying to fix?

d

[PATCH 1/3] test/corpora: add example with text/calendar attachment

[David Bremner]

Initially for testing rendering in Emacs.
---
 test/corpora/attachment/text-calendar.eml | 55 +++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 test/corpora/attachment/text-calendar.eml

diff --git a/test/corpora/attachment/text-calendar.eml b/test/corpora/attachment/text-calendar.eml
new file mode 100644
index 00000000..8e064f9b
--- /dev/null
+++ b/test/corpora/attachment/text-calendar.eml
@@ -0,0 +1,55 @@
+From: David Bremner <bremner@example.com>
+To: "david@tethera.net" <david@tethera.net>
+Subject: test
+Date: Thu Jan 01 00:00:10 +0000
+Message-ID:
+ <YT3PR01MB10572EFC9F7C81F9446214768CEF72@YT3PR01MB10572.CANPRD01.PROD.OUTLOOK.COM>
+Accept-Language: en-CA, en-US
+Content-Language: en-CA
+Content-Type: multipart/alternative;
+	boundary="_000_YT3PR01MB10572EFC9F7C81F9446214768CEF72YT3PR01MB10572CA_"
+MIME-Version: 1.0
+
+--_000_YT3PR01MB10572EFC9F7C81F9446214768CEF72YT3PR01MB10572CA_
+Content-Type: text/plain; charset="iso-8859-1"
+Content-Transfer-Encoding: quoted-printable
+
+This meeting will could have been an email
+
+
+--_000_YT3PR01MB10572EFC9F7C81F9446214768CEF72YT3PR01MB10572CA_
+Content-Type: text/calendar; charset="utf-8"; method=REQUEST
+Content-Transfer-Encoding: base64
+
+QkVHSU46VkNBTEVOREFSDQpNRVRIT0Q6UkVRVUVTVA0KUFJPRElEOk1pY3Jvc29mdCBFeGNoYW5n
+ZSBTZXJ2ZXIgMjAxMA0KVkVSU0lPTjoyLjANCkJFR0lOOlZUSU1FWk9ORQ0KVFpJRDpBdGxhbnRp
+YyBTdGFuZGFyZCBUaW1lDQpCRUdJTjpTVEFOREFSRA0KRFRTVEFSVDoxNjAxMDEwMVQwMjAwMDAN
+ClRaT0ZGU0VURlJPTTotMDMwMA0KVFpPRkZTRVRUTzotMDQwMA0KUlJVTEU6RlJFUT1ZRUFSTFk7
+SU5URVJWQUw9MTtCWURBWT0xU1U7QllNT05USD0xMQ0KRU5EOlNUQU5EQVJEDQpCRUdJTjpEQVlM
+SUdIVA0KRFRTVEFSVDoxNjAxMDEwMVQwMjAwMDANClRaT0ZGU0VURlJPTTotMDQwMA0KVFpPRkZT
+RVRUTzotMDMwMA0KUlJVTEU6RlJFUT1ZRUFSTFk7SU5URVJWQUw9MTtCWURBWT0yU1U7QllNT05U
+SD0zDQpFTkQ6REFZTElHSFQNCkVORDpWVElNRVpPTkUNCkJFR0lOOlZFVkVOVA0KT1JHQU5JWkVS
+O0NOPURhdmlkIEJyZW1uZXI6bWFpbHRvOmJyZW1uZXJAZXhhbXBsZS5jb20NCkFUVEVOREVFO1JP
+TEU9UkVRLVBBUlRJQ0lQQU5UO1BBUlRTVEFUPU5FRURTLUFDVElPTjtSU1ZQPVRSVUU7Q049ZGF2
+aWRAdGV0aA0KIGVyYS5uZXQ6bWFpbHRvOmRhdmlkQHRldGhlcmEubmV0DQpERVNDUklQVElPTjtM
+QU5HVUFHRT1lbi1DQTpUaGlzIG1lZXRpbmcgd2lsbCBjb3VsZCBoYXZlIGJlZW4gYW4gZW1haWxc
+blxuDQpSUlVMRTpGUkVRPURBSUxZO1VOVElMPTIwMjQwODI3VDE3MDAwMFo7SU5URVJWQUw9MQ0K
+VUlEOjA0MDAwMDAwODIwMEUwMDA3NEM1QjcxMDFBODJFMDA4MDAwMDAwMDAwRUE4NDFFNzk5QUZE
+QTAxMDAwMDAwMDAwMDAwMDAwDQogMDEwMDAwMDAwNjkyRkRFNzQzMzNGQ0I0N0JFNEY0REU5MDk0
+OThDQkUNClNVTU1BUlk7TEFOR1VBR0U9ZW4tQ0E6dGVzdCBbSW4tcGVyc29uXQ0KRFRTVEFSVDtU
+WklEPUF0bGFudGljIFN0YW5kYXJkIFRpbWU6MjAyNDA1MjdUMTQwMDAwDQpEVEVORDtUWklEPUF0
+bGFudGljIFN0YW5kYXJkIFRpbWU6MjAyNDA1MjdUMTQzMDAwDQpDTEFTUzpQVUJMSUMNClBSSU9S
+SVRZOjUNCkRUU1RBTVA6MjAyNDA1MjZUMTgyNDEwWg0KVFJBTlNQOk9QQVFVRQ0KU1RBVFVTOkNP
+TkZJUk1FRA0KU0VRVUVOQ0U6MA0KTE9DQVRJT047TEFOR1VBR0U9ZW4tQ0E6DQpYLU1JQ1JPU09G
+VC1DRE8tQVBQVC1TRVFVRU5DRTowDQpYLU1JQ1JPU09GVC1DRE8tT1dORVJBUFBUSUQ6MjEyMjcw
+MDA0Ng0KWC1NSUNST1NPRlQtQ0RPLUJVU1lTVEFUVVM6VEVOVEFUSVZFDQpYLU1JQ1JPU09GVC1D
+RE8tSU5URU5ERURTVEFUVVM6QlVTWQ0KWC1NSUNST1NPRlQtQ0RPLUFMTERBWUVWRU5UOkZBTFNF
+DQpYLU1JQ1JPU09GVC1DRE8tSU1QT1JUQU5DRToxDQpYLU1JQ1JPU09GVC1DRE8tSU5TVFRZUEU6
+MQ0KWC1NSUNST1NPRlQtRE9OT1RGT1JXQVJETUVFVElORzpGQUxTRQ0KWC1NSUNST1NPRlQtRElT
+QUxMT1ctQ09VTlRFUjpGQUxTRQ0KWC1NSUNST1NPRlQtUkVRVUVTVEVEQVRURU5EQU5DRU1PREU6
+SU5QRVJTT05SRVFVSVJFRA0KWC1NSUNST1NPRlQtSVNSRVNQT05TRVJFUVVFU1RFRDpUUlVFDQpY
+LU1JQ1JPU09GVC1MT0NBVElPTlM6W10NCkJFR0lOOlZBTEFSTQ0KREVTQ1JJUFRJT046UkVNSU5E
+RVINClRSSUdHRVI7UkVMQVRFRD1TVEFSVDotUFQxNU0NCkFDVElPTjpESVNQTEFZDQpFTkQ6VkFM
+QVJNDQpFTkQ6VkVWRU5UDQpFTkQ6VkNBTEVOREFSDQo=
+
+--_000_YT3PR01MB10572EFC9F7C81F9446214768CEF72YT3PR01MB10572CA_--
-- 
2.43.0

[PATCH 2/3] test/emacs: add regression test for display of calendar parts.

[David Bremner]

This will hopefully catch breakage due to either changes in
Emacs (especially Gnus) or changes to the notmuch-show code.
---
 test/T450-emacs-show.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 559df8aa..438a7b9b 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -318,6 +318,28 @@ test_expect_equal "$(cat MESSAGES)" "COMPLETE"
 
 add_email_corpus attachment
 
+test_begin_subtest "display of text/calendar"
+test_emacs '(let ((notmuch-show-all-multipart/alternative-parts t))
+	   (notmuch-show "id:YT3PR01MB10572EFC9F7C81F9446214768CEF72@YT3PR01MB10572.CANPRD01.PROD.OUTLOOK.COM"))
+	(test-visible-output "OUTPUT")'
+cat <<EOF > EXPECTED
+David Bremner <bremner@example.com> (1970-01-01) (inbox)
+Subject: test
+To: "david@tethera.net" <david@tethera.net>
+Date: Thu, 01 Jan 1970 00:00:00 +0000
+
+[ multipart/alternative ]
+[ text/plain ]
+This meeting will could have been an email
+[ text/calendar ]
+%%(and (diary-cyclic 1 5 27 2024) (diary-block 5 27 2024 8 27 2024)) 17:00-17:30 test [In-person]
+ Desc: This meeting will could have been an email
+
+
+ Organizer: mailto:bremner@example.com
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "tar not inlined by default"
 test_emacs '(notmuch-show "id:874llc2bkp.fsf@curie.anarc.at")
 	(test-visible-output "OUTPUT")'
-- 
2.43.0

[PATCH 3/3] WIP/test: add tests for rendering text/calendar parts

[David Bremner]

The second test should apparently fail, but it doesn't.
---
 test/T450-emacs-show.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 438a7b9b..85851e70 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -340,6 +340,32 @@ This meeting will could have been an email
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "display of text/calendar, symlinked tmpdir"
+test_subtest_known_broken
+mkdir real-tmp
+ln -s real-tmp tmp
+test_emacs "(let ((notmuch-show-all-multipart/alternative-parts t)
+	   (temporary-file-directory \"tmp\"))
+	   (notmuch-show \"id:YT3PR01MB10572EFC9F7C81F9446214768CEF72@YT3PR01MB10572.CANPRD01.PROD.OUTLOOK.COM\"))
+	(test-visible-output \"OUTPUT\")"
+cat <<EOF > EXPECTED
+David Bremner <bremner@example.com> (1970-01-01) (inbox)
+Subject: test
+To: "david@tethera.net" <david@tethera.net>
+Date: Thu, 01 Jan 1970 00:00:00 +0000
+
+[ multipart/alternative ]
+[ text/plain ]
+This meeting will could have been an email
+[ text/calendar ]
+%%(and (diary-cyclic 1 5 27 2024) (diary-block 5 27 2024 8 27 2024)) 17:00-17:30 test [In-person]
+ Desc: This meeting will could have been an email
+
+
+ Organizer: mailto:bremner@example.com
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "tar not inlined by default"
 test_emacs '(notmuch-show "id:874llc2bkp.fsf@curie.anarc.at")
 	(test-visible-output "OUTPUT")'
-- 
2.43.0

Re: [PATCH 3/3] WIP/test: add tests for rendering text/calendar parts

[Al Haji-Ali]

Yes, that is strange. Based on just looking at the test code, I think this test should indeed fail (I am not able to run in on my machine). In a similar situation, my inline calendar text shows

,----
| !!! Bodypart handler `notmuch-show-insert-part-text/calendar' threw an error:
| !!! Wrong type argument: stringp, nil
| !!! Bodypart handler `notmuch-show-insert-part-*/*' threw an error:
| !!! Wrong type argument: char-or-string-p, gnus-decoded
`----

To see the basic issue, check what this code outputs on your setup:
,----
| (let ((tmp-link  "~/link-tmp"))
|   (make-symbolic-link "/tmp/" tmp-link)
|   (let* ((temporary-file-directory tmp-link)
|          (file (make-temp-file "notmuch-ical")))
|     (find-file-noselect file)
|     (message
|      "Filename: %s
| get-file-buffer: %S
| find-buffer-visiting: %S
| buffer-file-name: %s"
|      file
|      (get-file-buffer file)
|      (find-buffer-visiting file)
|      (buffer-file-name (find-buffer-visiting file)))
|     (kill-buffer (find-buffer-visiting file))
|     (delete-file tmp-link)
|     (delete-file file)))
|     
`----

On my setup (MacOS, Emacs 29.1) this prints something like

,----
| Filename: /Users/al/link-tmp/notmuch-ical7MtHvd
| get-file-buffer: nil
| find-buffer-visiting: #<buffer notmuch-ical7MtHvd>
| buffer-file-name: /private/tmp/notmuch-ical7MtHvd
`----

So `get-file-buffer` returns nil. Reason is that the buffer-file-name has the resolved path (without symlinks). The documentation for `get-file-buffer` says: "The buffer's buffer-file-name must match exactly the expansion of FILENAME". `find-buffer-visiting` gets the truename of the filename before looking for its buffer.

The function `notmuch-show-insert-part-text/calendar` essentially does the same as my code above. Note also that `find-buffer-visiting` is used instead of `get-file-buffer` in `icalendar-import-buffer` for example. 

-- Al

On 26/05/2024, David Bremner wrote:

> The second test should apparently fail, but it doesn't.
> ---
>  test/T450-emacs-show.sh | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
> index 438a7b9b..85851e70 100755
> --- a/test/T450-emacs-show.sh
> +++ b/test/T450-emacs-show.sh
> @@ -340,6 +340,32 @@ This meeting will could have been an email
>  EOF
>  test_expect_equal_file EXPECTED OUTPUT
>  
> +test_begin_subtest "display of text/calendar, symlinked tmpdir"
> +test_subtest_known_broken
> +mkdir real-tmp
> +ln -s real-tmp tmp
> +test_emacs "(let ((notmuch-show-all-multipart/alternative-parts t)
> +	   (temporary-file-directory \"tmp\"))
> +	   (notmuch-show \"id:YT3PR01MB10572EFC9F7C81F9446214768CEF72@YT3PR01MB10572.CANPRD01.PROD.OUTLOOK.COM\"))
> +	(test-visible-output \"OUTPUT\")"
> +cat <<EOF > EXPECTED
> +David Bremner <bremner@example.com> (1970-01-01) (inbox)
> +Subject: test
> +To: "david@tethera.net" <david@tethera.net>
> +Date: Thu, 01 Jan 1970 00:00:00 +0000
> +
> +[ multipart/alternative ]
> +[ text/plain ]
> +This meeting will could have been an email
> +[ text/calendar ]
> +%%(and (diary-cyclic 1 5 27 2024) (diary-block 5 27 2024 8 27 2024)) 17:00-17:30 test [In-person]
> + Desc: This meeting will could have been an email
> +
> +
> + Organizer: mailto:bremner@example.com
> +EOF
> +test_expect_equal_file EXPECTED OUTPUT
> +
>  test_begin_subtest "tar not inlined by default"
>  test_emacs '(notmuch-show "id:874llc2bkp.fsf@curie.anarc.at")
>  	(test-visible-output "OUTPUT")'

Re: [PATCH 3/3] WIP/test: add tests for rendering text/calendar parts

[David Bremner]

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

>
> On my setup (MacOS, Emacs 29.1) this prints something like
>
> ,----
> | Filename: /Users/al/link-tmp/notmuch-ical7MtHvd
> | get-file-buffer: nil
> | find-buffer-visiting: #<buffer notmuch-ical7MtHvd>
> | buffer-file-name: /private/tmp/notmuch-ical7MtHvd
> `----

Here it prints

    Filename: /home/bremner/link-tmp/notmuch-icalDC01xk
    get-file-buffer: #<buffer notmuch-icalDC01xk>
    find-buffer-visiting: #<buffer notmuch-icalDC01xk>
    buffer-file-name: /home/bremner/link-tmp/notmuch-icalDC01xk

So I guess this is some system dependent (or emacs dependent; I have
29.3 here) behaviour. I can convert the test to a regression test after
the change is applied. It's not as nice, but it's better than not
testing it at all.

Re: [PATCH 3/3] WIP/test: add tests for rendering text/calendar parts

[Al Haji-Ali]

Ah, apologies. I should've done more testing.

Turns out I have `find-file-visit-truename` set to non-nil which causes the behaviour I describe.

Best regards,
-- Al

On 28/05/2024, David Bremner wrote:
> Al Haji-Ali <abdo.haji.ali@gmail.com> writes:
>
>>
>> On my setup (MacOS, Emacs 29.1) this prints something like
>>
>> ,----
>> | Filename: /Users/al/link-tmp/notmuch-ical7MtHvd
>> | get-file-buffer: nil
>> | find-buffer-visiting: #<buffer notmuch-ical7MtHvd>
>> | buffer-file-name: /private/tmp/notmuch-ical7MtHvd
>> `----
>
> Here it prints
>
>     Filename: /home/bremner/link-tmp/notmuch-icalDC01xk
>     get-file-buffer: #<buffer notmuch-icalDC01xk>
>     find-buffer-visiting: #<buffer notmuch-icalDC01xk>
>     buffer-file-name: /home/bremner/link-tmp/notmuch-icalDC01xk
>
> So I guess this is some system dependent (or emacs dependent; I have
> 29.3 here) behaviour. I can convert the test to a regression test after
> the change is applied. It's not as nice, but it's better than not
> testing it at all.