Skip to content

remote-viewer: Address review feedback from #11634 #11706

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.

Sign up for GitHub

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

Open
ogoffart wants to merge 1 commit into
base: feature/remote-viewer
Choose a base branch
from
Open

remote-viewer: Address review feedback from #11634 #11706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 2 deletions editors/vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@
},
{
"command": "slint.selectRemotePreview",
"title": "Connect to Remote Preview",
"title": "(Experimental) Connect to Remote Preview",
"category": "Slint",
"icon": "$(preview)"
},
{
"command": "slint.disconnectRemotePreview",
"title": "Disconnect Remote Preview",
"title": "(Experimental) Disconnect Remote Preview",
"category": "Slint",
"icon": "$(debug-disconnect)"
},
Expand Down
4 changes: 0 additions & 4 deletions internal/preview-protocol/src/preview_to_lsp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,9 @@ use crate::SourceFileVersion;
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum PreviewTarget {
#[allow(dead_code)]
ChildProcess,
#[allow(dead_code)]
EmbeddedWasm,
#[allow(dead_code)]
Remote,
#[allow(dead_code)]
Dummy,
}

Expand Down
72 changes: 15 additions & 57 deletions tools/lsp/language.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ pub fn send_state_to_preview(ctx: &Context) {
#[cfg(any(feature = "preview-external", feature = "preview-engine", feature = "preview-remote"))]
pub fn send_files_to_preview(ctx: &Context, files: &[lsp_types::Url]) {
for url in files {
if let Some(node) = ctx.document_cache.get_document(url).and_then(|doc| doc.node.as_ref()) {
let version = ctx.document_cache.document_version_by_path(node.source_file.path());
let contents = node.text().to_string().into();
tracing::debug!("Sending cached file {} to preview", url);
ctx.to_preview.send(&i_slint_preview_protocol::LspToPreviewMessage::SetContents {
url: i_slint_preview_protocol::VersionedUrl::new(url.clone(), version),
contents,
});
continue;
}
let Some(path) = url.to_file_path().ok() else {
tracing::warn!("Cannot convert URL to file path: {url}");
continue;
Expand Down Expand Up @@ -439,11 +449,14 @@ pub fn register_request_handlers(rh: &mut RequestHandler) {
}
#[cfg(feature = "preview-remote")]
CONNECT_REMOTE_PREVIEW_COMMAND => {
return connect_remote_preview_command(&params.arguments, ctx);
return crate::preview::connector::remote::connect_remote_preview_command(
&params.arguments,
ctx,
);
}
#[cfg(feature = "preview-remote")]
DISCONNECT_REMOTE_PREVIEW_COMMAND => {
disconnect_remote_preview_command(ctx);
crate::preview::connector::remote::disconnect_remote_preview_command(ctx);
}
_ => {
tracing::error!("Received unknown command {}", params.command.as_str());
Expand Down Expand Up @@ -673,61 +686,6 @@ pub fn show_preview(component: i_slint_preview_protocol::PreviewComponent, ctx:
ctx.to_preview.send(&i_slint_preview_protocol::LspToPreviewMessage::ShowPreview(component));
}

#[cfg(feature = "preview-remote")]
pub fn connect_remote_preview_command(
params: &[serde_json::Value],
ctx: &Context,
) -> Result<Option<serde_json::Value>, LspError> {
let addresses = params.first().and_then(serde_json::Value::as_array).map(|addresses| {
addresses.iter().filter_map(serde_json::Value::as_str).map(String::from).collect::<Vec<_>>()
});
let port = params.get(1).and_then(serde_json::Value::as_u64);

if let Some(addresses) = addresses {
if let Some(port) = port {
use crate::preview::connector::remote::RemoteLspToPreview;

let _ =
ctx.to_preview.set_preview_target(i_slint_preview_protocol::PreviewTarget::Remote);
ctx.to_preview.with_preview_target::<RemoteLspToPreview, Result<Option<serde_json::Value>, LspError>>(
|remote| {
let preview_to_lsp_sender = ctx.preview_to_lsp_sender.clone();
let future = remote.connect(addresses, port as u16);
crate::common::spawn_local(async move {
if let Err(err) = future.await {
LspError {
code: LspErrorCode::RequestFailed,
message: format!("Failed to connect to remote preview: {err}"),
};
} else {
let _ = preview_to_lsp_sender.send(PreviewToLspMessage::RequestState { files: Vec::new() });
}
});
Ok(None)
}).unwrap()
} else {
Err(LspError {
code: LspErrorCode::InvalidParameter,
message: "Need number as the second parameter".to_owned(),
})
}
} else {
Err(LspError {
code: LspErrorCode::InvalidParameter,
message: "Need array of string as the first parameter".to_owned(),
})
}
}

#[cfg(feature = "preview-remote")]
pub fn disconnect_remote_preview_command(ctx: &Context) {
let to_preview = ctx.to_preview.clone();
tracing::debug!("disconnect_remote_preview_command");
to_preview.with_preview_target::<crate::preview::connector::RemoteLspToPreview, _>(|remote| {
crate::common::spawn_local(remote.disconnect());
});
}

fn populate_command_range(
node: &SyntaxNode,
format: common::ByteFormat,
Expand Down
4 changes: 2 additions & 2 deletions tools/lsp/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use std::time::Duration;

use crate::common::{SwitchableLspToPreview, document_cache::CompilerConfiguration};
#[cfg(feature = "preview-remote")]
use crate::preview::connector::RemoteLspToPreview;
use crate::preview::connector::remote::RemoteLspToPreview;

#[cfg(not(any(
target_os = "openbsd",
Expand Down Expand Up @@ -359,7 +359,7 @@ async fn main_loop(
Box::new(preview::connector::EmbeddedLspToPreview::new(sn.clone()));
#[cfg(feature = "preview-remote")]
let remote_preview: Box<dyn common::LspToPreview> = Box::new(
preview::connector::RemoteLspToPreview::new(sn, preview_to_lsp_sender.clone()),
preview::connector::remote::RemoteLspToPreview::new(sn, preview_to_lsp_sender.clone()),
);
Rc::new(
preview::connector::SwitchableLspToPreview::new(
Expand Down
2 changes: 0 additions & 2 deletions tools/lsp/preview/connector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ pub use native::*;
pub use crate::common::SwitchableLspToPreview;
#[cfg(feature = "preview-remote")]
pub mod remote;
#[cfg(feature = "preview-remote")]
pub use remote::*;

use crate::preview;

Expand Down
58 changes: 58 additions & 0 deletions tools/lsp/preview/connector/remote.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,70 @@ use tokio::sync::mpsc;
use tokio::{sync::RwLock, task::JoinHandle};
use tokio_tungstenite_wasm::{Message, WebSocketStream};

use crate::language::{LspError, LspErrorCode};
use crate::preview::connector::remote::remote_notifications::{
ConnectionState, RemoteViewerConnectionState,
};

mod remote_notifications;

pub fn connect_remote_preview_command(
params: &[serde_json::Value],
ctx: &crate::language::Context,
) -> Result<Option<serde_json::Value>, LspError> {
let addresses = params.first().and_then(serde_json::Value::as_array).map(|addresses| {
addresses.iter().filter_map(serde_json::Value::as_str).map(String::from).collect::<Vec<_>>()
});
let port = params.get(1).and_then(serde_json::Value::as_u64);

if let Some(addresses) = addresses {
if let Some(port) = port {
let _ =
ctx.to_preview.set_preview_target(i_slint_preview_protocol::PreviewTarget::Remote);
ctx.to_preview.with_preview_target::<RemoteLspToPreview, Result<Option<serde_json::Value>, LspError>>(
|remote| {
let preview_to_lsp_sender = ctx.preview_to_lsp_sender.clone();
let future = remote.connect(addresses, port as u16);
crate::common::spawn_local(async move {
if let Err(err) = future.await {
let _ = preview_to_lsp_sender.send(
Copy link
Copy Markdown
Member Author

@ogoffart ogoffart May 12, 2026

Choose a reason for hiding this comment

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

Note that this is hard to see in the diff as this is part of a move (Why can't github show the moves correctly) but i also changed this to send a error message to the client instead of ignoring the error.

PreviewToLspMessage::SendShowMessage {
message: lsp_types::ShowMessageParams {
typ: lsp_types::MessageType::ERROR,
message: format!(
"Failed to connect to remote preview: {err}"
),
},
},
);
} else {
let _ = preview_to_lsp_sender.send(PreviewToLspMessage::RequestState { files: Vec::new() });
}
});
Ok(None)
}).unwrap()
} else {
Err(LspError {
code: LspErrorCode::InvalidParameter,
message: "Need number as the second parameter".to_owned(),
})
}
} else {
Err(LspError {
code: LspErrorCode::InvalidParameter,
message: "Need array of string as the first parameter".to_owned(),
})
}
}

pub fn disconnect_remote_preview_command(ctx: &crate::language::Context) {
let to_preview = ctx.to_preview.clone();
tracing::debug!("disconnect_remote_preview_command");
to_preview.with_preview_target::<RemoteLspToPreview, _>(|remote| {
crate::common::spawn_local(remote.disconnect());
});
}

struct RemoteLspConnection {
sender: SplitSink<WebSocketStream, Message>,
task: tokio::task::JoinHandle<()>,
Expand Down
Loading