Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Robert Pluim]

>>>>> On Thu, 12 Jan 2023 12:48:16 -0500 (EST), Juri Linkov <juri@jurta.org> said:

    Juri> branch: emacs-29
    Juri> commit 9ecebcdded157e1efc2f51b67967fd101797f225
    Juri> Author: Gregory Heytings <gregory@heytings.org>
    Juri> Commit: Juri Linkov <juri@linkov.net>

    Juri>     * lisp/simple.el (next-completion): Handle first completion specially.
    
    Juri>     When completions-header-format is nil and completion-show-help is nil,
    Juri>     the first completion is at the beginning the buffer, so 'M-<down>'
    Juri>     missed it and moved to the second completion.  Handle this case by
    Juri>     setting/checking the special text-property 'first-completion' that
    Juri>     is nil at the first call (bug#60411).

This causes the following test failure for me:

Making completion list...

Test completions-header-format-test backtrace:
  signal(ert-test-failed (((should (equal "ac" (get-text-property (poi
  ert-fail(((should (equal "ac" (get-text-property (point) 'completion
  #f(compiled-function () #<bytecode -0x141bfad86d70e309>)()
  minibuffer-setup()
  read-from-minibuffer("Prompt: " nil (keymap (menu-bar keymap (minibu
  completing-read-default("Prompt: " ("aa" "ab" "ac") nil nil nil nil 
  completing-read("Prompt: " ("aa" "ab" "ac"))
  #f(compiled-function () #<bytecode -0x1b72fd9b89f49f74>)()
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name completions-header-format-test :docum
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
  ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
  ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
  eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/minibuffer-tests" "-
  command-line()
  normal-top-level()
Test completions-header-format-test condition:
    (ert-test-failed
     ((should
       (equal "ac"
	      (get-text-property ... ...)))
      :form
      (equal "ac"
	     #("ab" 0 1
	       (face ...)
	       1 2
	       (face ...)))
      :value nil :explanation
      (array-elt 1
		 (different-atoms
		  (99 "#x63" "?c")
		  (98 "#x62" "?b")))))
   FAILED  25/25  completions-header-format-test (0.000478 sec) at lisp/minibuffer-tests.el:414

Robert
-- 

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Gregory Heytings <gregory@heytings.org>, Juri Linkov <juri@linkov.net>
> Date: Fri, 13 Jan 2023 13:49:47 +0100
> 
> >>>>> On Thu, 12 Jan 2023 12:48:16 -0500 (EST), Juri Linkov <juri@jurta.org> said:
> 
>     Juri> branch: emacs-29
>     Juri> commit 9ecebcdded157e1efc2f51b67967fd101797f225
>     Juri> Author: Gregory Heytings <gregory@heytings.org>
>     Juri> Commit: Juri Linkov <juri@linkov.net>
> 
>     Juri>     * lisp/simple.el (next-completion): Handle first completion specially.
>     
>     Juri>     When completions-header-format is nil and completion-show-help is nil,
>     Juri>     the first completion is at the beginning the buffer, so 'M-<down>'
>     Juri>     missed it and moved to the second completion.  Handle this case by
>     Juri>     setting/checking the special text-property 'first-completion' that
>     Juri>     is nil at the first call (bug#60411).
> 
> This causes the following test failure for me:
> 
> Making completion list...
> 
> Test completions-header-format-test backtrace:
>   signal(ert-test-failed (((should (equal "ac" (get-text-property (poi
>   ert-fail(((should (equal "ac" (get-text-property (point) 'completion
>   #f(compiled-function () #<bytecode -0x141bfad86d70e309>)()
>   minibuffer-setup()
>   read-from-minibuffer("Prompt: " nil (keymap (menu-bar keymap (minibu
>   completing-read-default("Prompt: " ("aa" "ab" "ac") nil nil nil nil 
>   completing-read("Prompt: " ("aa" "ab" "ac"))
>   #f(compiled-function () #<bytecode -0x1b72fd9b89f49f74>)()
>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>   ert-run-test(#s(ert-test :name completions-header-format-test :docum
>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>   ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
>   ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
>   ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
>   eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
>   command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/minibuffer-tests" "-
>   command-line()
>   normal-top-level()
> Test completions-header-format-test condition:
>     (ert-test-failed
>      ((should
>        (equal "ac"
> 	      (get-text-property ... ...)))
>       :form
>       (equal "ac"
> 	     #("ab" 0 1
> 	       (face ...)
> 	       1 2
> 	       (face ...)))
>       :value nil :explanation
>       (array-elt 1
> 		 (different-atoms
> 		  (99 "#x63" "?c")
> 		  (98 "#x62" "?b")))))
>    FAILED  25/25  completions-header-format-test (0.000478 sec) at lisp/minibuffer-tests.el:414

Fixed.  It's a clear case of a test that adapted the results to what
the code produced, not to the desired/expected value.

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Robert Pluim]

>>>>> On Fri, 13 Jan 2023 15:37:36 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> Fixed.  It's a clear case of a test that adapted the results to what
    Eli> the code produced, not to the desired/expected value.

Itʼs fixed, but Iʼll admit that I donʼt understand why:

      (should (equal "aa" (get-text-property (point) 'completion--string)))
      (next-completion 3)
      (should (equal "ac" (get-text-property (point) 'completion--string)))
      (previous-completion 2)
      (should (equal "aa" (get-text-property (point) 'completion--string)))

So we go forward 3 from "aa" to get to "ac", but then to get back to
"aa" itʼs back 2?

Robert
-- 

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org,  gregory@heytings.org,  juri@linkov.net
> Date: Fri, 13 Jan 2023 14:57:08 +0100
> 
> >>>>> On Fri, 13 Jan 2023 15:37:36 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> Fixed.  It's a clear case of a test that adapted the results to what
>     Eli> the code produced, not to the desired/expected value.
> 
> Itʼs fixed, but Iʼll admit that I donʼt understand why:
> 
>       (should (equal "aa" (get-text-property (point) 'completion--string)))
>       (next-completion 3)
>       (should (equal "ac" (get-text-property (point) 'completion--string)))
>       (previous-completion 2)
>       (should (equal "aa" (get-text-property (point) 'completion--string)))
> 
> So we go forward 3 from "aa" to get to "ac", but then to get back to
> "aa" itʼs back 2?

Because the first next-completion goes to "aa".  The original
"position" is _before_ the first candidate: that's what the commit
that you blamed was about.

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Juri Linkov]

>     Eli> Fixed.  It's a clear case of a test that adapted the results to what
>     Eli> the code produced, not to the desired/expected value.
>
> Itʼs fixed, but Iʼll admit that I donʼt understand why:
>
>       (should (equal "aa" (get-text-property (point) 'completion--string)))
>       (next-completion 3)
>       (should (equal "ac" (get-text-property (point) 'completion--string)))
>       (previous-completion 2)
>       (should (equal "aa" (get-text-property (point) 'completion--string)))
>
> So we go forward 3 from "aa" to get to "ac", but then to get back to
> "aa" itʼs back 2?

Indeed, the test was correct, but the implementation has a bug.

Re: emacs-29 9ecebcdded: * lisp/simple.el (next-completion): Handle first completion specially.

[Juri Linkov]

>>     Eli> Fixed.  It's a clear case of a test that adapted the results to what
>>     Eli> the code produced, not to the desired/expected value.
>>
>> Itʼs fixed, but Iʼll admit that I donʼt understand why:
>>
>>       (should (equal "aa" (get-text-property (point) 'completion--string)))
>>       (next-completion 3)
>>       (should (equal "ac" (get-text-property (point) 'completion--string)))
>>       (previous-completion 2)
>>       (should (equal "aa" (get-text-property (point) 'completion--string)))
>>
>> So we go forward 3 from "aa" to get to "ac", but then to get back to
>> "aa" itʼs back 2?
>
> Indeed, the test was correct, but the implementation has a bug.

This bug is fixed now.