-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support speaker notes #389
base: master
Are you sure you want to change the base?
Changes from 3 commits
de42655
fd6d57b
3151fbd
cbadf07
93a0dcb
22d70f7
4c7726e
df051d3
81cfbbc
4416af8
5f781ac
0fd812e
0f83b5a
dc0e039
27a6151
43b44c5
ddbaf41
3114df9
52d3f04
13e2d46
2964ac9
209cd10
64dcc88
7924524
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,14 +6,13 @@ theme: | |
background: false | ||
--- | ||
|
||
Code styling | ||
=== | ||
# Code styling | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not equivalent, I presume this was done by some prettifier? Can you roll back? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think my editor extension did this - I will revert this and add a separate example presentation as you suggested, with consistent syntax. |
||
|
||
This presentation shows how to: | ||
|
||
* Left-align code blocks. | ||
* Have code blocks without background. | ||
* Execute code snippets. | ||
- Left-align code blocks. | ||
- Have code blocks without background. | ||
- Execute code snippets. | ||
|
||
```rust | ||
pub struct Greeter { | ||
|
@@ -35,10 +34,11 @@ fn main() { | |
} | ||
``` | ||
|
||
<!-- speaker_note: These are speaker notes on slide 1. --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd create a separate example presentation on speaker notes specifically. I think the speaker notes require enough manual intervention to show up that I don't think it belongs in the main demo one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Started a new speaker notes presentation. |
||
|
||
<!-- end_slide --> | ||
|
||
Column layouts | ||
=== | ||
# Column layouts | ||
|
||
The same code as the one before but split into two columns to split the API definition with its usage: | ||
|
||
|
@@ -76,10 +76,11 @@ fn main() { | |
} | ||
``` | ||
|
||
<!-- speaker_note: These are speaker notes on slide 2. --> | ||
|
||
<!-- end_slide --> | ||
|
||
Snippet execution | ||
=== | ||
# Snippet execution | ||
|
||
Run code snippets from the presentation and display their output dynamically. | ||
|
||
|
@@ -90,10 +91,11 @@ for i in range(0, 5): | |
time.sleep(0.5) | ||
``` | ||
|
||
<!-- speaker_note: These are speaker notes on slide 3. --> | ||
|
||
<!-- end_slide --> | ||
|
||
Snippet execution - `stderr` | ||
=== | ||
# Snippet execution - `stderr` | ||
|
||
Output from `stderr` will also be shown as output. | ||
|
||
|
@@ -106,3 +108,5 @@ echo "This is a successful command again" | |
sleep 0.5 | ||
man # Missing argument | ||
``` | ||
|
||
<!-- speaker_note: These are speaker notes on slide 4. --> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,11 @@ use crate::{ | |
style::{Color, Colors}, | ||
theme::{Alignment, Margin, PresentationTheme}, | ||
}; | ||
use iceoryx2::{ | ||
port::{listener::Listener, notifier::Notifier}, | ||
prelude::EventId, | ||
service::ipc::Service, | ||
}; | ||
use serde::Deserialize; | ||
use std::{ | ||
cell::RefCell, | ||
|
@@ -30,6 +35,12 @@ pub(crate) struct Presentation { | |
pub(crate) state: PresentationState, | ||
} | ||
|
||
#[derive(Debug)] | ||
pub enum SpeakerNoteChannel { | ||
Notifier(Notifier<Service>), | ||
Listener(Listener<Service>), | ||
} | ||
|
||
impl Presentation { | ||
/// Construct a new presentation. | ||
pub(crate) fn new(slides: Vec<Slide>, modals: Modals, state: PresentationState) -> Self { | ||
|
@@ -253,6 +264,10 @@ impl Presentation { | |
false | ||
} | ||
} | ||
|
||
pub(crate) fn listen_for_speaker_note_evt(&self) -> Option<usize> { | ||
self.state.listen_for_speaker_note_evt() | ||
} | ||
} | ||
|
||
impl From<Vec<Slide>> for Presentation { | ||
|
@@ -274,6 +289,7 @@ pub(crate) type AsyncPresentationErrorHolder = Arc<Mutex<Option<AsyncPresentatio | |
pub(crate) struct PresentationStateInner { | ||
current_slide_index: usize, | ||
async_error_holder: AsyncPresentationErrorHolder, | ||
pub(crate) channel: Option<SpeakerNoteChannel>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This belongs in the |
||
} | ||
|
||
#[derive(Clone, Debug, Default)] | ||
|
@@ -292,6 +308,22 @@ impl PresentationState { | |
|
||
fn set_current_slide_index(&self, value: usize) { | ||
self.inner.deref().borrow_mut().current_slide_index = value; | ||
if let Some(SpeakerNoteChannel::Notifier(notifier)) = &self.inner.deref().borrow().channel { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following the idea of moving this to presenter, you can always emit this event after a command is handled. That would help detach this logic from out of here. |
||
notifier.notify_with_custom_event_id(EventId::new(value)).unwrap(); | ||
} | ||
} | ||
|
||
pub(crate) fn set_channel(&self, speaker_note_channel: SpeakerNoteChannel) { | ||
self.inner.deref().borrow_mut().channel = Some(speaker_note_channel); | ||
} | ||
|
||
fn listen_for_speaker_note_evt(&self) -> Option<usize> { | ||
if let Some(SpeakerNoteChannel::Listener(listener)) = &self.inner.deref().borrow().channel { | ||
if let Some(evt) = listener.try_wait_one().unwrap() { | ||
return Some(evt.as_value() + 1); | ||
} | ||
} | ||
None | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
use clap::ValueEnum; | ||
use schemars::JsonSchema; | ||
use serde::Deserialize; | ||
|
||
use crate::{ | ||
custom::KeyBindingsConfig, | ||
diff::PresentationDiffer, | ||
|
@@ -37,6 +41,13 @@ pub struct PresenterOptions { | |
pub validate_overflows: bool, | ||
} | ||
|
||
#[derive(Clone, Debug, Deserialize, ValueEnum, JsonSchema)] | ||
#[serde(rename_all = "kebab-case")] | ||
pub enum SpeakerNotesMode { | ||
Publisher, | ||
Receiver, | ||
} | ||
|
||
/// A slideshow presenter. | ||
/// | ||
/// This type puts everything else together. | ||
|
@@ -104,6 +115,11 @@ impl<'a> Presenter<'a> { | |
self.render(&mut drawer)?; | ||
|
||
loop { | ||
if let Some(idx) = self.state.presentation().listen_for_speaker_note_evt() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be interesting for this thing to be inside There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would make sense - it is a source of a command after all. So are you thinking:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly 👌 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
self.apply_command(Command::GoToSlide(idx as u32)); | ||
break; | ||
} | ||
|
||
if self.poll_async_renders()? { | ||
self.render(&mut drawer)?; | ||
} | ||
|
@@ -194,7 +210,11 @@ impl<'a> Presenter<'a> { | |
}; | ||
// If the screen is too small, simply ignore this. Eventually the user will resize the | ||
// screen. | ||
if matches!(result, Err(RenderError::TerminalTooSmall)) { Ok(()) } else { result } | ||
if matches!(result, Err(RenderError::TerminalTooSmall)) { | ||
Ok(()) | ||
} else { | ||
result | ||
} | ||
} | ||
|
||
fn apply_command(&mut self, command: Command) -> CommandSideEffect { | ||
|
@@ -264,7 +284,11 @@ impl<'a> Presenter<'a> { | |
panic!("unreachable commands") | ||
} | ||
}; | ||
if needs_redraw { CommandSideEffect::Redraw } else { CommandSideEffect::None } | ||
if needs_redraw { | ||
CommandSideEffect::Redraw | ||
} else { | ||
CommandSideEffect::None | ||
} | ||
} | ||
|
||
fn try_reload(&mut self, path: &Path, force: bool) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires rust 1.75 or newer, but CI runs rust 1.74.
A rust-toolchain file and Cargo.toml entry would help make this more visible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on bumping the rust version @mfontanini?
Otherwise a different version of
iceoryx2
(if one exists for rust 1.74) or different IPC crate will need to be used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need to investigate some warnings and proper configuration for usage of this crate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for it, 1.75 is almost a year old already