GNU bug report logs - #46351
28.0.50; Add convenient way to bypass Eshell's own pipelining

Previous Next

Full log

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 16:52:00 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

> I've got it working.  Please let me know what you think of the attached.

Some comments:

> --- /dev/null
> +++ b/test/lisp/eshell/em-extpipe-tests.el
> @@ -0,0 +1,122 @@
> +(load (expand-file-name "eshell-tests"
> +                        (file-name-directory (or load-file-name
> +                                                 default-directory))))

This is problematic. Loading eshell-tests.el declares also all ert tests
which are contained in that file. Running em-extpipe-tests.el in batch
would run also all tests from that file, which is not intended I believe.

A better approach would be to factor out the helper functions from
eshell-tests.el into an own file, and load it here and in eshell-tests.el.

> +(cl-macrolet
> +    ((deftest (name input expected)
> +       (let ((result (gensym)))
> +         `(ert-deftest ,name ()
> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")

I'm not sure this is the right approach. Why do you change
shell-file-name  and shell-command-switch? You've spoken in another
message about Tramp, but I don't understand this. Tramp has its own
machinery to handle them, via connection-local variables.

> +  (deftest em-extpipe-test-7

Looks like em-extpipe-test-6 is missing.

> Sean Whitton

Best regards, Michael.

Previous Next

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