elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.67k stars 299 forks source link

Too many ways to output objects: `put`, `echo`, `print`, `repr`, `pprint`, and now `show` #1065

Open krader1961 opened 4 years ago

krader1961 commented 4 years ago

Commit e9b40358 moved exc:show to builtin:show as part of overhauling how exceptions are manifested. However, the show command only handles exception objects and currently results in a panic if handed a $ok object. At present the sole purpose of builtin:show is to pretty-print an exception object. It seems to me it would be better to augment the pprint builtin to do the same thing when handed an exception object rather than introduce a new builtin. Do we really need show in addition to put, echo, print, repr and pprint?

krader1961 commented 4 years ago

In PR #1063 @xiaq pointed out that show colorizes its output. Also, pprint is just a variant repr that uses indentation for nested values. And, of course, print is just a variant of echo that doesn't include a terminating newline.

FWIW, my preference is to eliminate pprint and add a &pretty option to repr. Similarly, eliminate print and add a &no-newline (or similar) option to echo. It seems to me neither pprint or print are used frequently enough to justify their existence independent of their counterparts and it would be cognitively simpler to replace [put, echo, print, repr, pprint] with [put, echo, repr].

Note that the show command actually does more than colorize its output. In the case of exception objects it includes context that is missing from the output of echo or repr:

> fn wtf { e:wtf }
> x = ?(wtf)
> echo $x
[&reason=<unknown exec: "wtf": executable file not found in $PATH>]
> repr $x
[&reason=<unknown exec: "wtf": executable file not found in $PATH>]
> show $x
Exception: exec: "wtf": executable file not found in $PATH
Traceback:
  [tty 7], line 1:
    fn wtf { e:wtf }
  [tty 8], line 1:
    x = ?(wtf)

This raises a couple of questions. Should repr include the traceback context? Should repr have a &colorize option in addition to a &pretty option? Should pretty-printing exceptions use the show representation rather than treating the exception object as a pseudo-map? Note the current repr output of an exception object cannot be used to create an exception object. Which makes the map like output for an exception object not particularly useful.

xiaq commented 4 years ago

repr should include the traceback context, it's just that I haven't found a satisfying way to expose it.

Each entry of the traceback is a diag.Context, which contains the full code of the originating source:

https://github.com/elves/elvish/blob/a568bf589faaafb22caf61f2eb02dfb37b3ab80d/pkg/diag/context.go#L16

If this is exposed as is, print the repr of an exception will result in a really long string. But I haven't found a satisfying way to avoid that.

xiaq commented 4 years ago

Note that the show command actually does more than colorize its output. In the case of exception objects it includes context that is missing from the output of echo or repr:

Yes, in this aspect show is different from all the other commands in that it shows a value in a well-presented way, instead of a simple string (echo, print) or the internal structure (repr, pprint).

Should repr have a &colorize option in addition to a &pretty option?

Maybe. I am not sure.

Should pretty-printing exceptions use the show representation rather than treating the exception object as a pseudo-map?

No, pprint is in the same family as repr.

Note the current repr output of an exception object cannot be used to create an exception object. Which makes the map like output for an exception object not particularly useful.

It reveals the internal structure and is an introspection device.

Regarding merging the commands, I'll put some polls here.

xiaq commented 4 years ago

Poll 1:

The print command is the same as echo, except that it doesn't add a trailing newline. We should remove print and replace it with...

grizzlysmit commented 4 years ago

echo &end="\n" Would be fine

krader1961 commented 4 years ago

The echo &end="\n" seems appealing at first glance because it appears to be the most general solution. The problem is that it makes it harder to output text without a newline. The echo &n solution makes it slightly easier to output text without a newline and substitute whatever terminator you want; e.g., echo &n $v":". I think that strikes a better balance between handling the two most common cases while still making it possible to handle the less common case of a terminator that is not a newline.

A weak argument is that echo &n is going to be easier for most people to remember since it mimics echo -n from other shells.

krader1961 commented 4 years ago

Historical footnote: The reason shells like bash, ksh and zsh have both echo and print is that the POSIX standard for echo makes it unusable except for outputting text that does not begin with a hyphen and always includes a terminating newline. The POSIX standard does not mandate that echo -n yes suppress the newline. Some implementation do so while others output -n yes. Let's not repeat the mistakes of the past :-)

kolbycrouch commented 4 years ago

Print could go. Can always do fn print [@a]{ echo &end='' $@a }

zzamboni commented 4 years ago

A rough statistic from all my Elvish module repos:

[~/.e/l/g/zzamboni]─[⎇ master]─[●]=> egrep -w print (find . -name '*.elv') | wc -l
27
[~/.e/l/g/zzamboni]─[⎇ master]─[●]=> egrep -w echo (find . -name '*.elv') | wc -l
62
[~/.e/l/g/zzamboni]─[⎇ master]─[●]=> egrep -w pprint (find . -name '*.elv') | wc -l
6

I could live with changing print to echo &n. The &end option could be useful for sure, but I would still keep &n as a shortcut for &end="" given it's shorter to type and the historical relationship with echo -n.

About the others: I use pprint a lot interactively, so my vote would be to keep it. Honestly I didn't even know about repr until this discussion started! My vote would be to leave pprint and give it a &no-indent option (or something) to produce the output like repr does today. But that's just me :)

About show I don't have a strong feeling given it's quite new.

krader1961 commented 4 years ago

FWIW, Like @zzamboni I too have never used repr but do use pprint, interactively, on a regular basis when trying to understand elvish behavior. Note the "interactively" in the preceding sentence. I'm sure I've used it non-interactively but only the interactive use stands out in my mind. And even the non-interactive uses would have been solely for debugging. Which suggests that pprint would benefit from colorizing its output like show does for exception objects when the output is to a terminal (or a hypothetical &colorize option is used).

Despite my initial recommendation I now think it makes more sense to replace repr with pprint &simple. That is, keep pprint and eliminate repr. Alternatively, make repr a synonym for pprint but add a &simple option to get the current behavior. Since the probability is zero that any script is using repr other than for debugging it should be safe to make that backward incompatible change without a deprecation period.

krader1961 commented 4 years ago

Since the probability is zero that any script is using repr....

I know I simply assumed what I wanted to be true without actually providing data to support that belief. However, looking at @zzamboni's modules it appears all the uses of pprint are solely for debugging the code and there are no uses of repr. And while there might be elvish code that uses repr as a mechanism for serializing data into a form that can be read by the Elvish parser that seems unlikely.

I also did this GitHub code search for uses of pprint in elvish files and all of them appear to be solely for debugging. The same search for [repr]{https://github.com/search?q=extension%3Aelv+repr&type=code} returns no results other than a single use in a comment. I realize that there are Elvish uses not captured by GitHub. None of my elvish files are currently visible on GitHub. Nonetheless, it seems exceedingly unlikely that either repr or pprint are being used in a manner likely to be broken by changing their current behavior. Since we know pprint is being used, and there are no easily found uses of repr (and it's unlikely there are any), it makes more sense to retire repr and retain pprint with the addition of a &simple option to replicate the repr behavior.