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

improve error handling in ui module #1870

Merged
merged 2 commits into from
Oct 28, 2022
Merged
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
36 changes: 25 additions & 11 deletions zellij-server/src/panes/floating_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::rc::Rc;
use std::time::Instant;
use zellij_utils::{
data::{ModeInfo, Style},
errors::prelude::*,
input::command::RunCommand,
pane_size::{Offset, PaneGeom, Size, Viewport},
};
Expand Down Expand Up @@ -234,16 +235,25 @@ impl FloatingPanes {
resize_pty!(pane, os_api);
}
}
pub fn render(&mut self, output: &mut Output) {
pub fn render(&mut self, output: &mut Output) -> Result<()> {
let err_context = || "failed to render output";
let connected_clients: Vec<ClientId> =
{ self.connected_clients.borrow().iter().copied().collect() };
let mut floating_panes: Vec<_> = self.panes.iter_mut().collect();
floating_panes.sort_by(|(a_id, _a_pane), (b_id, _b_pane)| {
self.z_indices
.iter()
.position(|id| id == *a_id)
.unwrap()
.cmp(&self.z_indices.iter().position(|id| id == *b_id).unwrap())
.with_context(err_context)
.fatal()
.cmp(
&self
.z_indices
.iter()
.position(|id| id == *b_id)
.with_context(err_context)
.fatal(),
Comment on lines +248 to +255
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you using .fatal() instead of ? here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because returning result is not allowed in a closure

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed the closure above, my bad!

)
});

for (z_index, (kind, pane)) in floating_panes.iter_mut().enumerate() {
Expand All @@ -266,23 +276,27 @@ impl FloatingPanes {
.get(client_id)
.unwrap_or(&self.default_mode_info)
.mode;
pane_contents_and_ui.render_pane_frame(
*client_id,
client_mode,
self.session_is_mirrored,
);
pane_contents_and_ui
.render_pane_frame(*client_id, client_mode, self.session_is_mirrored)
.with_context(err_context)?;
if let PaneId::Plugin(..) = kind {
pane_contents_and_ui.render_pane_contents_for_client(*client_id);
pane_contents_and_ui
.render_pane_contents_for_client(*client_id)
.with_context(err_context)?;
}
// this is done for panes that don't have their own cursor (eg. panes of
// another user)
pane_contents_and_ui.render_fake_cursor_if_needed(*client_id);
pane_contents_and_ui
.render_fake_cursor_if_needed(*client_id)
.with_context(err_context)?;
}
if let PaneId::Terminal(..) = kind {
pane_contents_and_ui
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied());
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied())
.with_context(err_context)?;
}
}
Ok(())
}
pub fn resize(&mut self, new_screen_size: Size) {
let display_area = *self.display_area.borrow();
Expand Down
13 changes: 9 additions & 4 deletions zellij-server/src/panes/plugin_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,10 @@ impl Pane for PluginPane {
_client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)> {
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>> {
// FIXME: This is a hack that assumes all fixed-size panes are borderless. This
// will eventually need fixing!
if self.frame && !(self.geom.rows.is_fixed() || self.geom.cols.is_fixed()) {
let res = if self.frame && !(self.geom.rows.is_fixed() || self.geom.cols.is_fixed()) {
let pane_title = if self.pane_name.is_empty()
&& input_mode == InputMode::RenamePane
&& frame_params.is_main_client
Expand All @@ -251,10 +251,15 @@ impl Pane for PluginPane {
pane_title,
frame_params,
);
Some(frame.render())
Some(
frame
.render()
.with_context(|| format!("failed to render frame for client {_client_id}"))?,
)
} else {
None
}
};
Ok(res)
}
fn render_fake_cursor(
&mut self,
Expand Down
12 changes: 7 additions & 5 deletions zellij-server/src/panes/terminal_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ impl Pane for TerminalPane {
client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)> {
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>> {
let err_context = || format!("failed to render frame for client {client_id}");
// TODO: remove the cursor stuff from here
let pane_title = if self.pane_name.is_empty()
&& input_mode == InputMode::RenamePane
Expand Down Expand Up @@ -398,12 +399,12 @@ impl Pane for TerminalPane {
frame.add_exit_status(exit_status.as_ref().copied());
}

match self.frame.get(&client_id) {
let res = match self.frame.get(&client_id) {
// TODO: use and_then or something?
Some(last_frame) => {
if &frame != last_frame {
if !self.borderless {
let frame_output = frame.render();
let frame_output = frame.render().with_context(err_context)?;
self.frame.insert(client_id, frame);
Some(frame_output)
} else {
Expand All @@ -415,14 +416,15 @@ impl Pane for TerminalPane {
},
None => {
if !self.borderless {
let frame_output = frame.render();
let frame_output = frame.render().with_context(err_context)?;
self.frame.insert(client_id, frame);
Some(frame_output)
} else {
None
}
},
}
};
Ok(res)
}
fn render_fake_cursor(
&mut self,
Expand Down
35 changes: 23 additions & 12 deletions zellij-server/src/panes/tiled_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::cell::RefCell;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::rc::Rc;
use std::time::Instant;
use zellij_utils::errors::prelude::*;
use zellij_utils::{
data::{ModeInfo, Style},
input::command::RunCommand,
Expand Down Expand Up @@ -378,7 +379,8 @@ impl TiledPanes {
pub fn has_panes(&self) -> bool {
!self.panes.is_empty()
}
pub fn render(&mut self, output: &mut Output, floating_panes_are_visible: bool) {
pub fn render(&mut self, output: &mut Output, floating_panes_are_visible: bool) -> Result<()> {
let err_context = || "failed to render tiled panes";
let connected_clients: Vec<ClientId> =
{ self.connected_clients.borrow().iter().copied().collect() };
let multiple_users_exist_in_session = { self.connected_clients_in_app.borrow().len() > 1 };
Expand Down Expand Up @@ -409,15 +411,17 @@ impl TiledPanes {
.get(client_id)
.unwrap_or(&self.default_mode_info)
.mode;
let err_context =
|| format!("failed to render tiled panes for client {client_id}");
if let PaneId::Plugin(..) = kind {
pane_contents_and_ui.render_pane_contents_for_client(*client_id);
pane_contents_and_ui
.render_pane_contents_for_client(*client_id)
.with_context(err_context)?;
}
if self.draw_pane_frames {
pane_contents_and_ui.render_pane_frame(
*client_id,
client_mode,
self.session_is_mirrored,
);
pane_contents_and_ui
.render_pane_frame(*client_id, client_mode, self.session_is_mirrored)
.with_context(err_context)?;
} else {
let boundaries = client_id_to_boundaries
.entry(*client_id)
Expand All @@ -432,20 +436,27 @@ impl TiledPanes {
pane_contents_and_ui.render_terminal_title_if_needed(*client_id, client_mode);
// this is done for panes that don't have their own cursor (eg. panes of
// another user)
pane_contents_and_ui.render_fake_cursor_if_needed(*client_id);
pane_contents_and_ui
.render_fake_cursor_if_needed(*client_id)
.with_context(err_context)?;
}
if let PaneId::Terminal(..) = kind {
pane_contents_and_ui.render_pane_contents_to_multiple_clients(
connected_clients.iter().copied(),
);
pane_contents_and_ui
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied())
.with_context(err_context)?;
}
}
}
// render boundaries if needed
for (client_id, boundaries) in &mut client_id_to_boundaries {
// TODO: add some conditional rendering here so this isn't rendered for every character
output.add_character_chunks_to_client(*client_id, boundaries.render(), None);
output.add_character_chunks_to_client(
*client_id,
boundaries.render().with_context(err_context)?,
None,
);
}
Ok(())
}
pub fn get_panes(&self) -> impl Iterator<Item = (&PaneId, &Box<dyn Pane>)> {
self.panes.iter()
Expand Down
2 changes: 1 addition & 1 deletion zellij-server/src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ impl Pty {
.bus
.os_input
.as_mut()
.unwrap()
.ok_or_else(|| SpawnTerminalError::GenericSpawnError("os input is none"))?
.spawn_terminal(terminal_action, quit_cb, self.default_editor.clone())?;
let terminal_bytes = task::spawn({
let err_context = || format!("failed to run async task for terminal {terminal_id}");
Expand Down
2 changes: 1 addition & 1 deletion zellij-server/src/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ impl Screen {
let overlay = self.overlay.clone();
for (tab_index, tab) in &mut self.tabs {
if tab.has_selectable_tiled_panes() {
let vte_overlay = overlay.generate_overlay(size);
let vte_overlay = overlay.generate_overlay(size).context(err_context)?;
tab.render(&mut output, Some(vte_overlay))
.context(err_context)?;
} else {
Expand Down
9 changes: 6 additions & 3 deletions zellij-server/src/tab/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub trait Pane {
client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)>; // TODO: better
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>>; // TODO: better
fn render_fake_cursor(
&mut self,
cursor_color: PaletteColor,
Expand Down Expand Up @@ -1329,9 +1329,12 @@ impl Tab {

self.hide_cursor_and_clear_display_as_needed(output);
self.tiled_panes
.render(output, self.floating_panes.panes_are_visible());
.render(output, self.floating_panes.panes_are_visible())
.with_context(err_context)?;
if self.floating_panes.panes_are_visible() && self.floating_panes.has_active_panes() {
self.floating_panes.render(output);
self.floating_panes
.render(output)
.with_context(err_context)?;
}

// FIXME: Once clients can be distinguished
Expand Down
28 changes: 21 additions & 7 deletions zellij-server/src/ui/boundaries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::panes::terminal_character::{TerminalCharacter, EMPTY_TERMINAL_CHARACT
use crate::tab::Pane;
use ansi_term::Colour::{Fixed, RGB};
use std::collections::HashMap;
use zellij_utils::errors::prelude::*;
use zellij_utils::{data::PaletteColor, shared::colors};

use std::fmt::{Display, Error, Formatter};
Expand Down Expand Up @@ -47,18 +48,29 @@ impl BoundarySymbol {
self.color = color;
*self
}
pub fn as_terminal_character(&self) -> TerminalCharacter {
if self.invisible {
pub fn as_terminal_character(&self) -> Result<TerminalCharacter> {
let tc = if self.invisible {
EMPTY_TERMINAL_CHARACTER
} else {
let character = self.boundary_type.chars().next().unwrap();
let character = self
.boundary_type
.chars()
.next()
.context("no boundary symbols defined")
.with_context(|| {
format!(
"failed to convert boundary symbol {} into terminal character",
self.boundary_type
)
})?;
TerminalCharacter {
character,
width: 1,
styles: RESET_STYLES
.foreground(self.color.map(|palette_color| palette_color.into())),
}
}
};
Ok(tc)
}
}

Expand Down Expand Up @@ -528,16 +540,18 @@ impl Boundaries {
}
}
}
pub fn render(&self) -> Vec<CharacterChunk> {
pub fn render(&self) -> Result<Vec<CharacterChunk>> {
let mut character_chunks = vec![];
for (coordinates, boundary_character) in &self.boundary_characters {
character_chunks.push(CharacterChunk::new(
vec![boundary_character.as_terminal_character()],
vec![boundary_character
.as_terminal_character()
.context("failed to render as terminal character")?],
coordinates.x,
coordinates.y,
));
}
character_chunks
Ok(character_chunks)
}
fn rect_right_boundary_is_before_screen_edge(&self, rect: &dyn Pane) -> bool {
rect.x() + rect.cols() < self.viewport.cols
Expand Down
23 changes: 15 additions & 8 deletions zellij-server/src/ui/overlay/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
pub mod prompt;

use crate::ServerInstruction;
use zellij_utils::errors::prelude::*;
use zellij_utils::pane_size::Size;

#[derive(Clone, Debug)]
Expand All @@ -19,7 +20,7 @@ pub struct Overlay {
pub trait Overlayable {
/// Generates vte_output that can be passed into
/// the `render()` function
fn generate_overlay(&self, size: Size) -> String;
fn generate_overlay(&self, size: Size) -> Result<String>;
}

#[derive(Clone, Debug)]
Expand All @@ -28,9 +29,11 @@ pub enum OverlayType {
}

impl Overlayable for OverlayType {
fn generate_overlay(&self, size: Size) -> String {
fn generate_overlay(&self, size: Size) -> Result<String> {
match &self {
OverlayType::Prompt(prompt) => prompt.generate_overlay(size),
OverlayType::Prompt(prompt) => prompt
.generate_overlay(size)
.context("failed to generate VTE output from overlay type"),
}
}
}
Expand All @@ -44,15 +47,17 @@ pub struct OverlayWindow {
}

impl Overlayable for OverlayWindow {
fn generate_overlay(&self, size: Size) -> String {
fn generate_overlay(&self, size: Size) -> Result<String> {
let mut output = String::new();
//let clear_display = "\u{1b}[2J";
//output.push_str(&clear_display);
for overlay in &self.overlay_stack {
let vte_output = overlay.generate_overlay(size);
let vte_output = overlay
.generate_overlay(size)
.context("failed to generate VTE output from overlay window")?;
output.push_str(&vte_output);
}
output
Ok(output)
}
}

Expand All @@ -70,8 +75,10 @@ impl Overlay {
}

impl Overlayable for Overlay {
fn generate_overlay(&self, size: Size) -> String {
self.overlay_type.generate_overlay(size)
fn generate_overlay(&self, size: Size) -> Result<String> {
self.overlay_type
.generate_overlay(size)
.context("failed to generate VTE output from overlay")
}
}

Expand Down
Loading