sh: implement PS1 \D to print current time

[pstef]

Summary

\D{format} yields the result of calling strftime(3) with the provided
format and the current time.

When PS4 can use this, it will enable us to easily generate timestamps
when tracing script execution.

[pstef]

Place the result of strftime() in the right place instead of always at the beginning of the buffer.
Simplify conditional early exit.

[pstef]

Remove debugging changes not meant for review.

[pstef]

Handle rare cases the way bash does it.

[pstef]

Simplify and add comments.

[jilles]

A little unfortunate to have these strange backslash sequences spread out further, but it's nothing that bash doesn't do.

bin/sh/eval.c

764

This order of operations and lack of escaping means that a parameter expansion in a directory name (\w) will be expanded. If expandstr is extended to PS1 and PS2 and with command substitution, this may lead to command execution from changing into a weirdly-named directory.

I haven't thought of a good solution here yet, but this one is not the right one.

bin/sh/sh.1

1426–1429

and with the current time

1430

Brackets are [].

1431

Perhaps "Empty" is clearer.

[pstef]

A little unfortunate to have these strange backslash sequences spread out further, but it's nothing that bash doesn't do.

In a way, this has nothing to do with bash. I want sh PS4 to be able to generate some kind of timestamps when executing a script with tracing on. The way bash does it is just a convention which I think is not worth diverging from.

bin/sh/eval.c

764

This order of operations and lack of escaping means that a parameter expansion in a directory name (\w) will be expanded.

Isn't this true already? Currently we call expandstr() on ps4val() and getprompt() will not be much different.

[pstef]

bin/sh/eval.c

764

I now understand what you mean. Formatting sequences like \w are first translated to, for example, the CWD, and then subject to parameter expansion. So if CWD is /usr/$X, \w is translated to that and then the $X is expanded.

[pstef]

bin/sh/eval.c

764

Perhaps the solution is to escape all $ at the end of getprompt() and then apply expandstr() to both PS1 and PS2?

[jilles]

In D35840#814030, @pstef wrote:

In a way, this has nothing to do with bash. I want sh PS4 to be able to generate some kind of timestamps when executing a script with tracing on. The way bash does it is just a convention which I think is not worth diverging from.

The more POSIX-like way would be to add some level of command substitution to expandstr() (ensuring set -x doesn't apply to the nested command) and put in something like $(date +%s) in PS4. However, this is slower and harder to implement. It is also more flexible.

[jilles]

bin/sh/eval.c

764

That would be all $ and ` generated by backslash sequences.

Actually applying expandstr() to PS1 and PS2 will require some way to deal with re-entering the parser, so it shouldn't be done as part of this review.

[pstef]

Drop PS4 support and update the manual page changes according to feedback.

[pauamma_gundo.com]

bin/sh/sh.1

1429

[jilles]

Code is OK like this.

bin/sh/sh.1

1426–1430

Perhaps this could be formulated a bit differently so no prior knowledge of strftime is assumed: "The current time formatted using the .Xr strftime 3 format .Ar format."

[pstef]

In this iteration I tried to address all new feedback.
I also was annoyed by the current formatting of the file, so this is a diff against D37926 where I reduce indentation.

[pstef]

When I was looking at this code today, I thought that the unconditional memccpy() is a bit wasteful in the \D{} case, because the one copied byte is a special case and when it's noticed, the copied byte is overwriten by strcpy() with "%X".
There is little to be gained in either performance or legibility of the code, but when I rewrote this locally to be less wasteful, it didn't look much worse, nor did it look much better.
Opinions?

[pstef]

Simplify the code part a bit.
Also improve comments and some variable names.

[pstef]

Minutes after posting the previous version, I thought about handling the rare corner case when strftime() returns 0.

[pstef]

Back out the previous update.
The -1 was there for the i++ that the loop always performs, so it's needed regardless of whether strftime() returned 0 or more.
Split the calculation that was there before into two parts, one of which just decrements i and makes a comment on why it's done.