zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] E02 test failure / ztst *> issue
@ 2020-03-11 21:15 dana
  2020-03-12  2:37 ` Jun T
  0 siblings, 1 reply; 6+ messages in thread
From: dana @ 2020-03-11 21:15 UTC (permalink / raw)
  To: Zsh hackers list

When i run E02 on master, on macOS, i get an failure for the new 'preserves
tracing' test (see output below). On my machine, `which` outputs the name of
the ヌ function without quoting.

I was going to just change it like this:

  ->$'\M-c\M-\C-C\M-\C-L' () {
  +*>(ヌ|$'\M-c\M-\C-C\M-\C-L') \(\) {

But that doesn't work the way i expected — it breaks the surrounding lines.
The documentation (B01) says this about `*>`:

  # Each line of a '>' and '?' chunk may be preceded by a '*', so the line
  # starts '*>' or '*?'.  This signifies that for any line with '*' in front
  # the actual output should be pattern matched against the corresponding
  # lines in the test output.

In actuality, if *any* `>` line is preceded by `*`, *all* of them are treated
as patterns to be matched against the corresponding lines in the output. Thus,
if the surrounding lines contain pattern meta-characters (which they do), they
need to be escaped to work correctly.

I'm not sure if the documentation is wrong or if the behaviour is. It seems
like it would be helpful if it worked the way it said, but it looks like we'd
have to pass pattern-containing line numbers or something to ZTST_diff for it
to do that. idk

dana


--- /tmp/zsh.ztst.28982/ztst.out	2020-03-11 15:57:36.000000000 -0500
+++ /tmp/zsh.ztst.28982/ztst.tout	2020-03-11 15:57:36.000000000 -0500
@@ -6,7 +6,7 @@
 	# traced
 	echo inner
 }
-$'\M-c\M-\C-C\M-\C-L' () {
+ヌ () {
 	# traced
 	echo inner
 }
Test ./E02xtrace.ztst failed: output differs from expected as shown above for:
  test_cases=(
      f            # baseline
      foo-bar      # Dash
      ヌ           # Meta (the UTF-8 representation of this character has an 0x83 byte)
      \$\'ba\\0z\' # Nul, escaped as though by ${(qqqq)}
  )
  for 1 in "$test_cases[@]"; do
    eval "
      ${1}() {
        ${1}() { echo inner }
      }
      functions -T ${1}
      ${1}
      which ${1}
    "
  done
Was testing: a function that redefines itself preserves tracing


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] E02 test failure / ztst *> issue
  2020-03-11 21:15 [BUG] E02 test failure / ztst *> issue dana
@ 2020-03-12  2:37 ` Jun T
  2020-03-12 17:31   ` dana
  0 siblings, 1 reply; 6+ messages in thread
From: Jun T @ 2020-03-12  2:37 UTC (permalink / raw)
  To: zsh-workers


> 2020/03/12 6:15, dana <dana@dana.is> wrote:
> 
> When i run E02 on master, on macOS, i get an failure for the new 'preserves
> tracing' test (see output below). On my machine, `which` outputs the name of
> the ヌ function without quoting.

I noticed this on macOS few days ago or so, and was testing on other BSDs,
and yes, the test fails (at least) on Free/Net/Open BSDs. It turned out
that the problem is in zsh itself; a patch is at the end of this post.


The test is run under C-locale, but on BSDs "which" outputs the function
name ヌ (utdf-8: e3 83 8c) as raw bytes instead of escaping by \M and \C.
On Linux the test succeeds.

The function name is printed by quotedzputs(), which calls
is_mb_niceformat(), and mbrtowc() (line 5422, utils.c). Under C locale,
mbrtowc() behaves differently on Linux and BSDs.

On Linux, mbrtowc(&c, 0xe3;0x83;0x8c, ..) returns MB_INVALID if called
under C locale. So the 1st byte 0xe3 is an invalid byte.

On BSDs it returns 1, indicating that 0xe3 is a valid single byte
character (but not printable because isprint(0xe3) is 0).

But this should not make any difference, because zsh is *expected* to
print either invalid or unprintable byte by escaping with \M- and \C-
(by using one of nicechar-family functions).

The real problem is in is_wcs_nicechar() that is called from
is_mb_niceformat() (line 5452, utils.c); is_mb_niceformat(0xe3)
should return 1 but actually returns 0. The 2nd hunk in the path below
fixes this. But only with this hunk, the function name is output as
$'\M-c\M-^C\M-^L'
i.e., ^X instead of \C-X. The first hunk in the patch fixes this.



diff --git a/Src/utils.c b/Src/utils.c
index f9c2d4a2b..5ffc459f2 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -711,7 +711,7 @@ wcs_nicechar_sel(wchar_t c, size_t *widthp, char **swidep, int quotable)
 	    if (widthp)
 		*widthp = 6;
 	} else {
-	    strcpy(buf, nicechar((int)c));
+	    strcpy(buf, nicechar_sel((int)c, quotable));
 	    /*
 	     * There may be metafied characters from nicechar(),
 	     * so compute width and end position independently.
@@ -771,7 +771,7 @@ mod_export int is_wcs_nicechar(wchar_t c)
 	if (c == 0x7f || c == L'\n' || c == L'\t' || c < 0x20)
 	    return 1;
 	if (c >= 0x80) {
-	    return (c >= 0x100);
+	    return (c >= 0x100 || is_nicechar(c));
 	}
     }
     return 0;





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] E02 test failure / ztst *> issue
  2020-03-12  2:37 ` Jun T
@ 2020-03-12 17:31   ` dana
  2020-03-12 17:36     ` Peter Stephenson
  2020-03-13 10:08     ` Jun T
  0 siblings, 2 replies; 6+ messages in thread
From: dana @ 2020-03-12 17:31 UTC (permalink / raw)
  To: Jun T; +Cc: Zsh hackers list

On 11 Mar 2020, at 21:37, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> I noticed this on macOS few days ago or so, and was testing on other BSDs,
> and yes, the test fails (at least) on Free/Net/Open BSDs. It turned out
> that the problem is in zsh itself; a patch is at the end of this post.

Oh, i'd just assumed it was a normal platform/locale difference.

On Mojave, your patch fixes the test for me only if i manually set LC_CTYPE or
LC_ALL to C when running `make check` — which didn't have any effect before,
so it is definitely an improvement. If i run it normally (where i have
LC_CTYPE=en_GB.UTF-8), it still fails in the same way.

We could set LC_CTYPE=C in ztst, but that'd cause problems for some of the
multi-byte tests. So should we just set it for this one test? If i combine
this patch with yours, everything passes for me.

dana


diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
index a5a7bc55c..77088001d 100644
--- a/Test/E02xtrace.ztst
+++ b/Test/E02xtrace.ztst
@@ -147,22 +147,25 @@
 ?+(anon):0> true
 ?+fn:0> gn
 
-  test_cases=(
-      f            # baseline
-      foo-bar      # Dash
-      ヌ           # Meta (the UTF-8 representation of this character has an 0x83 byte)
-      \$\'ba\\0z\' # Nul, escaped as though by ${(qqqq)}
+  (
+    LC_ALL=C # Make `which` output consistent
+    test_cases=(
+        f            # baseline
+        foo-bar      # Dash
+        ヌ           # Meta (the UTF-8 representation of this character has an 0x83 byte)
+        \$\'ba\\0z\' # Nul, escaped as though by ${(qqqq)}
+    )
+    for 1 in "$test_cases[@]"; do
+      eval "
+        ${1}() {
+          ${1}() { echo inner }
+        }
+        functions -T ${1}
+        ${1}
+        which ${1}
+      "
+    done
   )
-  for 1 in "$test_cases[@]"; do
-    eval "
-      ${1}() {
-        ${1}() { echo inner }
-      }
-      functions -T ${1}
-      ${1}
-      which ${1}
-    "
-  done
 0:a function that redefines itself preserves tracing
 >f () {
 >  # traced


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] E02 test failure / ztst *> issue
  2020-03-12 17:31   ` dana
@ 2020-03-12 17:36     ` Peter Stephenson
  2020-03-13 10:08     ` Jun T
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Stephenson @ 2020-03-12 17:36 UTC (permalink / raw)
  To: zsh-workers

On Thu, 2020-03-12 at 12:31 -0500, dana wrote:
> We could set LC_CTYPE=C in ztst, but that'd cause problems for some of the
> multi-byte tests. So should we just set it for this one test? If i combine
> this patch with yours, everything passes for me.

Yes, I think that's reasonable.  It's just this one that's making some
assumption that things won't get *too* clever.

pws


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] E02 test failure / ztst *> issue
  2020-03-12 17:31   ` dana
  2020-03-12 17:36     ` Peter Stephenson
@ 2020-03-13 10:08     ` Jun T
  2020-03-13 13:52       ` dana
  1 sibling, 1 reply; 6+ messages in thread
From: Jun T @ 2020-03-13 10:08 UTC (permalink / raw)
  To: zsh-workers


> 2020/03/13 2:31, dana <dana@dana.is> wrote:
> 
> On 11 Mar 2020, at 21:37, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> 
> On Mojave, your patch fixes the test for me only if i manually set LC_CTYPE or
> LC_ALL to C when running `make check` — which didn't have any effect before,
> so it is definitely an improvement. If i run it normally (where i have
> LC_CTYPE=en_GB.UTF-8), it still fails in the same way.

ztst.zsh (lines 27-31) resets LC_* and LANG to C if they are non-empty.
So I was thinking tests were run under C locale by default ...
But now I've noticed LC_CTYPE is NOT reset here.

I guess you can avoid subushell by

LC_ALL=C which ${1}




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] E02 test failure / ztst *> issue
  2020-03-13 10:08     ` Jun T
@ 2020-03-13 13:52       ` dana
  0 siblings, 0 replies; 6+ messages in thread
From: dana @ 2020-03-13 13:52 UTC (permalink / raw)
  To: Jun T; +Cc: Zsh hackers list

On 13 Mar 2020, at 05:08, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> I guess you can avoid subushell by
> LC_ALL=C which ${1}

Yeah, and it's a cleaner diff. Committed like that, thanks

dana


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-13 13:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 21:15 [BUG] E02 test failure / ztst *> issue dana
2020-03-12  2:37 ` Jun T
2020-03-12 17:31   ` dana
2020-03-12 17:36     ` Peter Stephenson
2020-03-13 10:08     ` Jun T
2020-03-13 13:52       ` dana

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).