GNU bug report logs - #79177
31.0.50; [PATCH] [v4] Add tests to ispell.el

Previous Next

To reply to this bug, email your comments to 79177 AT debbugs.gnu.org.

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

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Tue, 05 Aug 2025 20:47:57 +0800

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

Continuing the discussion from emacs-devel:
https://lists.gnu.org/archive/html/emacs-devel/2025-07/msg00514.html

Attached is the patch which adds 36 tests for ispell.el. The following
symbols are tested:

0. ispell-program-name
1. ispell-with-safe-default-directory
2. ispell-call-process
3. ispell-create-debug-buffer
4. ispell-valid-dictionary-list
5. ispell-add-per-file-word-list
6. ispell-buffer-local-words
7. ispell-init-process
8. ispell-buffer-local-dict


-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

[0001-ispell.el-Add-36-tests.patch (text/x-patch, attachment)]

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: 79177 <at> debbugs.gnu.org
Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: Acknowledgement (31.0.50; [PATCH] [v4] Add tests to
 ispell.el)
Date: Thu, 07 Aug 2025 15:01:59 +0800

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

Updated version of the patch attached:

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

[0001-ispell.el-Add-39-tests.patch (text/x-patch, attachment)]

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

-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Thu, 07 Aug 2025 10:36:04 +0300

> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>
> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Date: Thu, 07 Aug 2025 15:01:59 +0800
> 
> Updated version of the patch attached:

Thanks, comments below.

> >From a5a489689f5361c4a86749ac3c9b33e83b23e528 Mon Sep 17 00:00:00 2001
> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Date: Thu, 24 Jul 2025 21:07:10 +0800
> Subject: [PATCH] ispell.el: Add 39 tests.
> 
> * test/lisp/textmodes/ispell-tests/*: Add 36 tests for basic utils
>   used in ispell.el.
>    ispell-program-name
>    ispell-with-safe-default-directory
>    ispell-call-process
>    ispell-create-debug-buffer
>    ispell-valid-dictionary-list
>    ispell-add-per-file-word-list
>    ispell-buffer-local-words
>    ispell-init-process
>    ispell-buffer-local-dict
> * test/lisp/textmodes/ispell-resources/fake-aspell: Add a mock
>   `aspell' for use in ispell.el test.

Please mention the bug number in the commit log message.

> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> new file mode 100755
> index 00000000000..4406a18a22e
> --- /dev/null
> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> @@ -0,0 +1,2 @@
> +#!/bin/bash
> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"

I asked to use Emacs instead of a Bash script?  If that causes
problems, let's discuss that, okay?

> +   (with-temp-buffer
> +     (let ((default-directory "86e44985-cfba-43ba-98dc-73be46addbc2"))
> +       (ispell-call-process "emacs" nil t nil '("--batch" "-q" "-nw" "--eval" "(progn (message default-directory) (kill-emacs))"))

There's no need for "-nw", "--batch" implies it.  Also, I suggest to
use "-Q" instead of "-q", so that site-init file is not read.

Same comment in the other places you invoke Emacs as a subordinate
process.

> +          (setopt ispell-library-directory "/tmp")

Please use temporary-file-directory, not a literal "/tmp" (which is
not portable), here and elsewhere.

> +    (let ((test-dict "nonexistent")
> +          (test-pdict "/tmp/lnonexistent.txt"))
                         ^^^^^^^^^^^^^^^^^^^^^^^
Likewise here: please use

   (expand-file-name "lnonexistent.txt" temporary-file-directory)

instead.  (There are several other places in the patch with the same
problem.)

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79177 <at> debbugs.gnu.org, Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Thu, 07 Aug 2025 16:07:54 +0800

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> Date: Thu, 07 Aug 2025 15:01:59 +0800
>> 
>> Updated version of the patch attached:
>
> Thanks, comments below.
>
>> >From a5a489689f5361c4a86749ac3c9b33e83b23e528 Mon Sep 17 00:00:00 2001
>> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> Date: Thu, 24 Jul 2025 21:07:10 +0800
>> Subject: [PATCH] ispell.el: Add 39 tests.
>> 
>> * test/lisp/textmodes/ispell-tests/*: Add 36 tests for basic utils
>>   used in ispell.el.
>>    ispell-program-name
>>    ispell-with-safe-default-directory
>>    ispell-call-process
>>    ispell-create-debug-buffer
>>    ispell-valid-dictionary-list
>>    ispell-add-per-file-word-list
>>    ispell-buffer-local-words
>>    ispell-init-process
>>    ispell-buffer-local-dict
>> * test/lisp/textmodes/ispell-resources/fake-aspell: Add a mock
>>   `aspell' for use in ispell.el test.
>
> Please mention the bug number in the commit log message.
>
>> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> new file mode 100755
>> index 00000000000..4406a18a22e
>> --- /dev/null
>> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> @@ -0,0 +1,2 @@
>> +#!/bin/bash
>> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>
> I asked to use Emacs instead of a Bash script?  If that causes
> problems, let's discuss that, okay?

Aspell needs to parse the "-vv" command-line argument, and Emacs
launched in batch mode cannot do that.  Or I did not manage to make it.

I am not intentionally ignoring your suggestions, some of my responses
got lost while moving from emacs-devel to the bug tracker.

>
>> +   (with-temp-buffer
>> +     (let ((default-directory "86e44985-cfba-43ba-98dc-73be46addbc2"))
>> +       (ispell-call-process "emacs" nil t nil '("--batch" "-q" "-nw" "--eval" "(progn (message default-directory) (kill-emacs))"))
>
> There's no need for "-nw", "--batch" implies it.  Also, I suggest to
> use "-Q" instead of "-q", so that site-init file is not read.
>
> Same comment in the other places you invoke Emacs as a subordinate
> process.

Fixed.

>> +          (setopt ispell-library-directory "/tmp")
>
> Please use temporary-file-directory, not a literal "/tmp" (which is
> not portable), here and elsewhere.

Fixed.

>> +    (let ((test-dict "nonexistent")
>> +          (test-pdict "/tmp/lnonexistent.txt"))
>                          ^^^^^^^^^^^^^^^^^^^^^^^
> Likewise here: please use
>
>    (expand-file-name "lnonexistent.txt" temporary-file-directory)
>
> instead.  (There are several other places in the patch with the same
> problem.)

This is not a file path.  This is just a string which looks like a file
path.  If something tries to access this as if it is a path during the
execution of the test, this is an error.

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

[0001-ispell.el-Add-40-tests.-Fix-79177.patch (text/x-patch, attachment)]

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

-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Thu, 07 Aug 2025 12:09:30 +0300

> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
> Date: Thu, 07 Aug 2025 16:07:54 +0800
> 
> >> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> >> new file mode 100755
> >> index 00000000000..4406a18a22e
> >> --- /dev/null
> >> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> >> @@ -0,0 +1,2 @@
> >> +#!/bin/bash
> >> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
> >
> > I asked to use Emacs instead of a Bash script?  If that causes
> > problems, let's discuss that, okay?
> 
> Aspell needs to parse the "-vv" command-line argument, and Emacs
> launched in batch mode cannot do that.  Or I did not manage to make it.

Sorry, I don't understand.  The Bash script just outputs a fixed
string, and Emacs is capable of doing that.  What did you try, and if
it didn''t work, how it failed to work?

> I am not intentionally ignoring your suggestions, some of my responses
> got lost while moving from emacs-devel to the bug tracker.

Sorry, I didn't see any explanations, only the new patch, and I
responded to what I saw.

> >> +    (let ((test-dict "nonexistent")
> >> +          (test-pdict "/tmp/lnonexistent.txt"))
> >                          ^^^^^^^^^^^^^^^^^^^^^^^
> > Likewise here: please use
> >
> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
> >
> > instead.  (There are several other places in the patch with the same
> > problem.)
> 
> This is not a file path.  This is just a string which looks like a file
> path.  If something tries to access this as if it is a path during the
> execution of the test, this is an error.

OK, but the file name you use is not an absolute file name on Windows,
so it could cause problems.  Is there a problem with using my
suggestion, even though you don't need a real file name?

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79177 <at> debbugs.gnu.org, Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Thu, 07 Aug 2025 17:59:06 +0800

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
>> Date: Thu, 07 Aug 2025 16:07:54 +0800
>> 
>> >> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> >> new file mode 100755
>> >> index 00000000000..4406a18a22e
>> >> --- /dev/null
>> >> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> >> @@ -0,0 +1,2 @@
>> >> +#!/bin/bash
>> >> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>> >
>> > I asked to use Emacs instead of a Bash script?  If that causes
>> > problems, let's discuss that, okay?
>> 
>> Aspell needs to parse the "-vv" command-line argument, and Emacs
>> launched in batch mode cannot do that.  Or I did not manage to make it.
>
> Sorry, I don't understand.  The Bash script just outputs a fixed
> string, and Emacs is capable of doing that.  What did you try, and if
> it didn''t work, how it failed to work?

No, it does not /just/ output a fixed string. It also _ignores_ the
command-line argument "-vv", which, if passed to emacs --batch, produces
and error Error: error ("Unknown option ‘-vv’"), and ispell.el does it
when initialising the backend.


>> >> +    (let ((test-dict "nonexistent")
>> >> +          (test-pdict "/tmp/lnonexistent.txt"))
>> >                          ^^^^^^^^^^^^^^^^^^^^^^^
>> > Likewise here: please use
>> >
>> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
>> >
>> > instead.  (There are several other places in the patch with the same
>> > problem.)
>> 
>> This is not a file path.  This is just a string which looks like a file
>> path.  If something tries to access this as if it is a path during the
>> execution of the test, this is an error.
>
> OK, but the file name you use is not an absolute file name on Windows,
> so it could cause problems.  Is there a problem with using my
> suggestion, even though you don't need a real file name?

ispell's man page says the following:

```
 The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
```

I do not want ispell to do any mangling of the "HOME" directory if this
"string which looks like a path" is by chance (if a test malfunctions)
passed to ispell binary and it creates a bogus file in the home
directory or overwrites something.
On windows "temporary-file-directory" probably does not begin with "/".


-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Thu, 07 Aug 2025 13:59:17 +0300

> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
> Date: Thu, 07 Aug 2025 17:59:06 +0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> >> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
> >> Date: Thu, 07 Aug 2025 16:07:54 +0800
> >> 
> >> >> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> >> >> new file mode 100755
> >> >> index 00000000000..4406a18a22e
> >> >> --- /dev/null
> >> >> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
> >> >> @@ -0,0 +1,2 @@
> >> >> +#!/bin/bash
> >> >> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
> >> >
> >> > I asked to use Emacs instead of a Bash script?  If that causes
> >> > problems, let's discuss that, okay?
> >> 
> >> Aspell needs to parse the "-vv" command-line argument, and Emacs
> >> launched in batch mode cannot do that.  Or I did not manage to make it.
> >
> > Sorry, I don't understand.  The Bash script just outputs a fixed
> > string, and Emacs is capable of doing that.  What did you try, and if
> > it didn''t work, how it failed to work?
> 
> No, it does not /just/ output a fixed string. It also _ignores_ the
> command-line argument "-vv", which, if passed to emacs --batch, produces
> and error Error: error ("Unknown option ‘-vv’"), and ispell.el does it
> when initialising the backend.

I feel there's some misunderstanding here.  What I suggested is that
instead of doing this:

  (let ((fake-aspell-path (expand-file-name
                           "./fake-aspell.bash"
                           tests-ispell-data-directory)))
    (chmod fake-aspell-path 504)
    (call-process fake-aspell-path nil nil nil)

where fake-aspell.bash does

  printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"

the code does this (not tested):

  (call-process "emacs" nil nil nil "-Q" "--batch" "--eval" "(princ \\"@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)\\")")

For best results, use

  (expand invocation-name invocation-directory)

instead of a literal "emacs".  This makes sure you invoke the same
Emacs as the one running the test, not some other version.

> >> >> +    (let ((test-dict "nonexistent")
> >> >> +          (test-pdict "/tmp/lnonexistent.txt"))
> >> >                          ^^^^^^^^^^^^^^^^^^^^^^^
> >> > Likewise here: please use
> >> >
> >> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
> >> >
> >> > instead.  (There are several other places in the patch with the same
> >> > problem.)
> >> 
> >> This is not a file path.  This is just a string which looks like a file
> >> path.  If something tries to access this as if it is a path during the
> >> execution of the test, this is an error.
> >
> > OK, but the file name you use is not an absolute file name on Windows,
> > so it could cause problems.  Is there a problem with using my
> > suggestion, even though you don't need a real file name?
> 
> ispell's man page says the following:
> 
> ```
>  The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
> ```
> 
> I do not want ispell to do any mangling of the "HOME" directory if this
> "string which looks like a path" is by chance (if a test malfunctions)
> passed to ispell binary and it creates a bogus file in the home
> directory or overwrites something.
> On windows "temporary-file-directory" probably does not begin with "/".

Yes, but a Windows port of ispell will know that, and detect absolute
file names as appropriate: by looking at the drive letter, as in
"d:/foo".  On the contrary, using file names that begin with a slash
on Windows is likely to fail the test mentioned by the man page.

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79177 <at> debbugs.gnu.org, Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Fri, 08 Aug 2025 10:21:37 +0800

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
>> Date: Thu, 07 Aug 2025 17:59:06 +0800
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> >> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
>> >> Date: Thu, 07 Aug 2025 16:07:54 +0800
>> >> 
>> >> >> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> >> >> new file mode 100755
>> >> >> index 00000000000..4406a18a22e
>> >> >> --- /dev/null
>> >> >> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> >> >> @@ -0,0 +1,2 @@
>> >> >> +#!/bin/bash
>> >> >> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>> >> >
>> >> > I asked to use Emacs instead of a Bash script?  If that causes
>> >> > problems, let's discuss that, okay?
>> >> 
>> >> Aspell needs to parse the "-vv" command-line argument, and Emacs
>> >> launched in batch mode cannot do that.  Or I did not manage to make it.
>> >
>> > Sorry, I don't understand.  The Bash script just outputs a fixed
>> > string, and Emacs is capable of doing that.  What did you try, and if
>> > it didn''t work, how it failed to work?
>> 
>> No, it does not /just/ output a fixed string. It also _ignores_ the
>> command-line argument "-vv", which, if passed to emacs --batch, produces
>> and error Error: error ("Unknown option ‘-vv’"), and ispell.el does it
>> when initialising the backend.
>
> I feel there's some misunderstanding here.  What I suggested is that
> instead of doing this:
>
>   (let ((fake-aspell-path (expand-file-name
>                            "./fake-aspell.bash"
>                            tests-ispell-data-directory)))
>     (chmod fake-aspell-path 504)
>     (call-process fake-aspell-path nil nil nil)
>
> where fake-aspell.bash does
>
>   printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>
> the code does this (not tested):
>
>   (call-process "emacs" nil nil nil "-Q" "--batch" "--eval" "(princ \\"@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)\\")")
>
> For best results, use
>
>   (expand invocation-name invocation-directory)
>
> instead of a literal "emacs".  This makes sure you invoke the same
> Emacs as the one running the test, not some other version.

I am not calling "fake_aspell.bash" with "(call-process fake-aspell-path
nil nil nil)" in the name of printing a string.  I am calling it in
order to make sure that calling it works on the target system.  Read the
test further on, the important lines of the test are
```
(setopt ispell-program-name fake-aspell-path)
(ispell-check-version t)
```
where ispell.el will call "fake_aspell.bash" in the way it likes calling
backend executables.

Maybe I should wrap it in (skip-unless), but I remember it behaving
strangely within a "let".



>> >> >> +    (let ((test-dict "nonexistent")
>> >> >> +          (test-pdict "/tmp/lnonexistent.txt"))
>> >> >                          ^^^^^^^^^^^^^^^^^^^^^^^
>> >> > Likewise here: please use
>> >> >
>> >> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
>> >> >
>> >> > instead.  (There are several other places in the patch with the same
>> >> > problem.)
>> >> 
>> >> This is not a file path.  This is just a string which looks like a file
>> >> path.  If something tries to access this as if it is a path during the
>> >> execution of the test, this is an error.
>> >
>> > OK, but the file name you use is not an absolute file name on Windows,
>> > so it could cause problems.  Is there a problem with using my
>> > suggestion, even though you don't need a real file name?
>> 
>> ispell's man page says the following:
>> 
>> ```
>>  The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
>> ```
>> 
>> I do not want ispell to do any mangling of the "HOME" directory if this
>> "string which looks like a path" is by chance (if a test malfunctions)
>> passed to ispell binary and it creates a bogus file in the home
>> directory or overwrites something.
>> On windows "temporary-file-directory" probably does not begin with "/".
>
> Yes, but a Windows port of ispell will know that, and detect absolute
> file names as appropriate: by looking at the drive letter, as in
> "d:/foo".  On the contrary, using file names that begin with a slash
> on Windows is likely to fail the test mentioned by the man page.

No, it will not fail.  Citing tree.c from ispell's source:
```c
        /*
        ** Figure out if p is an absolute path name.  Note that beginning
        ** with "./" and "../" is considered an absolute path, since this
        ** still means we can't prepend HOME.
        */
        abspath = IS_SLASH (*p)  ||  strncmp (p, "./", 2) == 0
          ||  strncmp (p, "../", 3) == 0;
#ifdef MSDOS
        /*
        ** DTRT with drive-letter braindamage and with backslashes.
        */
        abspath |= strncmp (p, ".\\", 2) == 0
          || strncmp (p, "..\\", 3) == 0
          || (p[0] != '\0'  &&  p[1] == ':');
#endif
```

That is it, all of it.
There is no #ifdef WINDOWS block, and even the MSDOS block only "adds"
to the absolute path check, so things starting with "/" are always
considered absolute.

In fact, quoting the README (line 258) from the ispell root:

```
Although the ispell maintainer does not support MS-DOS and
Windows, a generous contributor, Eli Zaretskii, has added MS-DOS
support.  You can build ispell for MS-DOS with either EMX/GCC or
DJGPP.  See the file pc/README for compilation instructions.
```

I am surprised that you are asking me this question.


-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 13:57:49 +0300

> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
> Date: Fri, 08 Aug 2025 10:21:37 +0800
> 
> >> >> Aspell needs to parse the "-vv" command-line argument, and Emacs
> >> >> launched in batch mode cannot do that.  Or I did not manage to make it.
> >> >
> >> > Sorry, I don't understand.  The Bash script just outputs a fixed
> >> > string, and Emacs is capable of doing that.  What did you try, and if
> >> > it didn''t work, how it failed to work?
> >> 
> >> No, it does not /just/ output a fixed string. It also _ignores_ the
> >> command-line argument "-vv", which, if passed to emacs --batch, produces
> >> and error Error: error ("Unknown option ‘-vv’"), and ispell.el does it
> >> when initialising the backend.
> >
> > I feel there's some misunderstanding here.  What I suggested is that
> > instead of doing this:
> >
> >   (let ((fake-aspell-path (expand-file-name
> >                            "./fake-aspell.bash"
> >                            tests-ispell-data-directory)))
> >     (chmod fake-aspell-path 504)
> >     (call-process fake-aspell-path nil nil nil)
> >
> > where fake-aspell.bash does
> >
> >   printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
> >
> > the code does this (not tested):
> >
> >   (call-process "emacs" nil nil nil "-Q" "--batch" "--eval" "(princ \\"@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)\\")")
> >
> > For best results, use
> >
> >   (expand invocation-name invocation-directory)
> >
> > instead of a literal "emacs".  This makes sure you invoke the same
> > Emacs as the one running the test, not some other version.
> 
> I am not calling "fake_aspell.bash" with "(call-process fake-aspell-path
> nil nil nil)" in the name of printing a string.  I am calling it in
> order to make sure that calling it works on the target system.  Read the
> test further on, the important lines of the test are
> ```
> (setopt ispell-program-name fake-aspell-path)
> (ispell-check-version t)
> ```
> where ispell.el will call "fake_aspell.bash" in the way it likes calling
> backend executables.

So this is intended to fail the version check because fake_aspell
script ignores the -vv switch?  If so, why calling Emacs would not do,
since Emacs also doesn't support -vv?

> Maybe I should wrap it in (skip-unless), but I remember it behaving
> strangely within a "let".

I'd prefer not to skip tests if we can avoid that.  Relying on shell
scripts will require us to do something special on platforms without
Bash, and I'd like to avoid that if possible.

> >> >> >> +    (let ((test-dict "nonexistent")
> >> >> >> +          (test-pdict "/tmp/lnonexistent.txt"))
> >> >> >                          ^^^^^^^^^^^^^^^^^^^^^^^
> >> >> > Likewise here: please use
> >> >> >
> >> >> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
> >> >> >
> >> >> > instead.  (There are several other places in the patch with the same
> >> >> > problem.)
> >> >> 
> >> >> This is not a file path.  This is just a string which looks like a file
> >> >> path.  If something tries to access this as if it is a path during the
> >> >> execution of the test, this is an error.
> >> >
> >> > OK, but the file name you use is not an absolute file name on Windows,
> >> > so it could cause problems.  Is there a problem with using my
> >> > suggestion, even though you don't need a real file name?
> >> 
> >> ispell's man page says the following:
> >> 
> >> ```
> >>  The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
> >> ```
> >> 
> >> I do not want ispell to do any mangling of the "HOME" directory if this
> >> "string which looks like a path" is by chance (if a test malfunctions)
> >> passed to ispell binary and it creates a bogus file in the home
> >> directory or overwrites something.
> >> On windows "temporary-file-directory" probably does not begin with "/".
> >
> > Yes, but a Windows port of ispell will know that, and detect absolute
> > file names as appropriate: by looking at the drive letter, as in
> > "d:/foo".  On the contrary, using file names that begin with a slash
> > on Windows is likely to fail the test mentioned by the man page.
> 
> No, it will not fail.  Citing tree.c from ispell's source:
> ```c
>         /*
>         ** Figure out if p is an absolute path name.  Note that beginning
>         ** with "./" and "../" is considered an absolute path, since this
>         ** still means we can't prepend HOME.
>         */
>         abspath = IS_SLASH (*p)  ||  strncmp (p, "./", 2) == 0
>           ||  strncmp (p, "../", 3) == 0;
> #ifdef MSDOS
>         /*
>         ** DTRT with drive-letter braindamage and with backslashes.
>         */
>         abspath |= strncmp (p, ".\\", 2) == 0
>           || strncmp (p, "..\\", 3) == 0
>           || (p[0] != '\0'  &&  p[1] == ':');
> #endif
> ```
> 
> That is it, all of it.
> There is no #ifdef WINDOWS block, and even the MSDOS block only "adds"
> to the absolute path check, so things starting with "/" are always
> considered absolute.

But it also recognizes the absolute file names that start with a drive
letter, which is the full form of an absolute name on Windows.  So
using expand-file-name will not fail the test, would it?  Moreover, we
cannot expect all Windows ports of the various spellers to do what
Ispell does, and it is IMO wrong to rely on specific details of a
specific port in the test suite.  What if in some not-too-distant
future this test will be also used for Aspell or Hunspell, which might
not consider "/foo/bar" and absolute file name on Windows?

> In fact, quoting the README (line 258) from the ispell root:
> 
> ```
> Although the ispell maintainer does not support MS-DOS and
> Windows, a generous contributor, Eli Zaretskii, has added MS-DOS
> support.  You can build ispell for MS-DOS with either EMX/GCC or
> DJGPP.  See the file pc/README for compilation instructions.
> ```
> 
> I am surprised that you are asking me this question.

You always remember every line of code you wrote 20+ years ago?

And anyway, the important point here is not to rely on such fine
details of the implementation, but instead to go with the general
conventions of the target platform.  Because if the application
doesn't behave according to those conventions, it's a bug in the
application that needs to be fixed there.

Thanks.

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79177 <at> debbugs.gnu.org, Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 19:49:19 +0800

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
>> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
>> Date: Fri, 08 Aug 2025 10:21:37 +0800
>> 
>> >> >> Aspell needs to parse the "-vv" command-line argument, and Emacs
>> >> >> launched in batch mode cannot do that.  Or I did not manage to make it.
>> >> >
>> >> > Sorry, I don't understand.  The Bash script just outputs a fixed
>> >> > string, and Emacs is capable of doing that.  What did you try, and if
>> >> > it didn''t work, how it failed to work?
>> >> 
>> >> No, it does not /just/ output a fixed string. It also _ignores_ the
>> >> command-line argument "-vv", which, if passed to emacs --batch, produces
>> >> and error Error: error ("Unknown option ‘-vv’"), and ispell.el does it
>> >> when initialising the backend.
>> >
>> > I feel there's some misunderstanding here.  What I suggested is that
>> > instead of doing this:
>> >
>> >   (let ((fake-aspell-path (expand-file-name
>> >                            "./fake-aspell.bash"
>> >                            tests-ispell-data-directory)))
>> >     (chmod fake-aspell-path 504)
>> >     (call-process fake-aspell-path nil nil nil)
>> >
>> > where fake-aspell.bash does
>> >
>> >   printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>> >
>> > the code does this (not tested):
>> >
>> >   (call-process "emacs" nil nil nil "-Q" "--batch" "--eval" "(princ \\"@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)\\")")
>> >
>> > For best results, use
>> >
>> >   (expand invocation-name invocation-directory)
>> >
>> > instead of a literal "emacs".  This makes sure you invoke the same
>> > Emacs as the one running the test, not some other version.
>> 
>> I am not calling "fake_aspell.bash" with "(call-process fake-aspell-path
>> nil nil nil)" in the name of printing a string.  I am calling it in
>> order to make sure that calling it works on the target system.  Read the
>> test further on, the important lines of the test are
>> ```
>> (setopt ispell-program-name fake-aspell-path)
>> (ispell-check-version t)
>> ```
>> where ispell.el will call "fake_aspell.bash" in the way it likes calling
>> backend executables.
>
> So this is intended to fail the version check because fake_aspell
> script ignores the -vv switch?  If so, why calling Emacs would not do,
> since Emacs also doesn't support -vv?

This is intended to succeed, because "fake_aspell.bash", irrespective of
the options, behaves in the way real aspell behaves when being called
with "-vv".

I thought that I might eventually mock the important bits of aspell
without actually implementing the dictionary-lookup algorithm, so that
it would be possible to run at least some backend-involving tests
without having _any_ backend installed on the device.  It would have
been nice to implement this mock in Emacs, but, as I said, emacs --batch
processing of command-line arguments does not allow that at present.

I do think that having bash is more common than having ispell, aspell,
or hunspell.  If you think otherwise, I can replace this call with a
call to real {ispell, aspell, hunspell, enchant-2}, like the other tests
do.

>> Maybe I should wrap it in (skip-unless), but I remember it behaving
>> strangely within a "let".
>
> I'd prefer not to skip tests if we can avoid that.  Relying on shell
> scripts will require us to do something special on platforms without
> Bash, and I'd like to avoid that if possible.

I can try to use an :override advice to `call-process' instead.  It
actually worked on my machine, even though I remember that advising
primitive forms is expected to be glitchy.

>> >> >> >> +    (let ((test-dict "nonexistent")
>> >> >> >> +          (test-pdict "/tmp/lnonexistent.txt"))
>> >> >> >                          ^^^^^^^^^^^^^^^^^^^^^^^
>> >> >> > Likewise here: please use
>> >> >> >
>> >> >> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
>> >> >> >
>> >> >> > instead.  (There are several other places in the patch with the same
>> >> >> > problem.)
>> >> >> 
>> >> >> This is not a file path.  This is just a string which looks like a file
>> >> >> path.  If something tries to access this as if it is a path during the
>> >> >> execution of the test, this is an error.
>> >> >
>> >> > OK, but the file name you use is not an absolute file name on Windows,
>> >> > so it could cause problems.  Is there a problem with using my
>> >> > suggestion, even though you don't need a real file name?
>> >> 
>> >> ispell's man page says the following:
>> >> 
>> >> ```
>> >>  The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
>> >> ```
>> >> 
>> >> I do not want ispell to do any mangling of the "HOME" directory if this
>> >> "string which looks like a path" is by chance (if a test malfunctions)
>> >> passed to ispell binary and it creates a bogus file in the home
>> >> directory or overwrites something.
>> >> On windows "temporary-file-directory" probably does not begin with "/".
>> >
>> > Yes, but a Windows port of ispell will know that, and detect absolute
>> > file names as appropriate: by looking at the drive letter, as in
>> > "d:/foo".  On the contrary, using file names that begin with a slash
>> > on Windows is likely to fail the test mentioned by the man page.
>> 
>> No, it will not fail.  Citing tree.c from ispell's source:
>> ```c
>>         /*
>>         ** Figure out if p is an absolute path name.  Note that beginning
>>         ** with "./" and "../" is considered an absolute path, since this
>>         ** still means we can't prepend HOME.
>>         */
>>         abspath = IS_SLASH (*p)  ||  strncmp (p, "./", 2) == 0
>>           ||  strncmp (p, "../", 3) == 0;
>> #ifdef MSDOS
>>         /*
>>         ** DTRT with drive-letter braindamage and with backslashes.
>>         */
>>         abspath |= strncmp (p, ".\\", 2) == 0
>>           || strncmp (p, "..\\", 3) == 0
>>           || (p[0] != '\0'  &&  p[1] == ':');
>> #endif
>> ```
>> 
>> That is it, all of it.
>> There is no #ifdef WINDOWS block, and even the MSDOS block only "adds"
>> to the absolute path check, so things starting with "/" are always
>> considered absolute.
>
> But it also recognizes the absolute file names that start with a drive
> letter, which is the full form of an absolute name on Windows.

No it does not. But even if it did... (see the next paragraph)...

> So
> using expand-file-name will not fail the test, would it?  Moreover, we
> cannot expect all Windows ports of the various spellers to do what
> Ispell does, and it is IMO wrong to rely on specific details of a
> specific port in the test suite.  What if in some not-too-distant
> future this test will be also used for Aspell or Hunspell, which might
> not consider "/foo/bar" and absolute file name on Windows?

If you look at the test carefully once again, you will see that it does
the following
#+begin_src emacs-lisp
(insert
       "hello\n\n\n"
       "<!-- " ispell-dictionary-keyword test-dict " -->"
       "<!-- " ispell-pdict-keyword test-pdict " -->")
#+end_src

This test runs the test on a buffer, which is such as if it is written
by a user.  Surely we can expect files having
"Local IspellPersDict: /tmp/nonexistent.dict" in the footer,
being _written_ by SOME users, by _opened_on_Windows_ by OTHER users,
don't you think?  Don't you think that such files are expected to be
processed gracefully by Emacs, regardless of whether such paths are
absolute, relative, nonexistent, or even UNC?  Moreover, a user might
even put something like:
"Local IspellPersDict: (my-best-personal-dictionary)",
or
"Local IspellPersDict: https://cgit.git.savannah.gnu.org/cgit/miscfiles.git/tree/web2"
(Such structures are not supported, and should be ignored, or warned
about by ~ispell.el~.)
This is "user I/O", and tests, at least usually, are not expected to
receive graceful input, they are expected to make sure that the code
works reasonably even on garbage input.

>> In fact, quoting the README (line 258) from the ispell root:
>>
>> ```
>> Although the ispell maintainer does not support MS-DOS and
>> Windows, a generous contributor, Eli Zaretskii, has added MS-DOS
>> support.  You can build ispell for MS-DOS with either EMX/GCC or
>> DJGPP.  See the file pc/README for compilation instructions.
>> ```
>>
>> I am surprised that you are asking me this question.
>
> You always remember every line of code you wrote 20+ years ago?

> And anyway, the important point here is not to rely on such fine
> details of the implementation, but instead to go with the general
> conventions of the target platform.  Because if the application
> doesn't behave according to those conventions, it's a bug in the
> application that needs to be fixed there.
>

Fair enough, but overall, the convention of Windows (or any systemn) is
to not fail on text files having some UNIX paths in them.  I can add a
test having "C:\TMP\nonexistent.pdict", to make sure that all platforms
are covered, if you'd like.

-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 16:27:11 +0300

> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Cc: Lockywolf <for_emacs_1 <at> lockywolf.net>,  79177 <at> debbugs.gnu.org
> Date: Sat, 09 Aug 2025 19:49:19 +0800
> 
> > So this is intended to fail the version check because fake_aspell
> > script ignores the -vv switch?  If so, why calling Emacs would not do,
> > since Emacs also doesn't support -vv?
> 
> This is intended to succeed, because "fake_aspell.bash", irrespective of
> the options, behaves in the way real aspell behaves when being called
> with "-vv".

If there's no way of using some program that we are sure will be
installed, please use skip-unless to skip the test when Bash is not
available.

> >> >> >> >> +    (let ((test-dict "nonexistent")
> >> >> >> >> +          (test-pdict "/tmp/lnonexistent.txt"))
> >> >> >> >                          ^^^^^^^^^^^^^^^^^^^^^^^
> >> >> >> > Likewise here: please use
> >> >> >> >
> >> >> >> >    (expand-file-name "lnonexistent.txt" temporary-file-directory)
> >> >> >> >
> >> >> >> > instead.  (There are several other places in the patch with the same
> >> >> >> > problem.)
> >> >> >> 
> >> >> >> This is not a file path.  This is just a string which looks like a file
> >> >> >> path.  If something tries to access this as if it is a path during the
> >> >> >> execution of the test, this is an error.
> >> >> >
> >> >> > OK, but the file name you use is not an absolute file name on Windows,
> >> >> > so it could cause problems.  Is there a problem with using my
> >> >> > suggestion, even though you don't need a real file name?
> >> >> 
> >> >> ispell's man page says the following:
> >> >> 
> >> >> ```
> >> >>  The  -p  option  is  used  to  specify an alternate personal dictionary file.  If the file name does not begin with "/", $HOME is prefixed.
> >> >> ```
> >> >> 
> >> >> I do not want ispell to do any mangling of the "HOME" directory if this
> >> >> "string which looks like a path" is by chance (if a test malfunctions)
> >> >> passed to ispell binary and it creates a bogus file in the home
> >> >> directory or overwrites something.
> >> >> On windows "temporary-file-directory" probably does not begin with "/".
> >> >
> >> > Yes, but a Windows port of ispell will know that, and detect absolute
> >> > file names as appropriate: by looking at the drive letter, as in
> >> > "d:/foo".  On the contrary, using file names that begin with a slash
> >> > on Windows is likely to fail the test mentioned by the man page.
> >> 
> >> No, it will not fail.  Citing tree.c from ispell's source:
> >> ```c
> >>         /*
> >>         ** Figure out if p is an absolute path name.  Note that beginning
> >>         ** with "./" and "../" is considered an absolute path, since this
> >>         ** still means we can't prepend HOME.
> >>         */
> >>         abspath = IS_SLASH (*p)  ||  strncmp (p, "./", 2) == 0
> >>           ||  strncmp (p, "../", 3) == 0;
> >> #ifdef MSDOS
> >>         /*
> >>         ** DTRT with drive-letter braindamage and with backslashes.
> >>         */
> >>         abspath |= strncmp (p, ".\\", 2) == 0
> >>           || strncmp (p, "..\\", 3) == 0
> >>           || (p[0] != '\0'  &&  p[1] == ':');
> >> #endif
> >> ```
> >> 
> >> That is it, all of it.
> >> There is no #ifdef WINDOWS block, and even the MSDOS block only "adds"
> >> to the absolute path check, so things starting with "/" are always
> >> considered absolute.
> >
> > But it also recognizes the absolute file names that start with a drive
> > letter, which is the full form of an absolute name on Windows.
> 
> No it does not. But even if it did... (see the next paragraph)...
> 
> > So
> > using expand-file-name will not fail the test, would it?  Moreover, we
> > cannot expect all Windows ports of the various spellers to do what
> > Ispell does, and it is IMO wrong to rely on specific details of a
> > specific port in the test suite.  What if in some not-too-distant
> > future this test will be also used for Aspell or Hunspell, which might
> > not consider "/foo/bar" and absolute file name on Windows?
> 
> If you look at the test carefully once again, you will see that it does
> the following
> #+begin_src emacs-lisp
> (insert
>        "hello\n\n\n"
>        "<!-- " ispell-dictionary-keyword test-dict " -->"
>        "<!-- " ispell-pdict-keyword test-pdict " -->")
> #+end_src
> 
> This test runs the test on a buffer, which is such as if it is written
> by a user.  Surely we can expect files having
> "Local IspellPersDict: /tmp/nonexistent.dict" in the footer,
> being _written_ by SOME users, by _opened_on_Windows_ by OTHER users,
> don't you think?  Don't you think that such files are expected to be
> processed gracefully by Emacs, regardless of whether such paths are
> absolute, relative, nonexistent, or even UNC?  Moreover, a user might
> even put something like:
> "Local IspellPersDict: (my-best-personal-dictionary)",
> or
> "Local IspellPersDict: https://cgit.git.savannah.gnu.org/cgit/miscfiles.git/tree/web2"
> (Such structures are not supported, and should be ignored, or warned
> about by ~ispell.el~.)
> This is "user I/O", and tests, at least usually, are not expected to
> receive graceful input, they are expected to make sure that the code
> works reasonably even on garbage input.

Look, when I see "/foo/bar" file names in Emacs sources, it
immediately draws my attention, because such file names cause
portability problems.  So, unless using expand-file-name and/or
temporary-file-directory here is somehow problematic and causes
trouble, please do use them, so that this doesn't stand out, okay?

> >> In fact, quoting the README (line 258) from the ispell root:
> >>
> >> ```
> >> Although the ispell maintainer does not support MS-DOS and
> >> Windows, a generous contributor, Eli Zaretskii, has added MS-DOS
> >> support.  You can build ispell for MS-DOS with either EMX/GCC or
> >> DJGPP.  See the file pc/README for compilation instructions.
> >> ```
> >>
> >> I am surprised that you are asking me this question.
> >
> > You always remember every line of code you wrote 20+ years ago?
> 
> > And anyway, the important point here is not to rely on such fine
> > details of the implementation, but instead to go with the general
> > conventions of the target platform.  Because if the application
> > doesn't behave according to those conventions, it's a bug in the
> > application that needs to be fixed there.
> >
> 
> Fair enough, but overall, the convention of Windows (or any systemn) is
> to not fail on text files having some UNIX paths in them.  I can add a
> test having "C:\TMP\nonexistent.pdict", to make sure that all platforms
> are covered, if you'd like.

See above: I just want these Unix-style absolute file name not to
stand out.

Thanks.

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79177 <at> debbugs.gnu.org, Lockywolf <for_emacs_1 <at> lockywolf.net>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 23:29:32 +0800

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

Eli Zaretskii <eliz <at> gnu.org> writes:

> If there's no way of using some program that we are sure will be
> installed, please use skip-unless to skip the test when Bash is not
> available.

Done.

> Look, when I see "/foo/bar" file names in Emacs sources, it
> immediately draws my attention, because such file names cause
> portability problems.  So, unless using expand-file-name and/or
> temporary-file-directory here is somehow problematic and causes
> trouble, please do use them, so that this doesn't stand out, okay?
>

I have tried to make the best of the discussion.
The test is there, and it should reliably fail on both GNU and Windows.


-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

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

From: Lockywolf <for_emacs_1 <at> lockywolf.net>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 23:31:29 +0800

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

Lockywolf <for_emacs_1 <at> lockywolf.net> writes:

> [[PGP Signed Part:Good signature from FBD6C1B20FEC6DAA Lockywolf (Provisional) <*@lockywolf.net> (trust ultimate) created at 2025-08-09T23:29:34+0800 using EDDSA]]
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> If there's no way of using some program that we are sure will be
>> installed, please use skip-unless to skip the test when Bash is not
>> available.
>
> Done.
>
>> Look, when I see "/foo/bar" file names in Emacs sources, it
>> immediately draws my attention, because such file names cause
>> portability problems.  So, unless using expand-file-name and/or
>> temporary-file-directory here is somehow problematic and causes
>> trouble, please do use them, so that this doesn't stand out, okay?
>>
>
> I have tried to make the best of the discussion.
> The test is there, and it should reliably fail on both GNU and Windows.

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

[0001-ispell.el-Add-42-tests.-Fix-79177.patch (text/x-patch, attachment)]

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

-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lockywolf <for_emacs_1 <at> lockywolf.net>
Cc: 79177 <at> debbugs.gnu.org
Subject: Re: bug#79177: 31.0.50; [PATCH] [v4] Add tests to ispell.el
Date: Sat, 09 Aug 2025 19:08:18 +0300

> From: Lockywolf <for_emacs_1 <at> lockywolf.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  79177 <at> debbugs.gnu.org
> Date: Sat, 09 Aug 2025 23:31:29 +0800
> 
> Lockywolf <for_emacs_1 <at> lockywolf.net> writes:
> 
> > [[PGP Signed Part:Good signature from FBD6C1B20FEC6DAA Lockywolf (Provisional) <*@lockywolf.net> (trust ultimate) created at 2025-08-09T23:29:34+0800 using EDDSA]]
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >> If there's no way of using some program that we are sure will be
> >> installed, please use skip-unless to skip the test when Bash is not
> >> available.
> >
> > Done.
> >
> >> Look, when I see "/foo/bar" file names in Emacs sources, it
> >> immediately draws my attention, because such file names cause
> >> portability problems.  So, unless using expand-file-name and/or
> >> temporary-file-directory here is somehow problematic and causes
> >> trouble, please do use them, so that this doesn't stand out, okay?
> >>
> >
> > I have tried to make the best of the discussion.
> > The test is there, and it should reliably fail on both GNU and Windows.

Thanks.  I've now run the tests on my system, and I have a couple of
minor comments.

First, the names of the test files don't follow our conventions.  They
should be ispell-aspell-tests.el and ispell-ispell-tests.el or just
ispell-tests.el.  IOW, the names must end in "-tests.el".

> +(ert-deftest ispell/ispell-init-process/works-nohome ()
> +  "Simple test to check that ispell-init-process works."
> +  :expected-result :failed
> +  (skip-unless (ispell-tests--some-backend-available-p))
> +  (letopt ((ispell-program-name (ispell-tests--some-backend)))
> +          (with-temp-buffer
> +            (ispell-init-process))))

This test unexpectedly passed on my system.  Can you explain why you
expected it to fail?

> +(ert-deftest ispell/ispell-accept-buffer-local-defs/simple ()
> +  "Check that `ispell-accept-buffer-local-defs' works for a
> +batch mode.
> +1. local words
> +2. dictionary and pdict
> +3. parser and extcharmode"
> +  (with-environment-variables (("HOME" temporary-file-directory))
> +    (with-temp-buffer
> +      (let ((test-dictname "english")
> +            (test-extcharmode "~latin3")
> +            (test-parser "~testparser")
> +            (test-localword1 "aaaaaaaaaaaaa")
> +            (test-localword2 "bbbbbbbbbbb")
> +            (test-pdict "test-pdict.pdict"))

This test is Ispell-specific, it will not work if the spell-checker
is, say, Hunspell.  For example, Hunspell's English dictionary is
"en_US", not "English".  So you should skip it if the backend is not
literally Ispell.

Previous Next

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