Compare commits

..

3 Commits

Author SHA1 Message Date
KyleBarton
bb3e3d01dd Remove implicit dependency on node env for data_dir devcontainer cli 2025-12-23 11:29:54 -08:00
Kirill Bulatov
251033f88f Fix the argument order when starting devcontainers (#45584)
Release Notes:

- (Preview only) Fix devcontainers not starting when certain env
variables were set

Co-authored-by: KyleBarton <kjb@initialcapacity.io>
2025-12-23 19:10:51 +00:00
Xiaobo Liu
9f90c1a1b7 git_ui: Show copy-SHA button on commit header hover (#45478)
Release Notes:

- git: Added the ability to copy a commit's SHA in the commit view.

---------

Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
2025-12-23 17:11:56 +00:00
5 changed files with 176 additions and 347 deletions

View File

@@ -8,9 +8,9 @@ use git::{
parse_git_remote_url,
};
use gpui::{
AnyElement, App, AppContext as _, AsyncApp, AsyncWindowContext, Context, Element, Entity,
EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, ParentElement,
PromptLevel, Render, Styled, Task, WeakEntity, Window, actions,
AnyElement, App, AppContext as _, AsyncApp, AsyncWindowContext, ClipboardItem, Context,
Element, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement,
ParentElement, PromptLevel, Render, Styled, Task, WeakEntity, Window, actions,
};
use language::{
Anchor, Buffer, Capability, DiskState, File, LanguageRegistry, LineEnding, OffsetRangeExt as _,
@@ -24,7 +24,7 @@ use std::{
sync::Arc,
};
use theme::ActiveTheme;
use ui::{DiffStat, Tooltip, prelude::*};
use ui::{ButtonLike, DiffStat, Tooltip, prelude::*};
use util::{ResultExt, paths::PathStyle, rel_path::RelPath, truncate_and_trailoff};
use workspace::item::TabTooltipContent;
use workspace::{
@@ -383,6 +383,7 @@ impl CommitView {
fn render_header(&self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
let commit = &self.commit;
let author_name = commit.author_name.clone();
let commit_sha = commit.sha.clone();
let commit_date = time::OffsetDateTime::from_unix_timestamp(commit.commit_timestamp)
.unwrap_or_else(|_| time::OffsetDateTime::now_utc());
let local_offset = time::UtcOffset::current_local_offset().unwrap_or(time::UtcOffset::UTC);
@@ -429,6 +430,19 @@ impl CommitView {
.full_width()
});
let clipboard_has_link = cx
.read_from_clipboard()
.and_then(|entry| entry.text())
.map_or(false, |clipboard_text| {
clipboard_text.trim() == commit_sha.as_ref()
});
let (copy_icon, copy_icon_color) = if clipboard_has_link {
(IconName::Check, Color::Success)
} else {
(IconName::Copy, Color::Muted)
};
h_flex()
.border_b_1()
.border_color(cx.theme().colors().border_variant)
@@ -454,13 +468,47 @@ impl CommitView {
h_flex()
.gap_1()
.child(Label::new(author_name).color(Color::Default))
.child(
Label::new(format!("Commit:{}", commit.sha))
.color(Color::Muted)
.size(LabelSize::Small)
.truncate()
.buffer_font(cx),
),
.child({
ButtonLike::new("sha")
.child(
h_flex()
.group("sha_btn")
.size_full()
.max_w_32()
.gap_0p5()
.child(
Label::new(commit_sha.clone())
.color(Color::Muted)
.size(LabelSize::Small)
.truncate()
.buffer_font(cx),
)
.child(
div().visible_on_hover("sha_btn").child(
Icon::new(copy_icon)
.color(copy_icon_color)
.size(IconSize::Small),
),
),
)
.tooltip({
let commit_sha = commit_sha.clone();
move |_, cx| {
Tooltip::with_meta(
"Copy Commit SHA",
None,
commit_sha.clone(),
cx,
)
}
})
.on_click(move |_, _, cx| {
cx.stop_propagation();
cx.write_to_clipboard(ClipboardItem::new_string(
commit_sha.to_string(),
));
})
}),
)
.child(
h_flex()

View File

@@ -53,7 +53,9 @@ async fn check_for_docker() -> Result<(), DevContainerError> {
}
}
async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, DevContainerError> {
async fn ensure_devcontainer_cli(
node_runtime: &NodeRuntime,
) -> Result<(PathBuf, bool), DevContainerError> {
let mut command = util::command::new_smol_command(&dev_container_cli());
command.arg("--version");
@@ -63,23 +65,42 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
e
);
let Ok(node_runtime_path) = node_runtime.binary_path().await else {
return Err(DevContainerError::NodeRuntimeNotAvailable);
};
let datadir_cli_path = paths::devcontainer_dir()
.join("node_modules")
.join(".bin")
.join(&dev_container_cli());
.join("@devcontainers")
.join("cli")
.join(format!("{}.js", &dev_container_cli()));
log::debug!(
"devcontainer not found in path, using local location: ${}",
datadir_cli_path.display()
);
let mut command =
util::command::new_smol_command(&datadir_cli_path.as_os_str().display().to_string());
util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
command.arg(datadir_cli_path.display().to_string());
command.arg("--version");
if let Err(e) = command.output().await {
log::error!(
match command.output().await {
Err(e) => log::error!(
"Unable to find devcontainer CLI in Data dir. Will try to install. Error: {:?}",
e
);
} else {
log::info!("Found devcontainer CLI in Data dir");
return Ok(datadir_cli_path.clone());
),
Ok(output) => {
if output.status.success() {
log::info!("Found devcontainer CLI in Data dir");
return Ok((datadir_cli_path.clone(), false));
} else {
log::error!(
"Could not run devcontainer CLI from data_dir. Will try once more to install. Output: {:?}",
output
);
}
}
}
if let Err(e) = fs::create_dir_all(paths::devcontainer_dir()).await {
@@ -101,7 +122,9 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
return Err(DevContainerError::DevContainerCliNotAvailable);
};
let mut command = util::command::new_smol_command(&datadir_cli_path.display().to_string());
let mut command =
util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
command.arg(datadir_cli_path.display().to_string());
command.arg("--version");
if let Err(e) = command.output().await {
log::error!(
@@ -110,22 +133,42 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
);
Err(DevContainerError::DevContainerCliNotAvailable)
} else {
Ok(datadir_cli_path)
Ok((datadir_cli_path, false))
}
} else {
log::info!("Found devcontainer cli on $PATH, using it");
Ok(PathBuf::from(&dev_container_cli()))
Ok((PathBuf::from(&dev_container_cli()), true))
}
}
async fn devcontainer_up(
path_to_cli: &PathBuf,
found_in_path: bool,
node_runtime: &NodeRuntime,
path: Arc<Path>,
) -> Result<DevContainerUp, DevContainerError> {
let mut command = util::command::new_smol_command(path_to_cli.display().to_string());
command.arg("up");
command.arg("--workspace-folder");
command.arg(path.display().to_string());
let Ok(node_runtime_path) = node_runtime.binary_path().await else {
log::error!("Unable to find node runtime path");
return Err(DevContainerError::NodeRuntimeNotAvailable);
};
let mut command = if found_in_path {
let mut command = util::command::new_smol_command(path_to_cli.display().to_string());
command.arg("up");
command.arg("--workspace-folder");
command.arg(path.display().to_string());
command
} else {
let mut command =
util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
command.arg(path_to_cli.display().to_string());
command.arg("up");
command.arg("--workspace-folder");
command.arg(path.display().to_string());
command
};
log::debug!("Running full devcontainer up command: {:?}", command);
match command.output().await {
Ok(output) => {
@@ -235,7 +278,7 @@ pub(crate) async fn start_dev_container(
) -> Result<(Connection, String), DevContainerError> {
check_for_docker().await?;
let path_to_devcontainer_cli = ensure_devcontainer_cli(node_runtime).await?;
let (path_to_devcontainer_cli, found_in_path) = ensure_devcontainer_cli(&node_runtime).await?;
let Some(directory) = project_directory(cx) else {
return Err(DevContainerError::DevContainerNotFound);
@@ -245,7 +288,13 @@ pub(crate) async fn start_dev_container(
container_id,
remote_workspace_folder,
..
}) = devcontainer_up(&path_to_devcontainer_cli, directory.clone()).await
}) = devcontainer_up(
&path_to_devcontainer_cli,
found_in_path,
&node_runtime,
directory.clone(),
)
.await
{
let project_name = get_project_name(
&path_to_devcontainer_cli,
@@ -273,6 +322,7 @@ pub(crate) enum DevContainerError {
DevContainerUpFailed,
DevContainerNotFound,
DevContainerParseFailed,
NodeRuntimeNotAvailable,
}
#[cfg(test)]

View File

@@ -158,6 +158,9 @@ fn handle_rpc_messages_over_child_process_stdio(
}
};
let status = remote_proxy_process.status().await?.code().unwrap_or(1);
if status != 0 {
anyhow::bail!("Remote server exited with status {status}");
}
match result {
Ok(_) => Ok(status),
Err(error) => Err(error),

View File

@@ -582,19 +582,21 @@ impl RemoteConnection for DockerExecConnection {
return Task::ready(Err(anyhow!("Remote binary path not set")));
};
let mut docker_args = vec![
"exec".to_string(),
"-w".to_string(),
self.remote_dir_for_server.clone(),
"-i".to_string(),
self.connection_options.container_id.to_string(),
];
let mut docker_args = vec!["exec".to_string()];
for env_var in ["RUST_LOG", "RUST_BACKTRACE", "ZED_GENERATE_MINIDUMPS"] {
if let Some(value) = std::env::var(env_var).ok() {
docker_args.push("-e".to_string());
docker_args.push(format!("{}='{}'", env_var, value));
}
}
docker_args.extend([
"-w".to_string(),
self.remote_dir_for_server.clone(),
"-i".to_string(),
self.connection_options.container_id.to_string(),
]);
let val = remote_binary_relpath
.display(self.path_style())
.into_owned();

View File

@@ -1065,73 +1065,43 @@ impl Element for TerminalElement {
// then have that representation be converted to the appropriate highlight data structure
let content_mode = self.terminal_view.read(cx).content_mode(window, cx);
let (rects, batched_text_runs) = match content_mode {
ContentMode::Scrollable => {
// In scrollable mode, the terminal already provides cells
// that are correctly positioned for the current viewport
// based on its display_offset. We don't need additional filtering.
TerminalElement::layout_grid(
cells.iter().cloned(),
0,
&text_style,
last_hovered_word.as_ref().map(|last_hovered_word| {
(link_style, &last_hovered_word.word_match)
}),
minimum_contrast,
cx,
)
}
ContentMode::Inline { .. } => {
let intersection = window.content_mask().bounds.intersect(&bounds);
let start_row = (intersection.top() - bounds.top()) / line_height_px;
let end_row = start_row + intersection.size.height / line_height_px;
let line_range = (start_row as i32)..=(end_row as i32);
// Calculate the intersection of the terminal's bounds with the current
// content mask (the visible viewport after all parent clipping).
// This allows us to only render cells that are actually visible, which is
// critical for performance when terminals are inside scrollable containers
// like the Agent Panel thread view.
//
// This optimization is analogous to the editor optimization in PR #45077
// which fixed performance issues with large AutoHeight editors inside Lists.
let visible_bounds = window.content_mask().bounds;
let intersection = visible_bounds.intersect(&bounds);
// If the terminal is entirely outside the viewport, skip all cell processing.
// This handles the case where the terminal has been scrolled past (above or
// below the viewport), similar to the editor fix in PR #45077 where start_row
// could exceed max_row when the editor was positioned above the viewport.
let (rects, batched_text_runs) = if intersection.size.height <= px(0.)
|| intersection.size.width <= px(0.)
{
(Vec::new(), Vec::new())
} else if intersection == bounds {
// Fast path: terminal fully visible, no clipping needed.
// Avoid grouping/allocation overhead by streaming cells directly.
TerminalElement::layout_grid(
cells.iter().cloned(),
0,
&text_style,
last_hovered_word
.as_ref()
.map(|last_hovered_word| (link_style, &last_hovered_word.word_match)),
minimum_contrast,
cx,
)
} else {
// Calculate which screen rows are visible based on pixel positions.
// This works for both Scrollable and Inline modes because we filter
// by screen position (enumerated line group index), not by the cell's
// internal line number (which can be negative in Scrollable mode for
// scrollback history).
let rows_above_viewport =
((intersection.top() - bounds.top()).max(px(0.)) / line_height_px) as usize;
let visible_row_count =
(intersection.size.height / line_height_px).ceil() as usize + 1;
// Group cells by line and filter to only the visible screen rows.
// skip() and take() work on enumerated line groups (screen position),
// making this work regardless of the actual cell.point.line values.
let visible_cells: Vec<_> = cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_above_viewport)
.take(visible_row_count)
.flat_map(|(_, line_cells)| line_cells)
.cloned()
.collect();
TerminalElement::layout_grid(
visible_cells.into_iter(),
rows_above_viewport as i32,
&text_style,
last_hovered_word
.as_ref()
.map(|last_hovered_word| (link_style, &last_hovered_word.word_match)),
minimum_contrast,
cx,
)
TerminalElement::layout_grid(
cells
.iter()
.skip_while(|i| &i.point.line < line_range.start())
.take_while(|i| &i.point.line <= line_range.end())
.cloned(),
*line_range.start(),
&text_style,
last_hovered_word.as_ref().map(|last_hovered_word| {
(link_style, &last_hovered_word.word_match)
}),
minimum_contrast,
cx,
)
}
};
// Layout cursor. Rectangle is used for IME, so we should lay it out even
@@ -2089,248 +2059,4 @@ mod tests {
let merged2 = merge_background_regions(regions2);
assert_eq!(merged2.len(), 3);
}
#[test]
fn test_screen_position_filtering_with_positive_lines() {
// Test the unified screen-position-based filtering approach.
// This works for both Scrollable and Inline modes because we filter
// by enumerated line group index, not by cell.point.line values.
use itertools::Itertools;
use terminal::IndexedCell;
use terminal::alacritty_terminal::index::{Column, Line, Point as AlacPoint};
use terminal::alacritty_terminal::term::cell::Cell;
// Create mock cells for lines 0-23 (typical terminal with 24 visible lines)
let mut cells = Vec::new();
for line in 0..24i32 {
for col in 0..3i32 {
cells.push(IndexedCell {
point: AlacPoint::new(Line(line), Column(col as usize)),
cell: Cell::default(),
});
}
}
// Scenario: Terminal partially scrolled above viewport
// First 5 lines (0-4) are clipped, lines 5-15 should be visible
let rows_above_viewport = 5usize;
let visible_row_count = 11usize;
// Apply the same filtering logic as in the render code
let filtered: Vec<_> = cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_above_viewport)
.take(visible_row_count)
.flat_map(|(_, line_cells)| line_cells)
.collect();
// Should have lines 5-15 (11 lines * 3 cells each = 33 cells)
assert_eq!(filtered.len(), 11 * 3, "Should have 33 cells for 11 lines");
// First filtered cell should be line 5
assert_eq!(
filtered.first().unwrap().point.line,
Line(5),
"First cell should be on line 5"
);
// Last filtered cell should be line 15
assert_eq!(
filtered.last().unwrap().point.line,
Line(15),
"Last cell should be on line 15"
);
}
#[test]
fn test_screen_position_filtering_with_negative_lines() {
// This is the key test! In Scrollable mode, cells have NEGATIVE line numbers
// for scrollback history. The screen-position filtering approach works because
// we filter by enumerated line group index, not by cell.point.line values.
use itertools::Itertools;
use terminal::IndexedCell;
use terminal::alacritty_terminal::index::{Column, Line, Point as AlacPoint};
use terminal::alacritty_terminal::term::cell::Cell;
// Simulate cells from a scrolled terminal with scrollback
// These have negative line numbers representing scrollback history
let mut scrollback_cells = Vec::new();
for line in -588i32..=-578i32 {
for col in 0..80i32 {
scrollback_cells.push(IndexedCell {
point: AlacPoint::new(Line(line), Column(col as usize)),
cell: Cell::default(),
});
}
}
// Scenario: First 3 screen rows clipped, show next 5 rows
let rows_above_viewport = 3usize;
let visible_row_count = 5usize;
// Apply the same filtering logic as in the render code
let filtered: Vec<_> = scrollback_cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_above_viewport)
.take(visible_row_count)
.flat_map(|(_, line_cells)| line_cells)
.collect();
// Should have 5 lines * 80 cells = 400 cells
assert_eq!(filtered.len(), 5 * 80, "Should have 400 cells for 5 lines");
// First filtered cell should be line -585 (skipped 3 lines from -588)
assert_eq!(
filtered.first().unwrap().point.line,
Line(-585),
"First cell should be on line -585"
);
// Last filtered cell should be line -581 (5 lines: -585, -584, -583, -582, -581)
assert_eq!(
filtered.last().unwrap().point.line,
Line(-581),
"Last cell should be on line -581"
);
}
#[test]
fn test_screen_position_filtering_skip_all() {
// Test what happens when we skip more rows than exist
use itertools::Itertools;
use terminal::IndexedCell;
use terminal::alacritty_terminal::index::{Column, Line, Point as AlacPoint};
use terminal::alacritty_terminal::term::cell::Cell;
let mut cells = Vec::new();
for line in 0..10i32 {
cells.push(IndexedCell {
point: AlacPoint::new(Line(line), Column(0)),
cell: Cell::default(),
});
}
// Skip more rows than exist
let rows_above_viewport = 100usize;
let visible_row_count = 5usize;
let filtered: Vec<_> = cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_above_viewport)
.take(visible_row_count)
.flat_map(|(_, line_cells)| line_cells)
.collect();
assert_eq!(
filtered.len(),
0,
"Should have no cells when all are skipped"
);
}
#[test]
fn test_layout_grid_positioning_math() {
// Test the math that layout_grid uses for positioning.
// When we skip N rows, we pass N as start_line_offset to layout_grid,
// which positions the first visible line at screen row N.
// Scenario: Terminal at y=-100px, line_height=20px
// First 5 screen rows are above viewport (clipped)
// So we skip 5 rows and pass offset=5 to layout_grid
let terminal_origin_y = -100.0f32;
let line_height = 20.0f32;
let rows_skipped = 5;
// The first visible line (at offset 5) renders at:
// y = terminal_origin + offset * line_height = -100 + 5*20 = 0
let first_visible_y = terminal_origin_y + rows_skipped as f32 * line_height;
assert_eq!(
first_visible_y, 0.0,
"First visible line should be at viewport top (y=0)"
);
// The 6th visible line (at offset 10) renders at:
let sixth_visible_y = terminal_origin_y + (rows_skipped + 5) as f32 * line_height;
assert_eq!(
sixth_visible_y, 100.0,
"6th visible line should be at y=100"
);
}
#[test]
fn test_unified_filtering_works_for_both_modes() {
// This test proves that the unified screen-position filtering approach
// works for BOTH positive line numbers (Inline mode) and negative line
// numbers (Scrollable mode with scrollback).
//
// The key insight: we filter by enumerated line group index (screen position),
// not by cell.point.line values. This makes the filtering agnostic to the
// actual line numbers in the cells.
use itertools::Itertools;
use terminal::IndexedCell;
use terminal::alacritty_terminal::index::{Column, Line, Point as AlacPoint};
use terminal::alacritty_terminal::term::cell::Cell;
// Test with positive line numbers (Inline mode style)
let positive_cells: Vec<_> = (0..10i32)
.flat_map(|line| {
(0..3i32).map(move |col| IndexedCell {
point: AlacPoint::new(Line(line), Column(col as usize)),
cell: Cell::default(),
})
})
.collect();
// Test with negative line numbers (Scrollable mode with scrollback)
let negative_cells: Vec<_> = (-10i32..0i32)
.flat_map(|line| {
(0..3i32).map(move |col| IndexedCell {
point: AlacPoint::new(Line(line), Column(col as usize)),
cell: Cell::default(),
})
})
.collect();
let rows_to_skip = 3usize;
let rows_to_take = 4usize;
// Filter positive cells
let positive_filtered: Vec<_> = positive_cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_to_skip)
.take(rows_to_take)
.flat_map(|(_, cells)| cells)
.collect();
// Filter negative cells
let negative_filtered: Vec<_> = negative_cells
.iter()
.chunk_by(|c| c.point.line)
.into_iter()
.skip(rows_to_skip)
.take(rows_to_take)
.flat_map(|(_, cells)| cells)
.collect();
// Both should have same count: 4 lines * 3 cells = 12
assert_eq!(positive_filtered.len(), 12);
assert_eq!(negative_filtered.len(), 12);
// Positive: lines 3, 4, 5, 6
assert_eq!(positive_filtered.first().unwrap().point.line, Line(3));
assert_eq!(positive_filtered.last().unwrap().point.line, Line(6));
// Negative: lines -7, -6, -5, -4
assert_eq!(negative_filtered.first().unwrap().point.line, Line(-7));
assert_eq!(negative_filtered.last().unwrap().point.line, Line(-4));
}
}