Skip to content
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

windows: Implement dock menus #17352

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ members = [
# Tooling
#

"tooling/xtask"
"tooling/xtask",
]
default-members = ["crates/zed"]

Expand Down Expand Up @@ -483,9 +483,11 @@ features = [
version = "0.58"
features = [
"implement",
"Foundation_Collections",
"Foundation_Numerics",
"System",
"System_Threading",
"UI_StartScreen",
"UI_ViewManagement",
"Wdk_System_SystemServices",
"Win32_Globalization",
Expand Down
9 changes: 9 additions & 0 deletions crates/gpui/src/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ pub trait Action: 'static + Send {
where
Self: Sized;

/// Get the arguments that pass to the new instance if this action
/// requires a new instance to perform.
///
/// Platform: Windows and Linux only.
/// Usage: Dock menu only.
fn arguments(&self) -> &str {
self.name()
}

/// Build this action from a JSON value. This is used to construct actions from the keymap.
/// A value of `{}` will be passed for actions that don't have any parameters.
fn build(value: serde_json::Value) -> Result<Box<dyn Action>>
Expand Down
4 changes: 4 additions & 0 deletions crates/gpui/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ mod fps;
#[cfg(target_os = "windows")]
mod windows;

#[cfg(target_os = "windows")]
/// TODO:
pub mod app_identifier;

use crate::{
point, Action, AnyWindowHandle, AppContext, AsyncWindowContext, BackgroundExecutor, Bounds,
DevicePixels, DispatchEventResult, Font, FontId, FontMetrics, FontRun, ForegroundExecutor,
Expand Down
61 changes: 61 additions & 0 deletions crates/gpui/src/platform/app_identifier.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use std::sync::OnceLock;

use windows::Win32::Foundation::MAX_PATH;

static APP_IDENTIFIER: OnceLock<String> = OnceLock::new();
static APP_MUTEX_IDENTIFIER: OnceLock<String> = OnceLock::new();
static APP_EVENT_IDENTIFIER: OnceLock<String> = OnceLock::new();
static APP_SHARED_MEMORY_IDENTIFIER: OnceLock<String> = OnceLock::new();

/// TODO:
pub const APP_SHARED_MEMORY_MAX_SIZE: usize = 1024;

/// TODO:
pub fn register_app_identifier(app_identifier: &str) {
APP_IDENTIFIER.get_or_init(|| app_identifier.to_string());
}

fn get_app_identifier() -> &'static str {
APP_IDENTIFIER.get_or_init(|| {
let rand_number = rand::random::<u32>();
let random_identifier = format!("Gpui-App-Identifier-{}", rand_number);
log::error!(
"No app identifier is set, call register_app_identifier first. Using {} instead.",
random_identifier
);
random_identifier
})
}

/// TODO:
pub fn get_app_instance_mutex_identifier() -> &'static str {
APP_MUTEX_IDENTIFIER.get_or_init(|| {
let identifier = format!("Local\\{}-Instance-Mutex", get_app_identifier());
if identifier.len() as u32 > MAX_PATH {
panic!("The length of app instance mutex identifier `{identifier}` is limited to {MAX_PATH} characters.");
}
identifier
})
}

/// TODO:
pub fn get_app_instance_event_identifier() -> &'static str {
APP_EVENT_IDENTIFIER.get_or_init(|| {
let identifier = format!("Local\\{}-Instance-Event", get_app_identifier());
if identifier.len() as u32 > MAX_PATH {
panic!("The length of app instance event identifier `{identifier}` is limited to {MAX_PATH} characters.");
}
identifier
})
}

/// TODO:
pub fn get_app_shared_memory_identifier() -> &'static str {
APP_SHARED_MEMORY_IDENTIFIER.get_or_init(|| {
let identifier = format!("Local\\{}-Shared-Memory", get_app_identifier());
if identifier.len() as u32 > MAX_PATH {
panic!("The length of app shared memory identifier `{identifier}` is limited to {MAX_PATH} characters.");
}
identifier
})
}
132 changes: 125 additions & 7 deletions crates/gpui/src/platform/windows/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ use std::{

use ::util::ResultExt;
use anyhow::{anyhow, Context, Result};
use app_identifier::{
get_app_instance_event_identifier, get_app_shared_memory_identifier, APP_SHARED_MEMORY_MAX_SIZE,
};
use collections::FxHashMap;
use futures::channel::oneshot::{self, Receiver};
use itertools::Itertools;
use parking_lot::RwLock;
Expand All @@ -29,14 +33,20 @@ use windows::{
RegisterClipboardFormatW, SetClipboardData,
},
LibraryLoader::*,
Memory::{GlobalAlloc, GlobalLock, GlobalUnlock, GMEM_MOVEABLE},
Memory::{
CreateFileMappingW, GlobalAlloc, GlobalLock, GlobalUnlock, MapViewOfFile,
UnmapViewOfFile, FILE_MAP_ALL_ACCESS, GMEM_MOVEABLE, PAGE_READWRITE,
},
Ole::*,
SystemInformation::*,
Threading::*,
},
UI::{Input::KeyboardAndMouse::*, Shell::*, WindowsAndMessaging::*},
},
UI::ViewManagement::UISettings,
UI::{
StartScreen::{JumpList, JumpListItem},
ViewManagement::UISettings,
},
};

use crate::*;
Expand All @@ -54,10 +64,13 @@ pub(crate) struct WindowsPlatform {
windows_version: WindowsVersion,
bitmap_factory: ManuallyDrop<IWICImagingFactory>,
validation_number: usize,
single_instance_event: Owned<HANDLE>,
shared_memory_handle: Owned<HANDLE>,
}

pub(crate) struct WindowsPlatformState {
callbacks: PlatformCallbacks,
dock_menu_actions: FxHashMap<String, Box<dyn Action>>,
// NOTE: standard cursor handles don't need to close.
pub(crate) current_cursor: HCURSOR,
}
Expand All @@ -75,10 +88,12 @@ struct PlatformCallbacks {
impl WindowsPlatformState {
fn new() -> Self {
let callbacks = PlatformCallbacks::default();
let dock_menu_actions = FxHashMap::default();
let current_cursor = load_cursor(CursorStyle::Arrow);

Self {
callbacks,
dock_menu_actions,
current_cursor,
}
}
Expand Down Expand Up @@ -108,6 +123,30 @@ impl WindowsPlatform {
register_clipboard_format(CLIPBOARD_METADATA_FORMAT).unwrap();
let windows_version = WindowsVersion::new().expect("Error retrieve windows version");
let validation_number = rand::random::<usize>();
let single_instance_event = unsafe {
Owned::new(
CreateEventW(
None,
false,
false,
&HSTRING::from(get_app_instance_event_identifier()),
)
.expect("Unable to create single instance event."),
)
};
let shared_memory_handle = unsafe {
Owned::new(
CreateFileMappingW(
INVALID_HANDLE_VALUE,
None,
PAGE_READWRITE,
0,
APP_SHARED_MEMORY_MAX_SIZE as u32,
&HSTRING::from(get_app_shared_memory_identifier()),
)
.expect("Unable to create shared memory"),
)
};

Self {
state,
Expand All @@ -121,6 +160,8 @@ impl WindowsPlatform {
windows_version,
bitmap_factory,
validation_number,
single_instance_event,
shared_memory_handle,
}
}

Expand Down Expand Up @@ -176,6 +217,73 @@ impl WindowsPlatform {

lock.is_empty()
}

fn configure_jump_list(&self, menus: Vec<MenuItem>) -> Result<()> {
let jump_list = JumpList::LoadCurrentAsync()?.get()?;
let items = jump_list.Items()?;
items.Clear()?;
for item in menus {
let item = match item {
MenuItem::Separator => JumpListItem::CreateSeparator()?,
MenuItem::Submenu(_) => {
log::error!("Set `MenuItemSubmenu` for dock menu on Windows is not supported.");
continue;
}
MenuItem::Action { name, action, .. } => {
let item_args = format!("--new-instance {}", action.arguments());
self.state
.borrow_mut()
.dock_menu_actions
.insert(action.arguments().to_string(), action);
JumpListItem::CreateWithArguments(
&HSTRING::from(item_args),
&HSTRING::from(name.as_ref()),
)?
}
};
items.Append(&item)?;
}
jump_list.SaveAsync()?.get()?;
Ok(())
}

fn handle_instance_message(&self) {
let msg = unsafe {
let memory_addr =
MapViewOfFile(*self.shared_memory_handle, FILE_MAP_ALL_ACCESS, 0, 0, 0);
let string = String::from_utf8_lossy(std::slice::from_raw_parts(
memory_addr.Value as *const _ as _,
APP_SHARED_MEMORY_MAX_SIZE,
))
.trim_matches('\0')
.to_string();
let empty_buffer = vec![0u8; string.len()];
std::ptr::copy_nonoverlapping(
empty_buffer.as_ptr(),
memory_addr.Value as _,
empty_buffer.len(),
);
UnmapViewOfFile(memory_addr).log_err();
string
};
println!("-> Single instance event, {},", msg);
let mut lock = self.state.borrow_mut();
if let Some(mut callback) = lock.callbacks.app_menu_action.take() {
let Some(action) = lock
.dock_menu_actions
.get(&msg)
.map(|action| action.boxed_clone())
else {
lock.callbacks.app_menu_action = Some(callback);
log::error!("Dock menu {msg} not found");
return;
};
drop(lock);
println!("==> Performing action: {msg}");
callback(&*action);
self.state.borrow_mut().callbacks.app_menu_action = Some(callback);
}
}
}

impl Platform for WindowsPlatform {
Expand All @@ -197,16 +305,23 @@ impl Platform for WindowsPlatform {
begin_vsync(*vsync_event);
'a: loop {
let wait_result = unsafe {
MsgWaitForMultipleObjects(Some(&[*vsync_event]), false, INFINITE, QS_ALLINPUT)
MsgWaitForMultipleObjects(
Some(&[*vsync_event, *self.single_instance_event]),
false,
INFINITE,
QS_ALLINPUT,
)
};

match wait_result {
// compositor clock ticked so we should draw a frame
WAIT_EVENT(0) => {
self.redraw_all();
}
// TODO:
WAIT_EVENT(1) => self.handle_instance_message(),
// Windows thread messages are posted
WAIT_EVENT(1) => {
WAIT_EVENT(2) => {
let mut msg = MSG::default();
unsafe {
while PeekMessageW(&mut msg, None, 0, 0, PM_REMOVE).as_bool() {
Expand Down Expand Up @@ -410,7 +525,10 @@ impl Platform for WindowsPlatform {

// todo(windows)
fn set_menus(&self, _menus: Vec<Menu>, _keymap: &Keymap) {}
fn set_dock_menu(&self, _menus: Vec<MenuItem>, _keymap: &Keymap) {}

fn set_dock_menu(&self, menus: Vec<MenuItem>, _keymap: &Keymap) {
self.configure_jump_list(menus).log_err();
}

fn on_app_menu_action(&self, callback: Box<dyn FnMut(&dyn Action)>) {
self.state.borrow_mut().callbacks.app_menu_action = Some(callback);
Expand Down Expand Up @@ -772,8 +890,8 @@ fn read_metadata_from_clipboard(metadata_format: u32) -> Option<String> {
}

// clipboard
pub const CLIPBOARD_HASH_FORMAT: PCWSTR = windows::core::w!("zed-text-hash");
pub const CLIPBOARD_METADATA_FORMAT: PCWSTR = windows::core::w!("zed-metadata");
const CLIPBOARD_HASH_FORMAT: PCWSTR = windows::core::w!("zed-text-hash");
const CLIPBOARD_METADATA_FORMAT: PCWSTR = windows::core::w!("zed-metadata");

#[cfg(test)]
mod tests {
Expand Down
16 changes: 15 additions & 1 deletion crates/zed/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,13 @@ fn init_ui(

fn main() {
let start_time = std::time::Instant::now();

#[cfg(target_os = "windows")]
{
use zed::windows_only_instance::*;
register_zed_identifier();
}

Comment on lines 319 to +328
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that we check single instance at the very beginning?

menu::init();
zed_actions::init();

Expand Down Expand Up @@ -361,11 +368,15 @@ fn main() {
}
}

let args = Args::parse();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want this behavior, the sqlite database we use can't support more than one instance. If we remove this, I'd be happy to merge this feature :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but I didn't quite understand your point. Could you please explain how let args = Args::parse(); is related to the SQLite database? I mean it is simply handling the arguments passed to Zed, like zed.exe --some_args, am I wrong?

Copy link
Contributor

@mikayla-maki mikayla-maki Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I misclicked the line. I meant to select the new_instance field of 'Args', which I think means 'create a new Zed process', and if there are two Zed processes talking to the same sqlite DB, things will break because sqlite requires serialized updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

Sorry for the confusion here! The new_instance here doesn't refer to creating a new instance of zed itself, but rather to the startup arguments passed to an instance.

Handling the Dock Menu on Windows is a bit more complicated. When you click a button on the Dock Menu, like New Window, Windows spawns a new instance of zed, similar to command::new("zed.exe").spawn(). To differentiate which button the user clicked, we need to pass some arguments to this new instance, such as zed.exe --new_instance="workspace::NewWindow".

When the new instance starts, the previously merged single instance feature detects an already running instance through the check_single_instance() function. It also notices the Dock Menu argument workspace::NewWindow attached to the new instance and forwards this argument to the existing instance before exiting itself. The flow looks something like this:

if !check_single_instance() {
    if let Some(dock_menu_args) = args.new_instance {
        send_to_existing_instance(dock_menu_args);
    }
    println!("Already running!");
    return;
}

Then, the existing instance, after receiving the Dock Menu argument workspace::NewWindow from the closing instance, begins executing the function associated with New Window.

Since English isn't my first language, I often struggle with naming things, so I simply used new_instance here. Sorry for the confusion caused!

#[cfg(target_os = "windows")]
{
use zed::windows_only_instance::*;
if !check_single_instance() {
println!("zed is already running");
if let Some(ref argument) = args.new_instance {
send_instance_message(argument);
}
return;
}
}
Expand Down Expand Up @@ -500,7 +511,6 @@ fn main() {
reliability::init(client.http_client(), installation_id, cx);
let prompt_builder = init_common(app_state.clone(), cx);

let args = Args::parse();
let urls: Vec<_> = args
.paths_or_urls
.iter()
Expand Down Expand Up @@ -1065,6 +1075,10 @@ struct Args {
/// URLs can either be `file://` or `zed://` scheme, or relative to <https://zed.dev>.
paths_or_urls: Vec<String>,

/// TODO:
#[arg(long)]
new_instance: Option<String>,

/// Instructs zed to run as a dev server on this machine. (not implemented)
#[arg(long)]
dev_server_token: Option<String>,
Expand Down
Loading
Loading