Re: master 40629706b1f 1/2: Avoid unnecessary escaping in 'url-build-query-string'

Re: master 40629706b1f 1/2: Avoid unnecessary escaping in 'url-build-query-string'

[john muhl]

[-- Attachment #1: Type: text/plain, Size: 112 bytes --]

Bisect says this commit caused the gravatar tests to start failing.

  make test/lisp/image/gravatar-tests.log


[-- Attachment #2: gravatar-tests.log --]
[-- Type: text/plain, Size: 2361 bytes --]

Running 5 tests (2024-04-19 07:59:37-0500, selector `(not (or (tag :unstable) (tag :nativecomp)))')
   passed  1/5  gravatar-build-url (0.000094 sec)
Test gravatar-default-image backtrace:
  signal(ert-test-failed (((should (equal (gravatar--query-string) "r=
  ert-fail(((should (equal (gravatar--query-string) "r=g&d=https%3A%2F
  (if (unwind-protect (setq value-47 (apply fn-45 args-46)) (setq form
  (let (form-description-49) (if (unwind-protect (setq value-47 (apply
  (let ((value-47 'ert-form-evaluation-aborted-48)) (let (form-descrip
  (let* ((fn-45 #'equal) (args-46 (condition-case err (list (gravatar-
  (let ((gravatar-default-image "https://foo/bar.png")) (let* ((fn-45 
  (let ((gravatar-force-default nil) (gravatar-size nil)) (let ((grava
  (closure (t) nil (let ((gravatar-force-default nil) (gravatar-size n
  #f(compiled-function () #<bytecode -0xadbdac02ae7c0ba>)()
  handler-bind-1(#f(compiled-function () #<bytecode -0xadbdac02ae7c0ba
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name gravatar-default-image :documentation
  ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
  ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
  ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
  ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
  eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
  command-line-1(("-L" ":." "-l" "ert" "--eval" "(setq treesit-extra-l
  command-line()
  normal-top-level()
Test gravatar-default-image condition:
    (ert-test-failed
     ((should
       (equal (gravatar--query-string) "r=g&d=https%3A%2F%2Ffoo%2Fbar.png"))
      :form
      (equal "r=g&d=https://foo/bar.png"
	     "r=g&d=https%3A%2F%2Ffoo%2Fbar.png")
      :value nil :explanation
      (arrays-of-different-length 25 33 "r=g&d=https://foo/bar.png"
				  "r=g&d=https%3A%2F%2Ffoo%2Fbar.png"
				  first-mismatch-at 11)))
   FAILED  2/5  gravatar-default-image (0.000103 sec) at lisp/image/gravatar-tests.el:43
   passed  3/5  gravatar-force-default (0.000112 sec)
   passed  4/5  gravatar-hash (0.000100 sec)
   passed  5/5  gravatar-size (0.000087 sec)

Ran 5 tests, 4 results as expected, 1 unexpected (2024-04-19 07:59:37-0500, 0.091112 sec)

1 unexpected results:
   FAILED  gravatar-default-image


[-- Attachment #3: Type: text/plain, Size: 2612 bytes --]


In GNU Emacs 30.0.50 (build 9, x86_64-pc-linux-gnu) of 2024-04-18 built
 on localhost
Repository revision: 0a57dfcff8d0abcf4427cfbfd886264bb3b8eaab
Repository branch: master
System Description: Fedora Linux 39 (Thirty Nine)

Configured using:
 'configure --without-x --without-native-compilation'

Configured features:
ACL DBUS GMP GNUTLS GPM LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY
INOTIFY PDUMPER SECCOMP SOUND SQLITE3 THREADS TREE_SITTER XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort regexp-opt mail-extr emacsbug message mailcap yank-media
puny dired dnd dired-loaddefs rfc822 mml mml-sec password-cache epa
derived epg rfc6068 epg-config gnus-util text-property-search time-date
subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils term/xterm xterm byte-opt gv
bytecomp byte-compile rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode tabulated-list
replace newcomment text-mode lisp-mode prog-mode register page tab-bar
menu-bar rfn-eshadow isearch easymenu timer select mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify multi-tty make-network-process emacs)

Memory information:
((conses 16 47773 9232) (symbols 48 5832 0) (strings 32 14290 1892)
 (string-bytes 1 350831) (vectors 16 7371) (vector-slots 8 65469 5312)
 (floats 8 24 11330) (intervals 56 210 0) (buffers 992 10))

Re: master 40629706b1f 1/2: Avoid unnecessary escaping in 'url-build-query-string'

[Eli Zaretskii]

> From: john muhl <jm@pub.pink>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> Date: Thu, 18 Apr 2024 11:01:56 -0500
> 
> Bisect says this commit caused the gravatar tests to start failing.
> 
>   make test/lisp/image/gravatar-tests.log

Thanks.  Ilmari, could you please look into this?  The question is
whether we need to adapt the gravatar tests, or fine-tune your changes
to url-utils.

Re: master 40629706b1f 1/2: Avoid unnecessary escaping in 'url-build-query-string'

[Dagfinn Ilmari Mannsåker]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: john muhl <jm@pub.pink>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
>> Date: Thu, 18 Apr 2024 11:01:56 -0500
>> 
>> Bisect says this commit caused the gravatar tests to start failing.
>> 
>>   make test/lisp/image/gravatar-tests.log
>
> Thanks.  Ilmari, could you please look into this?  The question is
> whether we need to adapt the gravatar tests, or fine-tune your changes
> to url-utils.

I think the gravatar test should be adjusted. Both the old and the new
form are valid, and this over-escaping is exactly what my change was
intended to address.

However, I appear to have exhausted the acceptable amount contribution
without doing the copyright assignment dance, which I have no time for,
so someone else will have to make the actual change.

- ilmari

Re: master 40629706b1f 1/2: Avoid unnecessary escaping in 'url-build-query-string'

[Eli Zaretskii]

> From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
> Cc: john muhl <jm@pub.pink>,  emacs-devel@gnu.org
> Date: Tue, 23 Apr 2024 17:07:31 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: john muhl <jm@pub.pink>
> >> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> >> Date: Thu, 18 Apr 2024 11:01:56 -0500
> >> 
> >> Bisect says this commit caused the gravatar tests to start failing.
> >> 
> >>   make test/lisp/image/gravatar-tests.log
> >
> > Thanks.  Ilmari, could you please look into this?  The question is
> > whether we need to adapt the gravatar tests, or fine-tune your changes
> > to url-utils.
> 
> I think the gravatar test should be adjusted. Both the old and the new
> form are valid, and this over-escaping is exactly what my change was
> intended to address.
> 
> However, I appear to have exhausted the acceptable amount contribution
> without doing the copyright assignment dance, which I have no time for,
> so someone else will have to make the actual change.

Thanks, someone already did.