impl Display for Expression

[Boscop]

Please impl Display for Expression so that commands can be printed without the awkward debug formatting :)

[oconnor663]

What do you think the display implementation should be? I've hesitated about doing this in the past, because I don't think there's one obvious way to do it.

[oconnor663]

Another practical issue is that internal OsStrings would probably need to be printed in a lossy way? I wonder if it would make more sense to expose the internal strings inside an expression to the caller directly, so that the caller could decide what to do with it all. But then again, that would be an even more complicated API. (Now you're talking about traversing a tree. Maybe using callbacks? Seems like overkill.)

[Boscop]

What do you think the display implementation should be? I've hesitated about doing this in the past, because I don't think there's one obvious way to do it.

The main question is how to escape spaces, quotes etc, right?
E.g. in chrome dev tools Network tab you can copy a request for curl in cmd format (for windows) or bash (for linux).
But since there can only be one Display impl, it could make sense to default it to the OS it's running on.
Or maybe it's better to not have a Display impl but two methods: format_cmd and format_bash.

My main use case for wanting this feature is for logging the commands that my tool is executing in a human-readable manner. I don't "need" perfect copy-pastability of printed commands, but it would be nice to have.
I'd already by happy to have it more human-readable than the Debug impl, so without the quotes around all the constituents.

[Alxandr]

I agree. Being able to log the command before it's executed would be very useful.

[oconnor663]

Could you tell me a little more about your use cases? Another thought that I've had here is that the majority of callers construct an Expression themselves, rather than getting one passed in from somewhere else, so needing to get the strings back out should be rare. But maybe that's not the right way for to think about it?

[Alxandr]

It's mainly just a convenience thing. I'm writing a xtask, and would like to log the commands that run. Now, there are a few ways I could go about this. I could probably create my own macro, and have that log and then create an Expression. However, that has some disadvantages (I'm now logging creation, not execution). Or I could wrap the expression in something that logs when it runs, but yeah, that's not really a good look either I think. Currently I'm exploring doing a before_spawn that does logging. I don't mind my arguments being wrapped in unnecessary quotes or anything like that. It's just for showing what the program is doing after all.

[Alxandr]

Example output from something I threw together:

[oconnor663]

Maybe it would make sense to have a method like to_string_lossy (name TBD). It's behavior is something like "lossily convert every path/arg into a String, concatenate them all with spaces, and concatenace every expression in a pipeline with |." The idea would be that this is what you'd like to log almost all the time, so it's nice to have, but we don't need to make it a comprehensive (or even stable) representation of the whole expression. For things like redirects to files, we can just print > [file] and not worry about it.

I think my litmus test for "not worth getting right" here is quoting/escaping whitespace. If we can offer a function that's doesn't even try to quote whitespace for you (and makes lots of other compromises like that one), it'll be dead simple to implement, and hopefully useful most of the time. How does that sound?

[Alxandr]

What you might want to do is create an iterator over some sort of "component" enum instead. to_lossy_parts()? Something that one could .map(ToString::to_string) and then join on space. I'm also of the option that quoting arguments even when they don't need it is better default than not quoting, cause the latter is more likely to be correct. I also did not do any sort of escaping of quotes or anything like that.

[Alxandr]

Also, if wanted, I can probably drum up a PR.

[Alxandr]

Or, better yet; to_parts().map(PartTrait::to_string_lossy).

[Alxandr]

@oconnor663 would you want a PR for this? I could look into it if wanted - but I don't want to start working on this if you think it's not something you want in the project.

[oconnor663]

The way I'm standing at the moment, I'd love to see a PR on a simpler "to_string_lossy" or something like that, but I'm skeptical about exposing a more complex, structured API for inspection. I start thinking about cases like this:

(cat xyz | sed s/a/b/) 2> stderr.txt | sed s/b/c/

As in "you've got three programs in a pipeline, and the stderrs of the first two have been jointly redirected to a file". Duct can execute that. But can an iterator of components faithfully describe it? It seems like you'd need a tree, which is of course what Duct uses internally. The prospect of exposing the entire internal tree (or some parallel structure) through the public API seems like much too much added complexity, for what would really be a niche debugging feature.

On the other hand, maybe I'm blowing it out of proportion, and you've thought of some sweet spot in between. What do you think?

[Alxandr]

I'm not thinking of structure, just display tokens. But tokens, so they can be formatted as you please, with a built in simple default. So parts should probably be named DisplayPart or somesuch. No intent to make it parseable (after all, you already lose env vars and similar by stringifying).