GNU bug report logs - #35018
26.1; Use diff as en ert-explainer for string=

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35018 in the body.
You can then email your comments to 35018 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <mail <at> ambrevar.xyz>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; Use diff as en ert-explainer for string=
Date: Wed, 27 Mar 2019 11:19:07 +0100

[Message part 1 (text/plain, inline)]

I've just committed webfeeder.el to ELPA.  In his review, Stefan Monnier
suggested we merged the following code snippet upstream:

--8<---------------cut here---------------start------------->8---
(defun webfeeder--string=-explainer (string-a string-b)
  "Return the diff output of STRING-A and STRING-B"
  (unless (string= string-a string-b)
    (let (file-a file-b)
      (unwind-protect
          (let (result)
            (setq file-a (make-temp-file "webfeeder")
                  file-b (make-temp-file "webfeeder"))
            (with-temp-file file-a
              (insert string-a))
            (with-temp-file file-b
              (insert string-b))
            (setq result
                  (with-temp-buffer
                    ;; The following generates a *Diff* buffer which is
                    ;; convenient for coloration.
                    (diff file-a file-b nil 'no-async)
                    (diff-no-select file-a file-b nil 'no-async (current-buffer))
                    (buffer-string)))
            result)
        (delete-file file-a)
        (delete-file file-b)))))

;; FIXME: Add this to ERT!
(put 'string= 'ert-explainer #'webfeeder--string=-explainer)
--8<---------------cut here---------------end--------------->8---

I've used this feature extensively in my ERT tests: it displays a "diff"
of string A and string B, which is a pretty nice default when the
strings are longer than a single line.

Food for thoughts :)

[Message part 2 (text/plain, inline)]

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Pierre Neidhardt <mail <at> ambrevar.xyz>
Cc: 35018 <at> debbugs.gnu.org
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Mon, 01 Apr 2019 21:05:01 -0400

severity 35018 wishlist
quit

Pierre Neidhardt <mail <at> ambrevar.xyz> writes:

> I've just committed webfeeder.el to ELPA.  In his review, Stefan Monnier
> suggested we merged the following code snippet upstream:
>
> (defun webfeeder--string=-explainer (string-a string-b)
>   "Return the diff output of STRING-A and STRING-B"
>   (unless (string= string-a string-b)

I guess a diff won't help so much for single line strings, so maybe the
condition should check for that? e.g.

    (or (string= string-a string-b)
        (not (string-match-p "\n" string-a))
        (not (string-match-p "\n" string-b))

>     (let (file-a file-b)
>       (unwind-protect
>           (let (result)
>             (setq file-a (make-temp-file "webfeeder")
>                   file-b (make-temp-file "webfeeder"))
>             (with-temp-file file-a
>               (insert string-a))
>             (with-temp-file file-b
>               (insert string-b))
>             (setq result
>                   (with-temp-buffer
>                     ;; The following generates a *Diff* buffer which is
>                     ;; convenient for coloration.
>                     (diff file-a file-b nil 'no-async)
>                     (diff-no-select file-a file-b nil 'no-async (current-buffer))

Isn't the diff-no-select redudant, since diff already calls it?

>                     (buffer-string)))
>             result)

You don't need the 'result' variable here, just make the
(with-temp-buffer...) the last expression in the let (which could then
be converted to progn).

>         (delete-file file-a)
>         (delete-file file-b)))))

You should check that each of file-a and file-b are non-nil before
trying to delete (in case either make-temp-file signals an error).

Message #13 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <mail <at> ambrevar.xyz>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35018 <at> debbugs.gnu.org
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Tue, 02 Apr 2019 09:59:59 +0200

[Message part 1 (text/plain, inline)]

Thanks for the review!

Noam Postavsky <npostavs <at> gmail.com> writes:

>> (defun webfeeder--string=-explainer (string-a string-b)
>>   "Return the diff output of STRING-A and STRING-B"
>>   (unless (string= string-a string-b)
>
> I guess a diff won't help so much for single line strings, so maybe the
> condition should check for that? e.g.
>
>     (or (string= string-a string-b)
>         (not (string-match-p "\n" string-a))
>         (not (string-match-p "\n" string-b))

Yes, this is very nice!

>>     (let (file-a file-b)
>>       (unwind-protect
>>           (let (result)
>>             (setq file-a (make-temp-file "webfeeder")
>>                   file-b (make-temp-file "webfeeder"))
>>             (with-temp-file file-a
>>               (insert string-a))
>>             (with-temp-file file-b
>>               (insert string-b))
>>             (setq result
>>                   (with-temp-buffer
>>                     ;; The following generates a *Diff* buffer which is
>>                     ;; convenient for coloration.
>>                     (diff file-a file-b nil 'no-async)
>>                     (diff-no-select file-a file-b nil 'no-async (current-buffer))
>
> Isn't the diff-no-select redudant, since diff already calls it?

The first diff is a typo.  It should be diff-no-select only.

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[signature.asc (application/pgp-signature, inline)]

Message #16 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pierre Neidhardt <mail <at> ambrevar.xyz>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Wed, 23 Jun 2021 15:42:11 +0200

Pierre Neidhardt <mail <at> ambrevar.xyz> writes:

> I've just committed webfeeder.el to ELPA.  In his review, Stefan Monnier
> suggested we merged the following code snippet upstream:

I've adapted this to ert to see how it would look, and...  I'm not sure
it's all that helpful as is.  The following test:

(ert-deftest test-test-test ()
  (should (string= "foo\nbar" "zot\nbar")))

then gives this output:

    (ert-test-failed
     ((should
       (string= "foo\12bar" "zot\12bar"))
      :form
      (string= "foo\12bar" "zot\12bar")
      :value nil :explanation "diff -u /tmp/webfeeder1kS4io /tmp/webfeederwYoKdx\12--- /tmp/webfeeder1kS4io\0112021-06-23 15:37:54.407053381 +0200\12+++ /tmp/webfeederwYoKdx\0112021-06-23 15:37:54.407053381 +0200\12@@ -1,2 +1,2 @@\12-foo\12+zot\12 bar\12\\ No newline at end of file\12\12Diff finished.  Wed Jun 23 15:37:54 2021\12"))

Which isn't all that nice.

But I guess we could massage the output.  Removing the header and
trailer, we get:

    (ert-test-failed
     ((should
       (string= "foo\12bar" "zot\12bar"))
      :form
      (string= "foo\12bar" "zot\12bar")
      :value nil :explanation "@@ -1,2 +1,2 @@\12-foo\12+zot\12 bar\12\\ "))

But...  that's still not exactly readable.

So I don't know.  Anybody got an opinion here, or an easy fix that'll
make this readable in this context?

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 6793b374ee..004bc81b4a 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -63,6 +63,7 @@
 (require 'ewoc)
 (require 'find-func)
 (require 'pp)
+(require 'diff)
 
 ;;; UI customization options.
 
@@ -535,6 +536,34 @@ ert--explain-equal
     (ert--explain-equal-rec a b)))
 (put 'equal 'ert-explainer 'ert--explain-equal)
 
+(defun ert--explain-string (string-a string-b)
+  "Return the diff output of STRING-A and STRING-B"
+  (unless (or (string= string-a string-b)
+              ;; Only do diffs if there are newlines.
+              (not (string-match-p "\n" string-a))
+              (not (string-match-p "\n" string-b)))
+    (let (file-a file-b)
+      (unwind-protect
+          (let (result)
+            (setq file-a (make-temp-file "ert")
+                  file-b (make-temp-file "ert"))
+            (with-temp-file file-a
+              (insert string-a))
+            (with-temp-file file-b
+              (insert string-b))
+            (setq result
+                  (with-temp-buffer
+                    ;; The following generates a *Diff* buffer which is
+                    ;; convenient for coloration.
+                    (diff-no-select file-a file-b nil 'no-async)
+                    (diff-no-select file-a file-b nil 'no-async
+                                    (current-buffer))
+                    (buffer-string)))
+            result)
+        (delete-file file-a)
+        (delete-file file-b)))))
+(put 'string= 'ert-explainer #'ert--explain-string)
+
 (defun ert--significant-plist-keys (plist)
   "Return the keys of PLIST that have non-null values, in order."
   (cl-assert (zerop (mod (length plist) 2)) t)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #21 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <mail <at> ambrevar.xyz>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Thu, 24 Jun 2021 09:06:24 +0200

[Message part 1 (text/plain, inline)]

Hi Lars,

thanks for giving this a shot!

To clarify, the original intent was to display the output in a diff-mode
buffer, which would make it readable I believe.

Then no need to parse the output of the diff command :)

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[signature.asc (application/pgp-signature, inline)]

Message #24 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pierre Neidhardt <mail <at> ambrevar.xyz>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Thu, 24 Jun 2021 16:52:35 +0200

Pierre Neidhardt <mail <at> ambrevar.xyz> writes:

> To clarify, the original intent was to display the output in a diff-mode
> buffer, which would make it readable I believe.

Oh, I see...  So it shouldn't be displayed in batch runs at all?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #27 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <mail <at> ambrevar.xyz>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Thu, 24 Jun 2021 17:59:32 +0200

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Oh, I see...  So it shouldn't be displayed in batch runs at all?  

The original implementation was not meant with batch mode in mind, but
we could make it more readable as you suggested.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pierre Neidhardt <mail <at> ambrevar.xyz>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Tue, 12 Oct 2021 17:33:32 +0200

Pierre Neidhardt <mail <at> ambrevar.xyz> writes:

> The original implementation was not meant with batch mode in mind, but
> we could make it more readable as you suggested.

It's still not clear to me how this is supposed to be used.  Do you have
a more complete implementation, or a description of how this is to be
used in a workflow?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #35 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <mail <at> ambrevar.xyz>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Tue, 12 Oct 2021 19:23:42 +0200

[Message part 1 (text/plain, inline)]

Sorry, I don't have any other implementation and I haven't used it in
years, so I'm afraid I can't be of much help here.

Pierre

[signature.asc (application/pgp-signature, inline)]

Message #38 received at 35018 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pierre Neidhardt <mail <at> ambrevar.xyz>
Cc: 35018 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#35018: 26.1; Use diff as en ert-explainer for string=
Date: Wed, 13 Oct 2021 13:21:45 +0200

Pierre Neidhardt <mail <at> ambrevar.xyz> writes:

> Sorry, I don't have any other implementation and I haven't used it in
> years, so I'm afraid I can't be of much help here.

OK -- it sounds like it's unlikely to be any further progress here in
this bug report, then, and I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.