[PATCH 2/2] gnu: emacs-helpful: Update to 0.19.

  • Done
  • quality assurance status badge
Details
4 participants
  • Erik Šabi?
  • Liliana Marie Prikler
  • Maxime Devos
  • yarl-baudig
Owner
unassigned
Submitted by
Erik Šabi?
Severity
normal
E
E
Erik Šabi? wrote on 27 May 2022 14:52
(address . guix-patches@gnu.org)
20220527125242.27817-2-erik.sab@gmail.com
* gnu/packages/emacs-xyz.scm (emacs-helpful): Update to 0.19.
---
gnu/packages/emacs-xyz.scm | 39 ++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)

Toggle diff (63 lines)
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index f62c733fb5..6a659fabf3 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -20126,27 +20126,29 @@ (define-public emacs-download-region
(define-public emacs-helpful
(package
(name "emacs-helpful")
- (version "0.18")
- (source
- (origin
- (method git-fetch)
- (uri (git-reference
- (url "https://github.com/Wilfred/helpful")
- (commit version)))
- (file-name (git-file-name name version))
- (sha256
- (base32 "0gdjxykqkal2x765mi51m99i5ql23i1fy909wy4mzj5ajhjfgqcc"))))
+ (version "0.19")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/Wilfred/helpful")
+ (commit version)))
+ (file-name (git-file-name name version))
+ (sha256
+ (base32
+ "0qwsifzsjw95l83m7z07fr9h1sqbhggwmcps1qgbddpan2a8ab8a"))))
(build-system emacs-build-system)
- (propagated-inputs
- (list emacs-elisp-refs emacs-dash emacs-s emacs-f emacs-shut-up))
- (native-inputs
- (list emacs-ert-runner emacs-undercover))
+ (propagated-inputs (list emacs-elisp-refs emacs-dash emacs-s emacs-f
+ emacs-shut-up))
+ (native-inputs (list emacs-ert-runner emacs-undercover))
(arguments
- `(#:tests? #t
- #:test-command '("ert-runner")))
+ ;; Tests are not passing; the author might not be using them anymore.
+ `(#:tests? #f
+ #:test-command
+ '("ert-runner")))
(home-page "https://github.com/Wilfred/helpful")
(synopsis "More contextual information in Emacs help")
- (description "@code{helpful} is an alternative to the built-in Emacs help
+ (description
+ "@code{helpful} is an alternative to the built-in Emacs help
that provides much more contextual information.
@itemize
@@ -20164,7 +20166,8 @@ (define-public emacs-helpful
@item Display any keybindings that apply to interactive functions.
@item Trace, disassemble functions from inside Helpful. This is discoverable
and doesn't require memorisation of commands.
-@end itemize\n")
+@end itemize
+")
(license license:gpl3+)))
(define-public emacs-logview
--
2.36.1
M
M
Maxime Devos wrote on 27 May 2022 15:12
7fa715e77a2f947b2104a3c578c001125842b01f.camel@telenet.be
Erik Šabi? schreef op vr 27-05-2022 om 14:52 [+0200]:
Toggle quote (3 lines)
> +     ;; Tests are not passing; the author might not be using them anymore.
> +     `(#:tests? #f

How do they fail? They might indicate a real problem in the code
(that's what tests are for -- detecting problems before they are
actually encountered).

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYpDOPhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7l+HAQDRNATWSBa4zcXjzvLnEe+1ykpb
fL9N9R4CEH6cRFu8SAEAgwvNvEhxhJ6ac9umyHJUidysR3gYuy2KZA/Yq19hBA8=
=vnE2
-----END PGP SIGNATURE-----


E
E
Erik Šabi? wrote on 27 May 2022 16:58
Broken tests
(address . 55674@debbugs.gnu.org)
CABeAMAmhPrucPUshGYrRHB0UugP206LSe+E_UkvsLBnTgcSTTA@mail.gmail.com
One of the tests fails. Looks like it's testing whether helpful--docstring
and
helpful--docstring-advice figure out that a function is advised.

I've tried using helpful-* functions from this updated package in Emacs,
and it works - advices are shown.

So it looks like the tests are broken.
(but I don't understand the ert tests, maybe someone else
should look at this)
Attachment: file
E
E
Erik Šabi? wrote on 27 May 2022 19:23
Re: [bug#55674] [PATCH 2/2] gnu: emacs-helpful: Update to 0.19.
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55674@debbugs.gnu.org)
CABeAMA=dJ7N8GgReaM1S3iG0ctNSUFJrE3R6HMpBBTYqGZyqGQ@mail.gmail.com
Okay, I figured it out. The test is indeed broken and
it's an easy fix. I will try sending a PR on github.

I have a lot to learn. I apologise for being a nuisance :)

Please tell me if there's anything else I can improve on.

Thank you!

here's a diff for the test:

diff --git a/test/helpful-unit-test.el b/test/helpful-unit-test.el
index a07aa8e..1a61a6f 100644
--- a/test/helpful-unit-test.el
+++ b/test/helpful-unit-test.el
@@ -119,7 +119,7 @@ bar")))
(should
(equal
(helpful--docstring #'test-foo-advised t)
- "Docstring here too.")))
+ "Docstring here too.\n\nThis function has :around advice:
`ad-Advice-test-foo-advised'.")))

(defun test-foo-no-docstring ()
nil)

On Fri, May 27, 2022 at 3:48 PM Maxime Devos <maximedevos@telenet.be> wrote:
Toggle quote (51 lines)
>
> [Please keep 55675@debbugs.gnu.org in CC]
>
> Erik Šabi? schreef op vr 27-05-2022 om 15:32 [+0200]:
> > Well it fails on a test defined like this:
> > (ert-deftest helpful--docstring ()
> > "Basic docstring fetching."
> > (should
> > (equal
> > (helpful--docstring #'test-foo t)
> > "Docstring here.")))
> >
> > And fails like so:
> > ...............s...s..Test helpful--docstring-advice backtrace:
>
>
> >
> > Test helpful--docstring-advice condition:
> >
> > (ert-test-failed
> > ((should
> > (equal
> > (helpful--docstring ... t)
> > "Docstring here too."))
> > :form
> > (equal "Docstring here too.\n\nThis function has :around
> > advice: `ad-Advice-test-foo-advised'." "Docstring here too.")
> > :value nil :explanation
> > (arrays-of-different-length 84 19 "Docstring here too.\n\nThis
> > function has :around advice: `ad-Advice-test-foo-advised'."
> > "Docstring here too." first-mismatch-at 19)))
> >
> > F............................................................s......
>
> Looks like a bug.
>
> > I don't know anything about emacs-ert. And I wasn't able to
> > figure out if it works for the upstream author.
> >
> > WDYT?
>
> I don't know if it fails for the upstream author, but I don't think
> that's relevant to Guix. More relevant I think, is that this bug
> appears in Guix, so it can cause problems for Guix users, so some
> fixing might be necessary.
>
> I don't know much about Emacs though, so you might need to contact
> upstream about the bug.
>
> Greetings,
> Maxime.
Y
Y
yarl-baudig wrote on 29 May 2022 07:05
PR accepted
(address . 55674@debbugs.gnu.org)
ea-mime-6292ff2f-9f6-695a28e3@www-7.mailo.com
Yes, your pull request have been accepted! Thank you. Mind if I update? I would like helpful to be fixed quickly.
Y
Y
yarl-baudig wrote on 29 May 2022 07:13
[PATCH] gnu: emacs-helpful: Update to 209971b
(address . 55674@debbugs.gnu.org)
ea-mime-629300ff-ca9-8667229@www-7.mailo.com
* gnu/packages/emacs-xyz.scm (emacs-helpful): Update to 209971b

Thanks to upstream pull request by Erik Šabi?
---
gnu/packages/emacs-xyz.scm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Toggle diff (24 lines)
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 31822dc641..90fd8a49d6 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -20131,7 +20131,7 @@ (define-public emacs-download-region
(define-public emacs-helpful
(package
(name "emacs-helpful")
- (version "0.18")
+ (version "209971ba9f576ba080352642cfbf25df5692b1d7")
(source
(origin
(method git-fetch)
@@ -20140,7 +20140,7 @@ (define-public emacs-helpful
(commit version)))
(file-name (git-file-name name version))
(sha256
- (base32 "0gdjxykqkal2x765mi51m99i5ql23i1fy909wy4mzj5ajhjfgqcc"))))
+ (base32 "1qrxr0ybvdvx6qw6akb1pv074bmffv4m57855kk8fr1vp4w07z0i"))))
(build-system emacs-build-system)
(propagated-inputs
(list emacs-elisp-refs emacs-dash emacs-s emacs-f emacs-shut-up))
--
2.36.0
L
L
Liliana Marie Prikler wrote on 29 May 2022 09:41
2f5f0d333b8a15a52ba25468cc3d3402229083c2.camel@gmail.com
Am Sonntag, dem 29.05.2022 um 07:13 +0200 schrieb
yarl-baudig@mailoo.org:
Toggle quote (18 lines)
> * gnu/packages/emacs-xyz.scm (emacs-helpful): Update to 209971b
>
> Fixes <https://issues.guix.gnu.org/55674>
> Thanks to upstream pull request by Erik Šabi?
> ---
>  gnu/packages/emacs-xyz.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index 31822dc641..90fd8a49d6 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -20131,7 +20131,7 @@ (define-public emacs-download-region
>  (define-public emacs-helpful
>    (package
>      (name "emacs-helpful")
> -    (version "0.18")
> +    (version "209971ba9f576ba080352642cfbf25df5692b1d7")
Commit hashes are not versions. Furthermore, there has been more than
just that bugfix since – as a workaround I think it'd be more
reasonable to just disable that one test until the next release.

Cheers
Y
Y
yarl-baudig wrote on 29 May 2022 11:09
ea-mime-62933838-600-37890f41@www-7.mailo.com
Toggle quote (4 lines)
> Commit hashes are not versions. Furthermore, there has been more than
> just that bugfix since – as a workaround I think it'd be more
> reasonable to just disable that one test until the next release.

Or Erik Šabi? might try its patch against version 0.19 then add it to the package definition and let a comment that as soon as it is updated to 0.20, the patch can go away?
L
L
Liliana Marie Prikler wrote on 29 May 2022 12:12
37d8fb49b31dd974ffc7c02689ddee2e3b9d98f9.camel@gmail.com
Am Sonntag, dem 29.05.2022 um 11:09 +0200 schrieb
yarl-baudig@mailoo.org:
Toggle quote (7 lines)
> > Commit hashes are not versions.  Furthermore, there has been more
> > than just that bugfix since – as a workaround I think it'd be more
> > reasonable to just disable that one test until the next release.
>
> Or Erik Šabi? might try [their] patch against version 0.19 then add
> it to the package definition and let a comment that as soon as it is
> updated to 0.20, the patch can go away?
That'd work too, but it'd be a slightly larger patch to create :)
Y
Y
yarl-baudig wrote on 30 May 2022 09:20
[PATCH] gnu: emacs-helpful: Update to 0.19
ea-mime-62947038-241f-6fb4c704@www-8.mailo.com
Includes fixe for test from Erik Šabi? against upstream,
see details in the patch file.

* gnu/packages/patches/emacs-helpful-fix-0.19-test.patch: New file.
* gnu/packages/emacs-xyz.scm: Update emacs-helpful to 0.19, use
the patch.
* gnu/local.mk: Update dist_patch_DATA.
---
gnu/local.mk | 1 +
gnu/packages/emacs-xyz.scm | 7 ++++--
.../patches/emacs-helpful-fix-0.19-test.patch | 23 +++++++++++++++++++
3 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/emacs-helpful-fix-0.19-test.patch

Toggle diff (68 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index bc82c5ba9f..71f6b7669a 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1029,6 +1029,7 @@ dist_patch_DATA = \
%D%/packages/patches/emacs-git-email-missing-parens.patch \
%D%/packages/patches/emacs-fix-scheme-indent-function.patch \
%D%/packages/patches/emacs-json-reformat-fix-tests.patch \
+ %D%/packages/patches/emacs-helpful-fix-0.19-test.patch \
%D%/packages/patches/emacs-highlight-stages-add-gexp.patch \
%D%/packages/patches/emacs-hyperbole-toggle-messaging.patch \
%D%/packages/patches/emacs-libgit-use-system-libgit2.patch \
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 31822dc641..c5fa725bf5 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -20131,7 +20131,7 @@ (define-public emacs-download-region
(define-public emacs-helpful
(package
(name "emacs-helpful")
- (version "0.18")
+ (version "0.19")
(source
(origin
(method git-fetch)
@@ -20140,7 +20140,10 @@ (define-public emacs-helpful
(commit version)))
(file-name (git-file-name name version))
(sha256
- (base32 "0gdjxykqkal2x765mi51m99i5ql23i1fy909wy4mzj5ajhjfgqcc"))))
+ (base32 "0qwsifzsjw95l83m7z07fr9h1sqbhggwmcps1qgbddpan2a8ab8a"))
+ ;; TODO: remove this patch as soon as the package is updated to 0.20
+ ;; See explanation in the patch file
+ (patches (search-patches "emacs-helpful-fix-0.19-test.patch"))))
(build-system emacs-build-system)
(propagated-inputs
(list emacs-elisp-refs emacs-dash emacs-s emacs-f emacs-shut-up))
diff --git a/gnu/packages/patches/emacs-helpful-fix-0.19-test.patch b/gnu/packages/patches/emacs-helpful-fix-0.19-test.patch
new file mode 100644
index 0000000000..64000f2b73
--- /dev/null
+++ b/gnu/packages/patches/emacs-helpful-fix-0.19-test.patch
@@ -0,0 +1,23 @@
+Erik Šabi? sent a pull request (patches 741b864 and 209971b) against 2f91e79,
+which is not a release.
+This is a diff between 0.19 and those 2 patches applied against 0.19.
+
+So, when 0.20 will be released, this patch can be removed.
+
+The procedure should be as simple as reverting the commit including
+this file then calling `guix refresh emacs-helpful`.
+diff --git a/test/helpful-unit-test.el b/test/helpful-unit-test.el
+index a07aa8e..8a95129 100644
+--- a/test/helpful-unit-test.el
++++ b/test/helpful-unit-test.el
+@@ -119,7 +119,9 @@ bar")))
+ (should
+ (equal
+ (helpful--docstring #'test-foo-advised t)
+- "Docstring here too.")))
++ (if (version< emacs-version "28")
++ "Docstring here too."
++ "Docstring here too.\n\nThis function has :around advice: `ad-Advice-test-foo-advised'."))))
+
+ (defun test-foo-no-docstring ()
+ nil)
--
2.36.0
L
L
Liliana Marie Prikler wrote on 30 May 2022 21:41
ee762fce0f5e4ad2c63e8deb84b002bda208a6f3.camel@gmail.com
Am Montag, dem 30.05.2022 um 09:20 +0200 schrieb
yarl-baudig@mailoo.org:
Toggle quote (7 lines)
> Includes fixe for test from Erik Šabi? against upstream,
> see details in the patch file.
>
> * gnu/packages/patches/emacs-helpful-fix-0.19-test.patch: New file.
> * gnu/packages/emacs-xyz.scm: Update emacs-helpful to 0.19, use
> the patch.
> * gnu/local.mk: Update dist_patch_DATA.
I locally applied your changes and would be ready to push, but noticed
your mail client is not really set up well ;)

For the sake of attribution, should I set "Yarl Baudig" or something
else as committer name?
Y
Y
yarl baudig wrote on 30 May 2022 23:11
ea-mime-6295331f-3928-481b85ae@www-8.mailo.com
Toggle quote (6 lines)
> your mail client is not really set up well ;)
>
> For the sake of attribution, should I set "Yarl Baudig" or something
> else as committer name?
>

Yes, please. I will try to fix this.
Thank you!
L
L
Liliana Marie Prikler wrote on 4 Jun 2022 09:00
89a9bf5beee51afb4edb0a3ed39f5904421a9872.camel@gmail.com
Am Montag, dem 30.05.2022 um 23:11 +0200 schrieb yarl baudig:
Toggle quote (9 lines)
> > your mail client is not really set up well ;)
> >
> > For the sake of attribution, should I set "Yarl Baudig" or
> > something
> > else as committer name?
> >
>
> Yes, please. I will try to fix this.
> Thank you!
Used "yarl baudig" as per this mail.
Closed
?