Change wording of EOF behaviour in stage_1

[WesleyAC]

While this is probably less technically correct, I was a bit confused by the original wording. I think people who are building a shell will implicitly assume the "repeat" part of it, and explicitly saying "exit" makes the behaviour that is expected make more sense to me.

[tokenrove]

This seems reasonable. I'll merge, if you don't mind making the following modifications: preserve the period at the end of the list; include the rationale in your PR description in the body of the comment, since I think it gives some useful context. Thanks!

(Also, it would be nice to give a clearer error from validate when the shell fails to exit; maybe harness should print "sending ^D to exit your shell" before it sends ^D, so you see that's the last thing that happened before it died.)

[WesleyAC]

Sounds good, will do 👍

[WesleyAC]

Do you mind if I overwrite the original commit to include the description?

[tokenrove]

Overwriting the original commit is exactly the correct behavior. This change should only be one commit.

[WesleyAC]

So, I looked into providing some feedback if the test times out on ^D. I was planning to do this by adding a puts "Sending ^D to shell to exit" to harness.tcl, and adding a message when the test fails due to a timeout.

It seems like right now, the return code for timeout is 1 when the script times out, and it passes through the code otherwise. However, the return code for the first test failing is also 1, thus making it impossible to differentiate.

It seems like the simplest solution to this is to change the return code of timeout to be something unlikely to occur in the test script, but that's not a very good solution, since it doesn't actually fix the problem. The other solution I could think of is to change harness.tcl to not return the the line number, since it isn't currently used (AFAICT), and instead have it return some non-zero, non-one value.

What do you think I should do about this?

[tokenrove]

I think it would be ok, in this case, to make timeout itself print a message to stdout so it gets displayed in the log on failure (possibly making it red to be consistent with the not_ok output of harness). This wouldn't be good in general, but here, we only use this tool for this purpose so it should be fine. Differentiating error codes would also work in practice, but is probably an unnecessary complication if we only need to provide the information that we timed out.

[tokenrove]

Oh, and if you do make this patch, I would write the message as (sending ^D to exit shell) as a parenthetical to reduce its visual distractiveness; or, I guess, we could actually represent this as an implicit:

→ ^D
☠ 0

and treat it as such. Heck, now that I think about it, I wonder if I shouldn't even make it explicit in the tests and potentially require ☠ at the end of every script.