Add Win32 GUI - #802
Conversation
mstange
left a comment
mstange
left a comment
There was a problem hiding this comment.
Cool stuff, could you put a screenshot in the PR?
| [lints.rust] | ||
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(gui)'] } |
There was a problem hiding this comment.
What does this do? Also unnecessary space at line start
| @@ -0,0 +1,24 @@ | |||
| fn main() { | |||
| #[cfg(windows)] | |||
There was a problem hiding this comment.
This should be scoped to the new feature. Or, could we use https://crates.io/crates/embed-manifest instead?
There was a problem hiding this comment.
Oh I misunderstood embed-manifest, that's also something you put in a build.rs. Ok so what you have here (with the feature check added) seems fine.
| eprintln!("Profiling process with pid {pid}..."); | ||
| eprintln!("Press Ctrl+C to stop."); | ||
| // TODO: Respect recording_props.time_limit, if specified | ||
| // Wait for Ctrl+C. |
There was a problem hiding this comment.
The "Wait for Ctrl+C" comment is dropped
| const COLLAPSED_H: i32 = 140; | ||
| const EXPANDED_H: i32 = 315; | ||
|
|
||
| unsafe fn create_button( |
There was a problem hiding this comment.
I'm curious to see a version which uses https://crates.io/crates/winsafe to reduce the amount of unsafe here
| recording_mode: RecordingMode, | ||
| recording_props: RecordingProps, | ||
| profile_creation_props: ProfileCreationProps, | ||
| stop_receiver: Option<std::sync::mpsc::Receiver<()>>, |
There was a problem hiding this comment.
I'd prefer a stop_condition: StopCondition arg which is either StopCondition::CtrlC or StopCondition::ReceiverSignaled(receiver)
|
Winsafe version is here: https://github.com/jrmuizel/samply/blob/winsafe-gui/samply/src/windows/gui.rs |
I think that looks a lot better, what do you think? |
1256b4c to
eb47863
Compare
2 participants
No description provided.