_es_ doesn't print non-printable characters correctly

[jpco]

For example:

; local (x = \201) {echo $#x; echo $x; var x}
1

x = 

I think the shell used to fall back to the backslash-escaped codes for nonprintable characters, but that seems to be broken now.

This is an obvious candidate for some test coverage.

[memreflect]

For example:

; local (x = \201) {echo $#x; echo $x; var x}
1

x = 

I think the shell used to fall back to the backslash-escaped codes for nonprintable characters, but that seems to be broken now.

What you want to look at is the Sconv() function used for the %S conversion specification.
For backslash escapes to occur, quoting must take place, and that only happens under one of three specific conditions:

1. %#S was used. ❌
2. The string argument is an empty string. ❌
3. At least one character, c, in the string satisfies nw[c] != 0 || c == '@'. ❌
   • Note: nw is defined in token.c and covers most of the shell's special characters like !, ', ~, \n, etc.

\201 does not satisfy any of those requirements, so quoting does not occur, resulting in printing the raw bytes of the string rather than backslash escapes for non-printable characters.

What you seem to be asking for is isprint(c) to be invoked even when quoting will not occur:

# Current behavior
; local (x = a%b a\201b a\tb a@b) { var x }
x = (
  a%b        # !nw['%']                      => no quoting, no backslash escapes
  ab         # !nw['\201']                   => no quoting, no backslash escapes
  'a'^\t^'b' # nw['\t'] && !isprint('\t')    => quoting, backslash escapes
  'a@b'      # '@' in string && isprint('@') => quoting, no backslash escapes
)

# Proposed behavior
; local (x = a%b a\201b a\tb a@b) { var x }
x = (
  a%b        # !nw['%'] && isprint('%')        => no quoting, no backslash escapes
  a^\201^b   # !nw['\201'] && !isprint('\201') => no quoting, backslash escapes
  'a'^\t^'b' # nw['\t'] && !isprint('\t')      => quoting, backslash escapes
  'a@b'      # '@' in string && isprint('@')   => quoting, no backslash escapes
)

Another requirement to making this work is the fact that ISO C and POSIX both require the LC_CTYPE locale category to be set correctly before isprint() is used.
Es doesn't contain any setlocale() invocations, so reliable usage of isprint() is restricted to the POSIX portable character set.

Another wrinkle involves multi-byte locales like those that use UTF-8, GB 18030, EUC-KR, etc., which means isprint() isn't enough in those locales.
Instead, one first needs to use mbtowc() or mbrtowc() to convert from a (possibly) multi-byte sequence with a maximum length of MB_CUR_MAX to a wchar_t value, after which that value may be passed as the argument to iswprint().
Most modern Linuxen, the BSDs, and Solaris/Illumos use a 32-bit wchar_t, which is more than capable of encoding U+10FFFF.
Unfortunately, wchar_t is allowed to be as narrow as 8 bits, so configure checks are needed to determine whether to enable support for mb[r]towc() and iswprint() in the implementation of Sconv()—contributions from users of environments lacking a 32-bit wchar_t are definitely welcome here.
MB_CUR_MAX == 1 || !WIDE_SCONV means the simpler isprint() gets used, avoiding the issue with wchar_t entirely, and multi-byte locales would be treated as single-byte locales by isprint() if WIDE_SCONV is false.

[jpco]

Wide-character support more broadly would also involve changes to $&split and $&fsplit -- for example, ; let (x = <={%fsplit '' 字}) echo $#x returns 3, which is not ideal.

[memreflect]

Wide-character support more broadly would also involve changes to $&split and $&fsplit -- for example, ; let (x = <={%fsplit '' 字}) echo $#x returns 3, which is not ideal.

And wide-character support doesn't automatically mean full Unicode support either.
For example, the NFC normalization form in which characters appear in precomposed forms as much as possible (e.g. U+00C0 À rather than U+0065 U+0300) sometimes still includes code points for combining characters.
Is splitting being performed on, say, the full multi-code-point glyph U+0105 U+0302 U+0301, or is each individual code point being used for splitting, meaning it is impossible to split with that entire glyph?
Also, someone can throw a \xBF byte in the middle of a string, which isn't necessarily the same as U+00BF, depending on the locale; should that be "fixed" like in Python 3 (despite the fact that \u00bf can be used in strings) or should it be left as a single byte like Go chose to do?

That Unicode discussion is one that i feel is necessary at some point, but i think fixing this issue by adjusting how bytes are emitted by Sconv() is probably enough for now.
echo 字 already works fine, so it's mainly just stuff like local (x = 字) {var x} and let (x=字) {whatis @ {}} that seem to have issues, which all leads back to Sconv().

[jpco]

I agree, a piecemeal approach is good -- I just wanted to note that multi-byte characters mess with $&split and $&fsplit because I don't have a good enough memory to keep track of it otherwise :)

Your Sconv() change looks pretty good to me -- one question though, you'd know this better than I, is the following output portable/reasonable these days? I'm not sure what kind of "environment protection" we need to worry about (I don't even know if PROTECT_ENV and Fconv() are really necessary any more.)

; local (λ = λ) {echo $'λ'; var λ; env | grep '=λ$'}
λ
λ = λ
__ce__bb=λ

[memreflect]

Your Sconv() change looks pretty good to me -- one question though, you'd know this better than I, is the following output portable/reasonable these days? I'm not sure what kind of "environment protection" we need to worry about (I don't even know if PROTECT_ENV and Fconv() are really necessary any more.)

; local (λ = λ) {echo $'λ'; var λ; env | grep '=λ$'}
λ
λ = λ
__ce__bb=λ

What happens when PROTECT_ENV is dropped?
Are = and _ in variable names exported as __3d and __5f, or is = forbidden in variable names while _ is exported as _?
I prefer the latter as it eliminates the need for Fconv() and Nconv() entirely.

Otherwise, Fconv() would benefit from the same change as Sconv().

Btw, allprintable() in dump.c uses isprint() as well.
And the code for $&limit as well as iscounting() in var.c both use isdigit().
I might have just turned a small hill into a huge mountain by mentioning UTF-8...

Whatever happens with PROTECT_ENV, i'm entirely on-board with allowing raw non-ASCII variable names to exported as-is.
When PROTECT_ENV is enabled, how environment variables with non-ASCII names are exported depends upon isalpha()/isalnum(), which means the locale's LC_CTYPE category should affect things.
If λ is encoded as UTF-8 and the shell uses UTF-8, then __ce__bb is exported unless Fconv() is adjusted to handle multibyte characters like Sconv().
If λ is encoded as ISO 8859-7 and the shell uses ISO 8859-7, then λ (\xeb) is exported.
If λ is encoded as ISO 8859-7 and the shell uses ASCII, then __eb is exported.
So if you're writing a bash script to interact with these "safe" names, which exported name do you use?

That said, when λ is exported as \xce\xbb (UTF-8), that variable is not accessible for reading with $λ or writing with λ=foo in bash or zsh, and dash doesn't even import the UTF-8 byte sequence.
Ksh93 and fish, however, have no problem if the locale's character encoding is UTF-8.
(Technically, zsh supports λ=foo and $λ if UTF-8 is used, but the λ used by zsh is different from the one exported by es, and zsh doesn't let you export its λ variable.)

On the other hand, the inaccessibility of UTF-8 variables in other shells might be a good thing.
I mean, do you really want to use bash or whatever to mess with the value of (foo bar baz^\017) in the first place?
For context, that value is exported as the bash string $'foo\017bar\017baz\016\017'.
This makes me wonder if perhaps ksh93 is right to only export variables with the first item in a list/array.
Zsh and bash don't even export variables with list/array type, and elvish's set-env only supports strings currently.

[jpco]

I prefer the latter as it eliminates the need for Fconv() and Nconv() entirely.

That's what seems to happen now.

On the other hand, the inaccessibility of UTF-8 variables in other shells might be a good thing.

I don't mind that, either. Really, I don't think %F was ever intended for sending variables from es to other shells; I think it's for sending variables from es to other es processes in a reasonably bulletproof way.

After some testing, I think the major issue with getting rid of it is that, without PROTECT_ENV, you get the following:

; fn x {echo function X}
; dash
$ es
; x
x: No such file or directory

I expect this is the exact kind of "brain-dead" behavior the authors are alluding to in the comments for Fconv in both rc and es. I don't see any other major shells that have this problem

; local (fn x {echo function X}) for (sh = dash bash ksh zsh fish tcsh elvish) {echo -n $sh\t; $sh -c './es -c x'}
dash	x: No such file or directory
bash	function X
ksh	function X
zsh	function X
fish	function X
tcsh	function X
elvish	function X

but dash is a reasonably important shell given its place as /bin/sh on Debian and friends.

Really, the conservative approach would be not just to keep Fconv and friends, but also change the isalpha() and isalnum() calls there to a strictly ASCII-only version, since we're targeting the POSIX sh variable name constraints, which demand ASCII.

[jpco]

Here's an interesting prior bug post from rc. rakitzis/rc#49

[memreflect]

; fn x {echo function X}
; dash
$ es
; x
x: No such file or directory

I expect this is the exact kind of "brain-dead" behavior the authors are alluding to in the comments for Fconv in both rc and es. I don't see any other major shells that have this problem

...

but dash is a reasonably important shell given its place as /bin/sh on Debian and friends.

Yeah, that's really the only shell i feel warrants special concern here.

Really, the conservative approach would be not just to keep Fconv and friends, but also change the isalpha() and isalnum() calls there to a strictly ASCII-only version, since we're targeting the POSIX sh variable name constraints, which demand ASCII.

That's easy enough to do, and it's probably the best choice given how dash behaves despite other shells supporting things in UTF-8 locales.

Here's an interesting prior bug post from rc. rakitzis/rc#49

That is definitely interesting, especially the comment about how Python decided to handle things in PEP 538 and PEP 540, though a "UTF-8 mode" for es is probably overkill.
The final comments that basically state rc is mostly locale-ignorant because it doesn't need to deal with them is an interesting one as well, but it makes sense:

; λ^A = (99 100)    # ^A is Ctrl-A (U+0001)
; whatis λ^A
λ=(99 100)

Basically the same as this issue in es, except rc decided to just emit the bytes, printable or not.
Do we want to do the same in es?
This would be bring es in a bit closer to rc behavior and definitely falls under "Check rc changes to see if any should be ported".

[jpco]

Do we want to do the same in es?

It's good evidence that not worrying about locale too much can be okay most of the time. A couple reservations/thoughts on that, though:

1. Rc does still call setlocale(LC_CTYPE, ""), so they're not completely ignorant of locale. (and given rakitzis/rc@864cac5, mishandling locale has bitten them in the past). So we should still fix Locale-related variables have no effect #177 (and I guess make sure we (re-)call setlocale() after .esrc? Or better yet maybe add a $&setlocale primitive? This should move to the discussion in Locale-related variables have no effect #177 though.)

2. Rc doesn't call any of the locale-ish is*() functions, or strto*(); we do. In some places we don't actually need to -- we could skip our current isdigit calls by just trying to parse strings into integers and handling the failures. But in some cases like tokenizing \201 it's more of a consideration. We should go through our existing locale-aware function calls and get more explicit with where we do or don't want to have that locale awareness.

3. Es makes it easier than rc to input characters that can't be output with backslashes:

; echo \xCE\xBB
λ

That's probably fine, though. Es is all about giving users all the footguns they could possibly need.

4. Es tries to provide a stronger guarantee than rc that var x produces output that can be fed back in as input; for example

[es]; var blah
blah =
[rc]; whatis blah
blah not found

This breaks with a case like your λ^A variable above, which is less than ideal (but may be too narrow a corner case to care about).

5. Rc scripts don't make use of %fsplit and %flatten quite like es' do, and I still feel like this

; echo <={%flatten l <={%fsplit λ 'heλλo worλd'}}
hellllo worlld

is more or less just a bug.

[memreflect]

2. Rc doesn't call any of the locale-ish is*() functions, or strto*(); we do. In some places we don't actually need to -- we could skip our current isdigit calls by just trying to parse strings into integers and handling the failures. But in some cases like tokenizing \201 it's more of a consideration. We should go through our existing locale-aware function calls and get more explicit with where we do or don't want to have that locale awareness.

I agree that's a good place to start.
Like you said, Fconv() really doesn't need to use the locale-aware is*() functions when ASCII-compatible export names are being targeted.
I have updated my Sconv branch to replace isalpha() and isalnum() in Fconv() with a lookup table like the one used by the lexer.
We can't reuse the existing dnw table because symbols like %*- are valid variable names according to that table.

3. Es makes it easier than rc to input characters that can't be output with backslashes:

   ; echo \xCE\xBB
   λ
   

   That's probably fine, though. Es is all about giving users all the footguns they could possibly need.

4. Es tries to provide a stronger guarantee than rc that var x produces output that can be fed back in as input; for example

   [es]; var blah
   blah =
   [rc]; whatis blah
   blah not found
   

   This breaks with a case like your λ^A variable above, which is less than ideal (but may be too narrow a corner case to care about).

I definitely see blah^\1 being more useful, so we should definitely try to move forward with something like my Sconv branch to help $&var out with pretty-printing names.

5. Rc scripts don't make use of %fsplit and %flatten quite like es' do, and I still feel like this

   ; echo <={%flatten γ <={%fsplit λ 'heλλo worλd'}}
   heγγγγo worγγd
   

   is more or less just a bug.

That's the result of es being punting on locale issues like rc.
It's probably better to stick with the simpler "split on each code point" behavior whenever that gets done.
Improved Unicode support where, say, ΐ (U+03B9 U+0308 U+0301, or the discouraged U+03B9 U+0344) is considered a single character for splitting purposes can come later.

As for implementation, whatever %fsplit uses would need to try to convert to some wide character type, whether wchar_t, char16_t or char32_t, and hope for the best, whether WIDE_SCONV is defined or not.
Because of that fact, there's not much of a point to WIDE_SCONV despite my intentions, so i have removed it; HAVE_MBRTOWC being undefined is enough to avoid using mbrtowc().
In that case, the existing byte-based implementation of %fsplit or whatever would be used (when i get around to implementing such a thing).
This is different for the renamed Sconvwc()→chisprint(), which can simply defer to isprint() if MB_CUR_MAX == 1 to sidestep the wchar_t size issue entirely.

There are options like the char32_t type defined in C11 and newer that i can consider if they're available (and some implementations like glibc and FreeBSD's libc allow the API to be used outside C11 mode) for conversion purposes, but that's going to be a separate issue from the isprint()/iswprint() issue because there is no isc32print() or whatever; such a function would need to be implemented manually by dealing with Unicode categories, which requires some research, else a library like ICU (International Components for Unicode) would need to be considered to save time.

[jpco]

Returning to this after a while: this discussion expanded to a lot of stuff, which is good, but I want to try to break it down to more actionable bits. I am going to write these items from the user perspective, not considering what kinds of overlaps there might be in implementing fixes.

A couple notes: I do think echo should just print characters "raw". I also think we should stick to only supporting ASCII-compatible encodings.

Given those things, in my mind, we have the following issues:

1. $&var does not format ASCII control characters in a way that the shell can correctly read back in, which is the whole point of $&var. (Control characters need to be escaped in any printing situation designed for the shell to read back in.)
2. $&var is not locale-aware enough to escape 8-bit characters when the locale is not 8-bit capable. This is related to Locale-related variables have no effect #177. If we do have an 8-bit capable locale, we can just pass those high-bit-set characters through and say it's probably fine. This is, I think, the same punt that rc does.
3. There is some latent inconsistent internal behavior due to isalnum() and other locale-dependent functions that should be flattened out to ASCII-specific versions.
4. Wide character (or, mostly-equivalently, string) $ifs handling in %flatten and friends would be nice.
5. Like $&var, anything printed within %closure should be correct when read back in again -- this might be potentially covered by Parsing closures containing \e is generally broken #242, depending on how we want to scope these issues.
6. Parsing %closure also causes crashes on lots of mildly-weird input, including cases of special-character handling, but that should definitely be covered by Parsing closures containing \e is generally broken #242.

What you've done in your Sconv branch certainly covers special character handling like (1)-(3), but it's quite a bit more sophisticated than seems necessary to me to just cover what I perceive to be the core set of problems. That said, are very much a step ahead of me on understanding the finer points of these topics -- am I underestimating the complexity here? (Probably!)