add support for dumping motd on session creation #6
Conversation
ethanpailes
force-pushed
the
motd
branch
2 times, most recently
from
d62e8f9 to
2044cdf
Compare
|
It turns out |
ethanpailes
force-pushed
the
motd
branch
19 times, most recently
from
fdb559f to
2517e9f
Compare
|
termini turns out to be slightly annoying to integrate since it does not have a clone derivation: pascalkuthe/termini#3 |
ethanpailes
force-pushed
the
motd
branch
2 times, most recently
from
bc11d20 to
aa0d5a0
Compare
| @@ -1,20 +1,14 @@ | |||
| [advisories] | |||
There was a problem hiding this comment.
You need to add version = 2, otherwise you get all the bad old defaults.
Also, you probably want to add at the top:
[graph]
all-features = true
libshpool/src/daemon/prompt.rs
Outdated
| "# | ||
| ) | ||
| } else { | ||
| return Err(anyhow!("don't know how to inject a prefix for shell '{}'", shell)); | ||
| }; | ||
|
|
||
| if needs_default_term { | ||
| script.push_str("\nclear\n"); |
There was a problem hiding this comment.
Are the if/else blocks swapped? Shouldn't you set TERM if needs_default_term (term.is_none())?
There was a problem hiding this comment.
Oh yeah, you're right, thanks
libshpool/src/daemon/shell.rs
Outdated
| if let ClientConnectionMsg::New(conn) = &client_conn { | ||
| let chunk = | ||
| protocol::Chunk { kind: protocol::ChunkKind::Data, buf: &buf[..len] }; | ||
| let mut s = conn.sink.lock().unwrap(); | ||
| let write_result = chunk.write_to(&mut *s).and_then(|_| s.flush()); | ||
| if let Err(err) = write_result { | ||
| info!("client_stream write err, assuming hangup: {:?}", err); | ||
| info!("client_stream write err (2), assuming hangup: {:?}", err); |
There was a problem hiding this comment.
It was to differentiate between identical log lines. I've tweaked the text for the above line so I can remove the numbers.
libshpool/src/daemon/show_motd.rs
Outdated
|
|
||
| /// Showers know how to show the message of the day. | ||
| #[derive(Debug, Clone)] | ||
| pub struct Shower { |
There was a problem hiding this comment.
WDYT about "DailyMessenger" or something like that? I can't not read this is the water thing. Every. Single. Time.
There was a problem hiding this comment.
Oh yeah I like that. I didn't like Shower at all but wasn't quite sure what else to call it.
This patch adds support for one of the two motd modes that I plan to support. In this basic "dump" mode, we only display the message of the day when a user first creates a session. Since we are doing it right at the beginning, we can be confident we won't be mangling the restore buffer. Directly injecting the message of the day into the output stream is much more fraught on reconnect. I was hoping that this would be a simple change, but it would up being quite a bit more involved than I had hoped. The main reason for this is the interaction between the prompt prefix injection and message of the day injection. We need to make sure that motd injection happens after the prompt prefix shell code has finished executing so that it does not get clobbered, but unfortunately this is not as easy as just doing one after the other. With the naive approach there is a race condition where first we write the prefix injection shell code to the shell process, then we write out the message of the day, then the shell finishes processing the shell code and issues the terminal reset code emitted by the `clear` command at the end of the prompt prefix shell code. To deal with this, I started scanning for the control code emitted by `clear` in the output stream. I was able to re-use the efficient trie I wrote for the keybindings engine to do this. This addresses the first part of #5, but the issue is not resolved yet. I also realized that two different config variables to control this behavior leaves too much room for weird states. In particular, I worried about doing a direct dump during reattach. I decided it is better to do everything via one variable, where the mode implies when we actually show the motd.
This patch adds support for one of the two motd modes that I plan to support. In this basic "dump" mode, we only display the message of the day when a user first creates a session. Since we are doing it right at the beginning, we can be confident we won't be
mangling the restore buffer. Directly injecting the message of the day into the output stream is much more fraught on reconnect.
I was hoping that this would be a simple change, but it would up being quite a bit more involved than I had hoped. The main reason for this is the interaction between the prompt prefix injection and message of the day injection. We need to make sure that motd injection happens after the prompt prefix shell code has finished executing so that it does not get clobbered, but unfortunately this is not as easy as just doing one after the other.
With the naive approach there is a race condition
where first we write the prefix injection shell code to the shell process, then we write out the message of the day, then the shell finishes processing the shell code and issues the terminal reset code emitted by the
clearcommand at the end of the prompt prefix shell code.To deal with this, I started scanning for the control code emitted by
clearin the output stream. I was able to re-use the efficient trie I wrote for thekeybindings engine to do this.
This addresses the first part of #5, but the issue is not resolved yet. I also realized that two different config variables to control this behavior leaves too much room for weird states. In particular, I worried about doing a direct dump during reattach. I decided it is better to do everything via one variable, where the mode implies when we actually show the motd.