diff --git a/Cargo.toml b/Cargo.toml index be8d330..aa26ae3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ tokio = { version = "1.52.1", features = ["rt-multi-thread", "net", "time", "mac tree-sitter = "0.19.5" tree-sitter-markdown = "0.7.1" memchr = "2.8.0" +thiserror = "2.0.18" [build-dependencies] serde_json = "1.0.149" diff --git a/fixtures/github/issues.pull_request.PR_kwDOAgent47.json b/fixtures/github/issues.pull_request.PR_kwDOAgent47.json index 9e06a6c..24c83e2 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOAgent47.json +++ b/fixtures/github/issues.pull_request.PR_kwDOAgent47.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDOAgent47", "title": "feat(prompts): split context loading from execution workers", "state": "OPEN", "is_draft": true, diff --git a/fixtures/github/issues.pull_request.PR_kwDODesign31.json b/fixtures/github/issues.pull_request.PR_kwDODesign31.json index 2b9af72..3ee4e03 100644 --- a/fixtures/github/issues.pull_request.PR_kwDODesign31.json +++ b/fixtures/github/issues.pull_request.PR_kwDODesign31.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDODesign31", "title": "chore(tokens): tighten dashboard spacing scale", "state": "OPEN", "is_draft": false, diff --git a/fixtures/github/issues.pull_request.PR_kwDOInfra19.json b/fixtures/github/issues.pull_request.PR_kwDOInfra19.json index 1d45af2..7535245 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOInfra19.json +++ b/fixtures/github/issues.pull_request.PR_kwDOInfra19.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDOInfra19", "title": "docs(deploy): document manual failover steps", "state": "CLOSED", "is_draft": false, diff --git a/fixtures/github/issues.pull_request.PR_kwDONovem84.json b/fixtures/github/issues.pull_request.PR_kwDONovem84.json index 3748cf5..8f048cd 100644 --- a/fixtures/github/issues.pull_request.PR_kwDONovem84.json +++ b/fixtures/github/issues.pull_request.PR_kwDONovem84.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDONovem84", "title": "feat(dashboard): hydrate issue pane from cached query state", "state": "OPEN", "is_draft": false, diff --git a/fixtures/github/issues.pull_request.PR_kwDONovem85.json b/fixtures/github/issues.pull_request.PR_kwDONovem85.json index 83ec988..e8a598d 100644 --- a/fixtures/github/issues.pull_request.PR_kwDONovem85.json +++ b/fixtures/github/issues.pull_request.PR_kwDONovem85.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDONovem85", "title": "feat(repo): add cached repository query for titlebar picker", "state": "OPEN", "is_draft": false, diff --git a/fixtures/github/issues.pull_request.PR_kwDOSprint62.json b/fixtures/github/issues.pull_request.PR_kwDOSprint62.json index 703df91..d713ab9 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOSprint62.json +++ b/fixtures/github/issues.pull_request.PR_kwDOSprint62.json @@ -1,4 +1,5 @@ { + "id": "PR_kwDOSprint62", "title": "feat(calendar): ship release handoff checklist in weekly planner", "state": "MERGED", "is_draft": false, diff --git a/fixtures/github/issues.pull_request_file_tree.PR_kwDONovem85.json b/fixtures/github/issues.pull_request_file_tree.PR_kwDONovem85.json new file mode 100644 index 0000000..097ccb4 --- /dev/null +++ b/fixtures/github/issues.pull_request_file_tree.PR_kwDONovem85.json @@ -0,0 +1,19 @@ +{ + "node": { + "__typename": "PullRequest", + "files": { + "edges": [ + { + "cursor": "file:PR_kwDONovem85:1", + "node": { + "changeType": "MODIFIED", + "additions": 42, + "deletions": 7, + "path": "src/api/repo.rs", + "viewerViewedState": "UNVIEWED" + } + } + ] + } + } +} diff --git a/src/api.rs b/src/api.rs index 241c2ee..2765410 100644 --- a/src/api.rs +++ b/src/api.rs @@ -33,20 +33,29 @@ pub(crate) struct GithubCredentials { pub(crate) client_id: &'static str, } -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub(crate) enum Error { + #[error("unauthenticated api request")] Unauthenticated, + #[error("request not allowed")] NotAllowed, + #[error("requested resource does not exist")] DoesNotExist, #[cfg(debug_assertions)] + #[error("missing mock fixture for {0:?}")] MissingMockFixture(String), + #[error(transparent)] Github(GithubError), + #[error("malformed response")] MalformedResponse(String), - HttpError(reqwest::Error), + #[error("generic http error: {0:?}")] + HttpError(#[from] reqwest::Error), + #[error("graphql api error: {0:?}")] GraphQLError(Vec), } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, thiserror::Error)] +#[error("github error {error:?}: {error_description:?}")] pub(crate) struct GithubError { pub error: String, pub error_description: Option, @@ -96,12 +105,6 @@ pub(crate) fn use_github_fixtures() -> bool { impl query::Context for QueryContext {} -impl From for Error { - fn from(value: reqwest::Error) -> Self { - Self::HttpError(value) - } -} - impl From for Error { fn from(value: serde_json::Error) -> Self { Self::MalformedResponse(value.to_string()) diff --git a/src/api/graphql/fetch_pull_request.graphql b/src/api/graphql/fetch_pull_request.graphql index 36bba12..ada56ea 100644 --- a/src/api/graphql/fetch_pull_request.graphql +++ b/src/api/graphql/fetch_pull_request.graphql @@ -2,6 +2,7 @@ query PullRequestQuery($id: ID!) { node(id: $id) { __typename ... on PullRequest { + id title body state diff --git a/src/api/issues.rs b/src/api/issues.rs index a4ac271..9d2b03f 100644 --- a/src/api/issues.rs +++ b/src/api/issues.rs @@ -41,6 +41,7 @@ pub(crate) struct PullRequest { #[derive(Debug, Deserialize)] pub(crate) struct DetailedPullRequest { + pub(crate) id: Id, pub(crate) title: Arc, pub(crate) state: PullRequestState, pub(crate) is_draft: bool, @@ -189,7 +190,7 @@ pub(crate) struct ChangedFile { pub(crate) change_type: ChangeType, pub(crate) additions: i64, pub(crate) deletions: i64, - pub(crate) path: String, + pub(crate) path: Arc, pub(crate) viewer_viewed_state: FileViewedState, } @@ -408,6 +409,7 @@ impl query::QueryFn for FetchPullRequest { })?; Ok(DetailedPullRequest { + id: Id(p.id.into()), title: p.title.into(), state: p.state, is_draft: p.is_draft, @@ -531,7 +533,7 @@ impl query::QueryFn for FetchPullRequestFileTree { }, additions: node.additions, deletions: node.deletions, - path: node.path, + path: node.path.into(), viewer_viewed_state: node.viewer_viewed_state, }) }) diff --git a/src/api/mock.rs b/src/api/mock.rs index 9a8485a..bc9dfad 100644 --- a/src/api/mock.rs +++ b/src/api/mock.rs @@ -376,6 +376,29 @@ mod tests { ) .unwrap_or_else(|_| panic!("head fixture should exist for {path}")); } + + let _ = fetch_pull_request_file_tree(&issues::Id::from("PR_kwDONovem85")) + .expect("repo picker pull request file tree fixture should parse"); + + let repo_picker_file_tree_json: serde_json::Value = serde_json::from_str( + issues_pull_request_file_tree("PR_kwDONovem85") + .expect("repo picker pull request file tree fixture json should exist"), + ) + .expect("repo picker pull request file tree fixture json should parse"); + + let repo_picker_file_paths = repo_picker_file_tree_json + .get("node") + .and_then(|node| node.get("files")) + .and_then(|files| files.get("edges")) + .and_then(serde_json::Value::as_array) + .expect("repo picker pull request file tree fixture should contain file edges") + .iter() + .filter_map(|edge| edge.get("node")) + .filter_map(|node| node.get("path")) + .filter_map(serde_json::Value::as_str) + .collect::>(); + + assert_eq!(repo_picker_file_paths, vec!["src/api/repo.rs"]); } #[test] diff --git a/src/api/repo.rs b/src/api/repo.rs index f84d1e7..09aa290 100644 --- a/src/api/repo.rs +++ b/src/api/repo.rs @@ -78,8 +78,8 @@ impl query::QueryFn for FetchFileContent { fn key(&self) -> query::Key { match &self.reff { - | Some(reff) => format!("repo/fetch/{}/{}/{}", self.repo_slug, self.path, reff).into(), - | None => format!("repo/fetch/{}/{}", self.repo_slug, self.path).into(), + | Some(reff) => format!("repo/fetch/{}/{}/{}", self.repo_slug, self.path, reff).into(), + | None => format!("repo/fetch/{}/{}", self.repo_slug, self.path).into(), } } @@ -94,11 +94,11 @@ impl query::QueryFn for FetchFileContent { } let path = match &self.reff { - | Some(reff) => format!( - "/repos/{}/contents/{}?ref={}", - self.repo_slug, self.path, reff - ), - | None => format!("/repos/{}/contents/{}", self.repo_slug, self.path), + | Some(reff) => format!( + "/repos/{}/contents/{}?ref={}", + self.repo_slug, self.path, reff + ), + | None => format!("/repos/{}/contents/{}", self.repo_slug, self.path), }; let res = c @@ -117,9 +117,18 @@ pub struct FetchFileDiff { pub head: FileRef, } +#[derive(Debug, thiserror::Error)] +pub enum FetchFileDiffError { + #[error("api error when fetching file diff: {0:?}")] + ApiError(#[from] api::Error), + + #[error("invalid utf8 content or unsupported file type")] + InvalidTextContent, +} + impl query::QueryFn for FetchFileDiff { - type Data = util::diff::ContentDiff; - type Error = api::Error; + type Data = Arc; + type Error = FetchFileDiffError; type Context = api::QueryContext; fn key(&self) -> query::Key { @@ -134,35 +143,108 @@ impl query::QueryFn for FetchFileDiff { async fn fetch_content( r: &FileRef, c: &::Context, - ) -> Result, api::Error> { - let path = match &r.reff { - | Some(reff) => format!("/repos/{}/contents/{}?ref={}", r.repo_slug, r.path, reff), - | None => format!("/repos/{}/contents/{}", r.repo_slug, r.path), + ) -> Result { + #[cfg(debug_assertions)] + let bytes = if c.should_use_fixtures { + super::mock::fetch_file_content(&r.repo_slug, &r.path, r.reff.as_deref())? + } else { + let path = match &r.reff { + | Some(reff) => { + format!("/repos/{}/contents/{}?ref={}", r.repo_slug, r.path, reff) + } + | None => format!("/repos/{}/contents/{}", r.repo_slug, r.path), + }; + + let res = c + .github_request(Method::GET, &path)? + .header("Accept", "application/vnd.github.raw+json") + .send() + .await + .map_err(api::Error::HttpError)?; + + api::raw_content(res).await? }; - let res = c - .github_request(Method::GET, &path)? - .header("Accept", "application/vnd.github.raw+json") - .send() - .await?; + #[cfg(not(debug_assertions))] + let bytes = { + let path = match &r.reff { + | Some(reff) => { + format!("/repos/{}/contents/{}?ref={}", r.repo_slug, r.path, reff) + } + | None => format!("/repos/{}/contents/{}", r.repo_slug, r.path), + }; - res.headers().get("Content-Type"); + let res = c + .github_request(Method::GET, &path)? + .header("Accept", "application/vnd.github.raw+json") + .send() + .await + .map_err(api::Error::HttpError)?; - let bytes = api::raw_content(res).await?; - let file::ContentType::Text = file::classify_content(&bytes) else { - return Ok(None); + api::raw_content(res).await? }; - Ok(Some(bytes)) + match file::classify_content(&bytes) { + | file::ContentType::Text => Ok(bytes), + | _ => Err(FetchFileDiffError::InvalidTextContent), + } } - let (old, new) = tokio::join!(fetch_content(&self.base, c), fetch_content(&self.head, c),); + let (old, new) = + tokio::try_join!(fetch_content(&self.base, c), fetch_content(&self.head, c),)?; - match (old, new) { - | (Ok(Some(old)), Ok(Some(new))) => Ok(util::diff::diff_content(old, new)), - | _ => Err(api::Error::MalformedResponse( - "failed to fetch content".to_string(), - )), - } + util::diff::diff_content(old, new) + .map(|diff| Arc::new(diff)) + .ok_or(FetchFileDiffError::InvalidTextContent) + } +} + +#[cfg(test)] +mod tests { + use crate::query::QueryFn; + + use super::*; + + #[tokio::test] + async fn fetch_file_diff_uses_repo_file_content_fixtures() { + let pull_request = + super::super::mock::fetch_pull_request(&api::issues::Id::from("PR_kwDONovem84")) + .expect("pull request fixture should parse"); + + let diff = FetchFileDiff { + base: FileRef { + repo_slug: pull_request.base_repo_slug.clone(), + path: Arc::from("src/query.rs"), + reff: Some(pull_request.base_ref.clone()), + }, + head: FileRef { + repo_slug: pull_request.head_repo_slug.clone(), + path: Arc::from("src/query.rs"), + reff: Some(pull_request.head_ref.clone()), + }, + } + .run(&api::QueryContext { + http: reqwest::Client::new(), + auth: None, + github: api::GithubCredentials { + base_url: "", + client_id: "", + }, + should_use_fixtures: true, + }) + .await + .expect("fetch file diff should succeed from fixtures"); + + assert!(diff.len() > 0); + assert!( + (0..diff.len()) + .filter_map(|i| diff.get(i).old_content.as_deref()) + .any(|line| { line.contains("pub struct CachedSelection") }) + ); + assert!( + (0..diff.len()) + .filter_map(|i| diff.get(i).new_content.as_deref()) + .any(|line| { line.contains("pub struct CachedQueryState") }) + ); } } diff --git a/src/component/code_view.rs b/src/component/code_view.rs index 55e5703..9fdc86a 100644 --- a/src/component/code_view.rs +++ b/src/component/code_view.rs @@ -1,108 +1,158 @@ -use std::{rc::Rc, sync::Arc}; +use std::{num::NonZeroUsize, rc::Rc, sync::Arc}; -use gpui::{IntoElement, ParentElement, Styled, div, list, px, rems}; +use gpui::{ + IntoElement, ParentElement, Refineable, Styled, div, list, prelude::FluentBuilder, px, rems, +}; use crate::app; #[derive(gpui::IntoElement, Clone)] -pub(crate) struct Line { - line_number_col_width: gpui::Pixels, - line_number: usize, - content: gpui::SharedString, - diff_marker: DiffMarker, +pub(crate) struct CodeLine { + line_number: Option, + content: Option, + diff_marker: CodeLineMarker, + gutter_width: gpui::Pixels, + style: gpui::StyleRefinement, } #[derive(Clone)] -enum DiffMarker { +pub(crate) enum CodeLineMarker { Added, Deleted, Unchanged, + Placeholder, } - #[derive(Clone)] -struct CodeViewState(gpui::ListState); +pub(crate) struct CodeViewState(gpui::ListState); -#[derive(Clone)] -struct Lines(Rc>); - -struct CodeView { +pub(crate) struct CodeView { state: CodeViewState, - lines: Lines, + content: CodeViewContent, } -pub(crate) fn line( - line_number: usize, - content: impl Into>, - diff_marker: DiffMarker, -) -> Line { - Line { - line_number, - diff_marker, - content: gpui::SharedString::new(content), - line_number_col_width: px(0.), +pub(crate) struct CodeViewContent { + lines: Rc<[CodeLine]>, +} + +pub(crate) fn code_view(state: CodeViewState, content: CodeViewContent) -> CodeView { + CodeView { state, content } +} + +pub(crate) fn code_line( + line_index: Option, + content: Option, + marker: CodeLineMarker, +) -> CodeLine { + CodeLine { + line_number: line_index.map(|i| unsafe { NonZeroUsize::new_unchecked(i + 1) }), + content, + diff_marker: marker, + gutter_width: px(0.), + style: gpui::StyleRefinement::default(), } } -pub(crate) fn code_view(state: CodeViewState, lines: Lines) -> CodeView { - CodeView { state, lines } +impl CodeViewContent { + pub(crate) fn new(lines: Vec) -> Self { + Self { + lines: lines.into(), + } + } } -impl FromIterator for Lines { - fn from_iter>(iter: T) -> Self { - Lines(Rc::new(iter.into_iter().collect())) +impl CodeLine { + pub(crate) fn gutter_width(mut self, width: gpui::Pixels) -> Self { + self.gutter_width = width; + self } } impl gpui::RenderOnce for CodeView { fn render(self, window: &mut gpui::Window, cx: &mut gpui::App) -> impl gpui::IntoElement { let digits = self + .content .lines - .0 - .last() - .map(|l| l.line_number.to_string().len()) + .iter() + .rfind(|l| l.line_number.is_some()) + .map(|l| l.line_number.unwrap().to_string().len()) .unwrap_or(0); let text_style = window.text_style(); let font_size = text_style.font_size.to_pixels(window.rem_size()); let font_id = window.text_system().resolve_font(&gpui::font("Menlo")); - let line_number_col_width = window + let gutter_width = window .text_system() .ch_advance(font_id, font_size) .unwrap_or(px(7.2)) * digits; - list(self.state.0, move |i, _window, _app| { - let mut line = self.lines.0[i].clone(); - line.line_number_col_width = line_number_col_width; + println!("gutter width {}", gutter_width); + list(self.state.0, move |i, _window, _app| { + let line = self.content.lines[i].clone(); div() .flex() .flex_row() .items_start() .w_full() - .child(line) + .child(line.gutter_width(gutter_width)) .into_any_element() }) } } -impl gpui::RenderOnce for Line { +impl gpui::Styled for CodeLine { + #[doc = " Returns a reference to the style memory of this element."] + fn style(&mut self) -> &mut gpui::StyleRefinement { + &mut self.style + } +} + +impl gpui::RenderOnce for CodeLine { fn render(self, _window: &mut gpui::Window, cx: &mut gpui::App) -> impl IntoElement { let theme = app::current_theme(cx); - div() + let mut div = div() .flex() .flex_row() .font_family("Menlo") .text_color(theme.colors.text) + .text_xs() .child( div() .bg(theme.colors.surface) - .w(self.line_number_col_width) + .w(self.gutter_width + px(16.)) .text_align(gpui::TextAlign::Right) - .child(self.line_number.to_string()), + .px_2() + .when_some(self.line_number, |it, line_number| { + it.child(line_number.to_string()) + }) + .when(matches!(self.diff_marker, CodeLineMarker::Added), |it| { + it.bg(theme.colors.success_muted) + .text_color(theme.colors.success_fg) + .border_l_2() + .border_color(theme.colors.success_border) + }) + .when(matches!(self.diff_marker, CodeLineMarker::Deleted), |it| { + it.bg(theme.colors.danger_muted) + .text_color(theme.colors.danger_fg) + .border_l_2() + .border_color(theme.colors.danger_border) + }), ) - .child(self.content) + .when_some(self.content, |it, content| { + it.child(div().px_2().w_full().min_w_0().child(content)) + }) + .when(matches!(self.diff_marker, CodeLineMarker::Added), |it| { + it.bg(theme.colors.success_muted) + }) + .when(matches!(self.diff_marker, CodeLineMarker::Deleted), |it| { + it.bg(theme.colors.danger_muted) + }); + + div.style().refine(&self.style); + + div } } diff --git a/src/component/diff_view.rs b/src/component/diff_view.rs new file mode 100644 index 0000000..828a321 --- /dev/null +++ b/src/component/diff_view.rs @@ -0,0 +1,138 @@ +use std::sync::Arc; + +use gpui::{IntoElement, ParentElement, Styled, div, list, px}; + +use crate::{ + component::code_view::{self, CodeLine, code_line}, + util::{self, str::ToSharedString}, +}; + +#[derive(gpui::IntoElement)] +pub(crate) struct DiffView { + state: DiffViewState, + content: DiffViewContent, +} + +#[derive(Clone)] +pub(crate) struct DiffViewState(gpui::ListState); + +#[derive(Clone)] +pub(crate) struct DiffViewContent { + diff: Arc, +} + +#[derive(Clone, gpui::IntoElement)] +struct DiffRow { + line: util::diff::DiffLine, + old_side_gutter_width: gpui::Pixels, + new_side_gutter_width: gpui::Pixels, +} + +pub(crate) fn diff_view(state: DiffViewState, content: DiffViewContent) -> DiffView { + DiffView { state, content } +} + +impl From> for DiffViewContent { + fn from(value: Arc) -> Self { + Self { diff: value } + } +} + +impl DiffViewState { + pub(crate) fn new() -> Self { + Self(gpui::ListState::new(0, gpui::ListAlignment::Top, px(100.))) + } + + pub(crate) fn reset(&mut self, line_count: usize) { + self.0.reset(line_count); + } +} + +impl gpui::RenderOnce for DiffView { + fn render(self, window: &mut gpui::Window, cx: &mut gpui::App) -> impl gpui::IntoElement { + let (old_digits, new_digits) = self + .content + .diff + .last() + .map(|l| (l.old_line.to_string().len(), l.new_line.to_string().len())) + .unwrap_or((1, 1)); + + let text_style = window.text_style(); + let font_size = text_style.font_size.to_pixels(window.rem_size()); + let font_id = window.text_system().resolve_font(&gpui::font("Menlo")); + + let ch = window + .text_system() + .ch_advance(font_id, font_size) + .unwrap_or(px(7.2)); + + let old_side_gutter_width = ch * old_digits; + let new_side_gutter_width = ch * new_digits; + + list(self.state.0, move |i, _, cx| { + DiffRow { + line: self.content.diff.get(i).clone(), + old_side_gutter_width, + new_side_gutter_width, + } + .into_any_element() + }) + .bg(gpui::red()) + .size_full() + } +} + +impl DiffRow { + fn old_code_line(&self) -> CodeLine { + code_line( + Some(self.line.old_line), + self.line + .old_content + .as_ref() + .map(|it| it.to_shared_string()), + match self.line.op { + | util::diff::Op::Equal => code_view::CodeLineMarker::Unchanged, + | util::diff::Op::Insert => code_view::CodeLineMarker::Placeholder, + | util::diff::Op::Replace | util::diff::Op::Delete => { + code_view::CodeLineMarker::Deleted + } + }, + ) + } + + fn new_code_line(&self) -> CodeLine { + code_line( + Some(self.line.new_line), + self.line + .new_content + .as_ref() + .map(|it| it.to_shared_string()), + match self.line.op { + | util::diff::Op::Equal => code_view::CodeLineMarker::Unchanged, + | util::diff::Op::Insert | util::diff::Op::Replace => code_view::CodeLineMarker::Added, + | util::diff::Op::Delete => code_view::CodeLineMarker::Deleted, + }, + ) + } +} + +impl gpui::RenderOnce for DiffRow { + fn render(self, window: &mut gpui::Window, cx: &mut gpui::App) -> impl gpui::IntoElement { + div() + .w_full() + .flex() + .flex_row() + .child( + self.old_code_line() + .gutter_width(self.old_side_gutter_width) + .min_w_0() + .flex_1(), + ) + .child( + self.new_code_line() + .gutter_width(self.new_side_gutter_width) + .min_w_0() + .flex_1(), + ) + } +} diff --git a/src/component/mod.rs b/src/component/mod.rs index bef24f4..56ed26b 100644 --- a/src/component/mod.rs +++ b/src/component/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod button; pub(crate) mod code_view; +pub(crate) mod diff_view; pub(crate) mod font_icon; pub(crate) mod markdown; pub(crate) mod text; diff --git a/src/query.rs b/src/query.rs index 9e8c631..aaa5f74 100644 --- a/src/query.rs +++ b/src/query.rs @@ -137,13 +137,13 @@ where })?; match wait_state { - | WaitState::Cached => { - return Ok(ent); - } - | WaitState::Waiting { rx, sub } => { - _ = sub; - _ = rx.await; - } + | WaitState::Cached => { + return Ok(ent); + } + | WaitState::Waiting { rx, sub } => { + _ = sub; + _ = rx.await; + } } } } @@ -181,12 +181,27 @@ where let state = query.raw.read(cx); match &state.data { - | QueryData::Loading | QueryData::Pending | QueryData::Stale => QueryStatus::Loading, - | QueryData::Some(data) => QueryStatus::Loaded(data.downcast_ref::().unwrap()), - | QueryData::Err(error) => QueryStatus::Err(error.downcast_ref::().unwrap()), + | QueryData::Loading | QueryData::Pending | QueryData::Stale => QueryStatus::Loading, + | QueryData::Some(data) => QueryStatus::Loaded(data.downcast_ref::().unwrap()), + | QueryData::Err(error) => QueryStatus::Err(error.downcast_ref::().unwrap()), } } +pub fn observe_query( + query: &Entity, + mut on_notify: impl FnMut(&mut E, &Entity, &mut gpui::Context) + 'static, + cx: &mut gpui::Context, +) -> gpui::Subscription +where + E: 'static, + F: QueryFn, +{ + let q = query.clone(); + cx.observe(&query, move |this, _, cx| { + on_notify(this, &q, cx); + }) +} + // ================= Store ================== pub(crate) trait Context: Clone {} @@ -284,14 +299,14 @@ where entity.raw.update(cx, |state, cx| { state.data = match result { - | Ok(data) => { - println!("[query] OK {}", q.key()); - QueryData::Some(Box::new(data)) - } - | Err(err) => { - println!("[query] ERR {:?}: {:?}", q.key(), err); - QueryData::Err(Box::new(err)) - } + | Ok(data) => { + println!("[query] OK {}", q.key()); + QueryData::Some(Box::new(data)) + } + | Err(err) => { + println!("[query] ERR {:?}: {:?}", q.key(), err); + QueryData::Err(Box::new(err)) + } }; cx.notify(); })?; @@ -317,8 +332,8 @@ where .raw .update(cx, |query, cx| { query.data = match result { - | Ok(data) => QueryData::Some(Box::new(data)), - | Err(err) => QueryData::Err(Box::new(err)), + | Ok(data) => QueryData::Some(Box::new(data)), + | Err(err) => QueryData::Err(Box::new(err)), }; cx.notify(); true diff --git a/src/screen/dashboard/pull_request_diff_view.rs b/src/screen/dashboard/pull_request_diff_view.rs index 839051b..4c3badc 100644 --- a/src/screen/dashboard/pull_request_diff_view.rs +++ b/src/screen/dashboard/pull_request_diff_view.rs @@ -1,21 +1,43 @@ +use std::sync::Arc; + use crate::{ api, app, - component::text::text, - query::{self, QueryStatus, read_query, use_query}, + component::{ + diff_view::{DiffViewContent, DiffViewState, diff_view}, + text::text, + }, + query::{self, QueryStatus, observe_query, read_query, use_query}, }; +use gpui::{ParentElement, Styled, div}; pub(crate) struct PullRequestDiffView { - selected_file_path: Option, + selected_file_path: Option>, pr_query: query::Entity, + file_tree_query: query::Entity, content_diff_query: Option>, + + diff_view_state: DiffViewState, + diff_view_content: Option, } -fn new(pr_id: api::issues::Id, cx: &mut gpui::Context) -> PullRequestDiffView { +pub(crate) fn new( + pr_id: api::issues::Id, + cx: &mut gpui::Context, +) -> PullRequestDiffView { let mut view = PullRequestDiffView { selected_file_path: None, - pr_query: use_query(api::issues::FetchPullRequest { id: pr_id }, cx), + pr_query: use_query(api::issues::FetchPullRequest { id: pr_id.clone() }, cx), + file_tree_query: use_query( + api::issues::FetchPullRequestFileTree { + id: pr_id, + first: 100, + }, + cx, + ), content_diff_query: None, + diff_view_state: DiffViewState::new(), + diff_view_content: None, }; view.on_create(cx); view @@ -29,22 +51,42 @@ impl PullRequestDiffView { }) .detach(); + _ = cx + .observe(&self.file_tree_query, |this, _, cx| { + this.start_content_queries(cx); + }) + .detach(); + // if pr is already loaded, start content queries self.start_content_queries(cx); } fn start_content_queries(&mut self, cx: &mut gpui::Context) { + if self.content_diff_query.is_some() { + return; + } + + if self.selected_file_path.is_none() + && let QueryStatus::Loaded(files) = read_query(&self.file_tree_query, cx) + { + self.selected_file_path = files.first().map(|file| Arc::clone(&file.path)); + } + + let Some(selected_file_path) = self.selected_file_path.as_deref() else { + return; + }; + let Some((old_file_ref, new_file_ref)) = ({ if let QueryStatus::Loaded(pr) = read_query(&self.pr_query, cx) { Some(( api::repo::FileRef { repo_slug: pr.base_repo_slug.clone(), - path: pr.base_branch_name.clone(), + path: Arc::from(selected_file_path), reff: Some(pr.base_ref.clone()), }, api::repo::FileRef { repo_slug: pr.head_repo_slug.clone(), - path: pr.head_branch_name.clone(), + path: Arc::from(selected_file_path), reff: Some(pr.head_ref.clone()), }, )) @@ -63,6 +105,20 @@ impl PullRequestDiffView { cx, ); + _ = observe_query( + &content_diff_query, + |this, query, cx| { + if let QueryStatus::Loaded(diff) = read_query(query, cx) { + println!("diff len {}", diff.len()); + this.diff_view_state.reset(diff.len()); + this.diff_view_content = Some(Arc::clone(diff).into()); + } + cx.notify(); + }, + cx, + ) + .detach(); + self.content_diff_query = Some(content_diff_query); } } @@ -73,20 +129,27 @@ impl gpui::Render for PullRequestDiffView { _window: &mut gpui::Window, cx: &mut gpui::Context, ) -> impl gpui::IntoElement { - use gpui::{ParentElement, Styled, div}; - let theme = app::current_theme(cx); - div() + let content_diff = self + .content_diff_query + .as_ref() + .map(|q| read_query(q, cx)) + .unwrap_or(QueryStatus::Loading); + + match content_diff { + | QueryStatus::Err(_) | QueryStatus::Loading => div() .size_full() .bg(theme.colors.surface) .p_4() - .child( - text( - "Pull request diff rendering is still under construction. Launch the DiffOps playground with NOVEM_DIFFOPS_PLAYGROUND=1 cargo run.", - ) - .text_sm() - .text_color(theme.colors.text_muted), - ) + .child(text("asd")), + + | QueryStatus::Loaded(_) => match &self.diff_view_content { + | Some(content) => div() + .size_full() + .child(diff_view(self.diff_view_state.clone(), content.clone())), + | None => div(), + }, + } } } diff --git a/src/screen/dashboard/pull_request_view.rs b/src/screen/dashboard/pull_request_view.rs index f64c637..d9f853a 100644 --- a/src/screen/dashboard/pull_request_view.rs +++ b/src/screen/dashboard/pull_request_view.rs @@ -15,7 +15,7 @@ use crate::{ text::text, }, query::{self, QueryStatus, read_query, use_query}, - screen::dashboard::pull_request_diff_view::PullRequestDiffView, + screen::dashboard::pull_request_diff_view::{self, PullRequestDiffView}, }; pub(crate) struct PullRequestView { @@ -49,12 +49,14 @@ impl PullRequestView { _ = cx .observe(&query.clone(), move |this, _, cx| { this.load_markdown_content(cx); + this.load_pr_diff(cx); }) .detach(); // cached query will not trigger observe callback // this is required so that content is loaded immediately for cached query self.load_markdown_content(cx); + self.load_pr_diff(cx); cx.notify(); } @@ -78,6 +80,25 @@ impl PullRequestView { cx.notify(); } + fn load_pr_diff(&mut self, cx: &mut gpui::Context) { + let Some(query) = &self.pull_request_query else { + return; + }; + + let pr_id = { + let data = read_query(&query, cx); + if let QueryStatus::Loaded(pr) = data { + Some(pr.id.clone()) + } else { + None + } + }; + + self.diff_view = pr_id.map(|id| cx.new(|cx| pull_request_diff_view::new(id, cx))); + + cx.notify(); + } + fn pr_content( &self, pr: &api::issues::DetailedPullRequest, @@ -264,7 +285,11 @@ impl gpui::Render for PullRequestView { ) -> impl gpui::IntoElement { div().size_full().child(match &self.pull_request_query { | Some(q) => match read_query(q, cx) { - | QueryStatus::Loaded(pr) => self.pr_content(pr, cx), + | QueryStatus::Loaded(pr) => match &self.diff_view { + | Some(v) => v.clone().into_any_element(), + | None => self.pr_content(pr, cx), + }, + | QueryStatus::Err(e) => div() .size_full() .child(format!("{:?}", e)) @@ -274,6 +299,7 @@ impl gpui::Render for PullRequestView { .child("loading pr content") .into_any_element(), }, + | None => div().size_full().child("no pr selected").into_any_element(), }) } diff --git a/src/screen/dashboard/screen.rs b/src/screen/dashboard/screen.rs index f6f8c9c..984640e 100644 --- a/src/screen/dashboard/screen.rs +++ b/src/screen/dashboard/screen.rs @@ -4,6 +4,7 @@ use crate::{ api, app, screen::dashboard::{ issue_list::{self, IssueList}, + pull_request_diff_view::{self, PullRequestDiffView}, pull_request_view::{self, PullRequestView}, titlebar::{self, TitleBar}, }, @@ -13,6 +14,7 @@ pub(crate) struct Screen { titlebar: gpui::Entity, issue_list: gpui::Entity, pull_request_view: gpui::Entity, + pull_request_diff_view: Option>, issue_filter: Option<&'static str>, } @@ -22,6 +24,7 @@ pub(crate) fn new(cx: &mut gpui::Context) -> Screen { titlebar: cx.new(titlebar::new), issue_list: cx.new(issue_list::new), pull_request_view: cx.new(pull_request_view::new), + pull_request_diff_view: None, issue_filter: None, }; @@ -33,9 +36,9 @@ impl Screen { fn on_create(&mut self, cx: &mut gpui::Context) { _ = cx .subscribe(&self.issue_list, |this, _, event, cx| match event { - | issue_list::Event::ItemSelected(pr_id) => { - this.handle_issue_list_item_selected(pr_id, cx); - } + | issue_list::Event::ItemSelected(pr_id) => { + this.handle_issue_list_item_selected(pr_id, cx); + } }) .detach(); } @@ -50,7 +53,9 @@ impl Screen { view.change_displayed_pull_request(id.clone(), cx); println!("change displayed pull request: {:?}", id); cx.notify(); - }) + }); + self.pull_request_diff_view = + Some(cx.new(|cx| pull_request_diff_view::new(id.clone(), cx))); } } diff --git a/src/screen/diffops_playground.rs b/src/screen/diffops_playground.rs index c109c22..eda3a93 100644 --- a/src/screen/diffops_playground.rs +++ b/src/screen/diffops_playground.rs @@ -1,7 +1,10 @@ +use std::{ops::Range, sync::Arc}; + use bytes::Bytes; use gpui::{ AnyElement, AppContext, InteractiveElement, IntoElement, ParentElement, - StatefulInteractiveElement, Styled, div, point, px, size, + StatefulInteractiveElement, + Styled, div, point, px, size, }; use crate::{ @@ -10,7 +13,7 @@ use crate::{ button::{self, button}, text::text, }, - util::diff::{ContentDiff, DiffRow, DiffSide, Op, Span, diff_content}, + util::diff::{ContentDiff, DiffLine, Op, diff_content}, }; pub(crate) fn is_enabled() -> bool { @@ -51,7 +54,29 @@ pub(crate) struct Screen { struct DiffCase { title: &'static str, description: &'static str, - diff: ContentDiff, + old_lines: Vec, + new_lines: Vec, + op_groups: Vec, +} + +#[derive(Clone)] +struct SourceLine { + line_number: usize, + content: Arc, +} + +#[derive(Clone)] +struct OpGroup { + op: Op, + old_range: Range, + new_range: Range, + rows: Vec, +} + +#[derive(Clone, Copy)] +enum SourceSide { + Old, + New, } fn new(_cx: &mut gpui::Context) -> Screen { @@ -108,28 +133,17 @@ impl gpui::Render for Screen { .collect(); let op_cards: Vec = case - .diff - .spans() + .op_groups .iter() .enumerate() - .map(|(index, span)| render_op_card(index, span, theme).into_any_element()) + .map(|(index, group)| render_op_card(index, group, theme).into_any_element()) .collect(); let op_groups: Vec = case - .diff - .spans() + .op_groups .iter() .enumerate() - .map(|(index, span)| { - render_op_group( - index, - span, - case.diff.rows_for_span(index), - &case.diff, - theme, - ) - .into_any_element() - }) + .map(|(index, group)| render_op_group(index, group, theme).into_any_element()) .collect(); div() @@ -160,7 +174,7 @@ impl gpui::Render for Screen { .child(text("DiffOps Playground").text_lg()) .child( text( - "Sample content is diffed once at startup, then the UI renders the stored DiffOps and aligned rows.", + "Sample content is diffed once at startup, then the UI derives grouped ops and aligned rows from the stored diff rows.", ) .text_sm() .text_color(theme.colors.text_muted), @@ -188,7 +202,7 @@ impl gpui::Render for Screen { .flex() .flex_col() .gap_2() - .child(text("Precomputed DiffOps").text_sm()) + .child(text("Derived Op Groups").text_sm()) .children(op_cards), ), ), @@ -218,9 +232,9 @@ impl gpui::Render for Screen { .child( text(format!( "{} ops, {} old lines, {} new lines", - case.diff.spans().len(), - case.diff.old_line_count(), - case.diff.new_line_count(), + case.op_groups.len(), + line_count(&case.old_lines), + line_count(&case.new_lines), )) .text_xs() .font_family("Menlo") @@ -238,11 +252,11 @@ impl gpui::Render for Screen { .border_b_1() .border_color(theme.colors.border_muted) .child( - panel_header("Old", case.diff.old_line_count(), theme) + panel_header("Old", line_count(&case.old_lines), theme) .flex_1(), ) .child( - panel_header("New", case.diff.new_line_count(), theme) + panel_header("New", line_count(&case.new_lines), theme) .flex_1(), ), ) @@ -259,8 +273,12 @@ impl gpui::Render for Screen { .flex_col() .gap_3() .child(text("Source Content").text_sm()) - .child(render_source_content(&case.diff, theme)) - .child(text("DiffOps Render").text_sm()) + .child(render_source_content( + &case.old_lines, + &case.new_lines, + theme, + )) + .child(text("Diff Rows Render").text_sm()) .children(op_groups), ), ), @@ -273,7 +291,7 @@ fn sample_cases() -> Vec { vec![ DiffCase::new( "Insert Block", - "A pure insert leaves the old side with an empty anchor span such as 2..2 while the new side grows.", + "A pure insert leaves the old side with an empty anchor range such as 2..2 while the new side grows.", r#"fn config() { let host = "localhost"; start(host); @@ -288,7 +306,7 @@ fn sample_cases() -> Vec { ), DiffCase::new( "Delete Block", - "A delete keeps the old side non-empty and gives the new side an empty anchor span at the removal point.", + "A delete keeps the old side non-empty and gives the new side an empty anchor range at the removal point.", r#"fn handle(req: Request) { trace_request(&req); authorize(&req); @@ -303,7 +321,7 @@ fn sample_cases() -> Vec { ), DiffCase::new( "Replace Span", - "A replace can cover different line counts on each side. The viewer pairs rows by position inside the op span.", + "A replace can cover different line counts on each side. The viewer pairs rows by position inside the derived op group.", r#"fn render() { let theme = current_theme(cx); layout(theme); @@ -318,7 +336,7 @@ fn sample_cases() -> Vec { ), DiffCase::new( "Mixed Hunk", - "This sample produces several DiffOps in sequence so you can see equal, replace, insert, and delete spans together.", + "This sample produces several op groups in sequence so you can see equal, replace, insert, and delete rows together.", r#"use crate::auth::Token; use crate::http::Client; @@ -350,13 +368,18 @@ impl DiffCase { old: &'static str, new: &'static str, ) -> Self { + let diff = diff_content( + Bytes::from_static(old.as_bytes()), + Bytes::from_static(new.as_bytes()), + ) + .expect("sample content should always be valid utf-8"); + Self { title, description, - diff: diff_content( - Bytes::from_static(old.as_bytes()), - Bytes::from_static(new.as_bytes()), - ), + old_lines: collect_source_lines(&diff, SourceSide::Old), + new_lines: collect_source_lines(&diff, SourceSide::New), + op_groups: collect_op_groups(&diff), } } } @@ -382,27 +405,28 @@ fn panel_header(label: &'static str, line_count: usize, theme: &crate::theme::Th ) } -fn render_source_content(diff: &ContentDiff, theme: &crate::theme::Theme) -> gpui::Div { +fn render_source_content( + old_lines: &[SourceLine], + new_lines: &[SourceLine], + theme: &crate::theme::Theme, +) -> gpui::Div { div() .flex() .flex_row() .gap_2() - .child(render_source_panel("Old Content", DiffSide::Old, diff, theme).flex_1()) - .child(render_source_panel("New Content", DiffSide::New, diff, theme).flex_1()) + .child(render_source_panel("Old Content", old_lines, theme).flex_1()) + .child(render_source_panel("New Content", new_lines, theme).flex_1()) } fn render_source_panel( title: &'static str, - side: DiffSide, - diff: &ContentDiff, + lines: &[SourceLine], theme: &crate::theme::Theme, ) -> gpui::Div { - let line_count = match side { - | DiffSide::Old => diff.old_line_count(), - | DiffSide::New => diff.new_line_count(), - }; + let line_count = line_count(lines); - let lines: Vec = (0..line_count) + let rows: Vec = lines + .iter() .map(|line| { div() .flex() @@ -418,7 +442,7 @@ fn render_source_panel( .font_family("Menlo") .text_xs() .text_color(theme.colors.text_subtle) - .child(format!("{:>4}", line + 1)), + .child(format!("{:>4}", line.line_number + 1)), ) .child( div() @@ -429,7 +453,7 @@ fn render_source_panel( .font_family("Menlo") .text_xs() .text_color(theme.colors.text) - .child(display_bytes(diff.line_slice_at(side, line))), + .child(display_text(&line.content)), ) .into_any_element() }) @@ -460,11 +484,11 @@ fn render_source_panel( .text_color(theme.colors.text_subtle), ), ) - .child(div().flex().flex_col().children(lines)) + .child(div().flex().flex_col().children(rows)) } -fn render_op_card(index: usize, span: &Span, theme: &crate::theme::Theme) -> gpui::Div { - let colors = tag_colors(span.op, theme); +fn render_op_card(index: usize, group: &OpGroup, theme: &crate::theme::Theme) -> gpui::Div { + let colors = tag_colors(group.op, theme); div() .rounded_md() @@ -476,38 +500,33 @@ fn render_op_card(index: usize, span: &Span, theme: &crate::theme::Theme) -> gpu .flex_col() .gap_1() .child( - text(format!("Op {index}: {}", tag_label(span.op))) + text(format!("Op {index}: {}", tag_label(group.op))) .text_sm() .text_color(colors.foreground), ) .child( text(format!( "old {:?} new {:?}", - span.old_range, span.new_range + group.old_range, group.new_range )) .text_xs() .font_family("Menlo") .text_color(theme.colors.text_muted), ) .child( - text(format!("{:?}", span.op)) + text(format!("{} aligned rows", group.rows.len())) .text_xs() .font_family("Menlo") .text_color(theme.colors.text_subtle), ) } -fn render_op_group( - index: usize, - span: &Span, - rows: Vec, - diff: &ContentDiff, - theme: &crate::theme::Theme, -) -> gpui::Div { - let colors = tag_colors(span.op, theme); - let row_elements: Vec = rows - .into_iter() - .map(|row| render_row(row, diff, theme).into_any_element()) +fn render_op_group(index: usize, group: &OpGroup, theme: &crate::theme::Theme) -> gpui::Div { + let colors = tag_colors(group.op, theme); + let row_elements: Vec = group + .rows + .iter() + .map(|row| render_row(index, row, theme).into_any_element()) .collect(); div() @@ -527,14 +546,14 @@ fn render_op_group( .justify_between() .items_center() .child( - text(format!("Op {index}: {}", tag_label(span.op))) + text(format!("Op {index}: {}", tag_label(group.op))) .text_sm() .text_color(colors.foreground), ) .child( text(format!( "old {:?} new {:?}", - span.old_range, span.new_range + group.old_range, group.new_range )) .text_xs() .font_family("Menlo") @@ -544,16 +563,7 @@ fn render_op_group( .child(div().flex().flex_col().children(row_elements)) } -fn render_row(row: DiffRow, diff: &ContentDiff, theme: &crate::theme::Theme) -> gpui::Div { - let old_text = row - .old_content_range - .as_ref() - .map(|range| display_bytes(diff.line_slice(DiffSide::Old, range))); - let new_text = row - .new_content_range - .as_ref() - .map(|range| display_bytes(diff.line_slice(DiffSide::New, range))); - +fn render_row(op_index: usize, row: &DiffLine, theme: &crate::theme::Theme) -> gpui::Div { div() .flex() .flex_row() @@ -561,18 +571,18 @@ fn render_row(row: DiffRow, diff: &ContentDiff, theme: &crate::theme::Theme) -> .border_b_1() .border_color(theme.colors.border_muted) .child(render_line_cell( - row.op_index, + op_index, row.op, - row.old_line, - old_text, + row.old_content.as_ref().map(|_| row.old_line), + row.old_content.as_deref().map(display_text), true, theme, )) .child(render_line_cell( - row.op_index, + op_index, row.op, - row.new_line, - new_text, + row.new_content.as_ref().map(|_| row.new_line), + row.new_content.as_deref().map(display_text), false, theme, )) @@ -619,7 +629,7 @@ fn render_line_cell( .font_family("Menlo") .text_xs() .text_color(colors.foreground) - .child(content.unwrap_or_else(|| format!("anchor for span {op_index}"))), + .child(content.unwrap_or_else(|| format!("anchor for op {op_index}"))), ) } @@ -632,10 +642,10 @@ fn tag_label(op: Op) -> &'static str { } } -fn display_bytes(bytes: &[u8]) -> String { +fn display_text(text: &str) -> String { let mut rendered = String::new(); - for ch in String::from_utf8_lossy(bytes).chars() { + for ch in text.chars() { match ch { | '\n' => rendered.push_str("\\n"), | '\r' => rendered.push_str("\\r"), @@ -651,6 +661,91 @@ fn display_bytes(bytes: &[u8]) -> String { rendered } +fn line_count(lines: &[SourceLine]) -> usize { + lines.last().map(|line| line.line_number + 1).unwrap_or(0) +} + +fn collect_source_lines(diff: &ContentDiff, side: SourceSide) -> Vec { + let mut lines = Vec::new(); + + for i in 0..diff.len() { + let row = diff.get(i); + + match side { + | SourceSide::Old => { + if let Some(content) = &row.old_content { + lines.push(SourceLine { + line_number: row.old_line, + content: Arc::clone(content), + }); + } + } + | SourceSide::New => { + if let Some(content) = &row.new_content { + lines.push(SourceLine { + line_number: row.new_line, + content: Arc::clone(content), + }); + } + } + } + } + + lines +} + +fn collect_op_groups(diff: &ContentDiff) -> Vec { + let mut groups = Vec::new(); + let mut start = 0; + + while start < diff.len() { + let op = diff.get(start).op; + let mut end = start + 1; + + while end < diff.len() && diff.get(end).op == op { + end += 1; + } + + let rows: Vec = (start..end).map(|i| diff.get(i).clone()).collect(); + + groups.push(OpGroup { + op, + old_range: group_range(&rows, SourceSide::Old), + new_range: group_range(&rows, SourceSide::New), + rows, + }); + + start = end; + } + + groups +} + +fn group_range(rows: &[DiffLine], side: SourceSide) -> Range { + let anchor = match side { + | SourceSide::Old => rows.first().map(|row| row.old_line).unwrap_or(0), + | SourceSide::New => rows.first().map(|row| row.new_line).unwrap_or(0), + }; + + let mut first = None; + let mut last = None; + + for line_number in rows.iter().filter_map(|row| match side { + | SourceSide::Old => row.old_content.as_ref().map(|_| row.old_line), + | SourceSide::New => row.new_content.as_ref().map(|_| row.new_line), + }) { + if first.is_none() { + first = Some(line_number); + } + last = Some(line_number); + } + + match (first, last) { + | (Some(start), Some(end)) => start..end + 1, + | _ => anchor..anchor, + } +} + struct Colors { background: gpui::Rgba, border: gpui::Rgba, diff --git a/src/util/diff.rs b/src/util/diff.rs index 4de0a5e..926414a 100644 --- a/src/util/diff.rs +++ b/src/util/diff.rs @@ -1,12 +1,7 @@ -use std::ops::Range; +use std::{ops::Range, slice::Iter, sync::Arc, thread::current}; -pub(crate) struct ContentDiff { - pub(crate) old_content: bytes::Bytes, - pub(crate) new_content: bytes::Bytes, - pub(crate) spans: Vec, - old_line_ranges: Vec>, - new_line_ranges: Vec>, -} +use memchr::{memchr2, memchr2_iter, memchr3_iter}; +use similar::DiffableStr; pub(crate) struct Span { pub(crate) op: Op, @@ -14,7 +9,7 @@ pub(crate) struct Span { pub(crate) new_range: Range, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Op { Equal, Delete, @@ -37,147 +32,184 @@ pub(crate) struct DiffRow { pub(crate) new_content_range: Option>, } -pub(crate) fn diff_content(old_content: bytes::Bytes, new_content: bytes::Bytes) -> ContentDiff { +#[derive(Clone)] +pub(crate) struct DiffLine { + pub(crate) op: Op, + pub(crate) old_content: Option>, + pub(crate) old_line: usize, + pub(crate) new_content: Option>, + pub(crate) new_line: usize, +} + +#[derive(Clone)] +pub(crate) struct ContentDiff(Vec); + +pub(crate) fn diff_content( + old_content: bytes::Bytes, + new_content: bytes::Bytes, +) -> Option { let old_line_ranges = line_ranges(&old_content); let new_line_ranges = line_ranges(&new_content); let diff = similar::TextDiff::from_lines::<[u8]>(&old_content, &new_content); - let spans = diff - .ops() - .iter() - .map(|op| match op { - | &similar::DiffOp::Equal { - old_index, - new_index, - len, - } => Span { - op: Op::Equal, - old_range: old_index..(old_index + len), - new_range: new_index..(new_index + len), - }, + let mut diff_lines: Vec = Vec::new(); - | &similar::DiffOp::Delete { - old_index, - old_len, - new_index, - } => Span { - op: Op::Delete, - old_range: old_index..(old_index + old_len), - new_range: new_index..new_index, - }, + for op in diff.ops() { + match op { + | &similar::DiffOp::Equal { + old_index, + new_index, + len, + } => { + for i in 0..len { + let old_line = old_index + i; + let new_line = new_index + i; + let old_line_range = &old_line_ranges[old_line]; + let content = Arc::from(old_content.slice(old_line_range.clone()).as_str()?); + diff_lines.push(DiffLine { + op: Op::Equal, + old_line, + old_content: Some(Arc::clone(&content)), + new_line, + new_content: Some(content), + }); + } + } - | &similar::DiffOp::Insert { - old_index, - new_index, - new_len, - } => Span { - op: Op::Insert, - old_range: old_index..old_index, - new_range: new_index..(new_index + new_len), - }, + | &similar::DiffOp::Insert { + old_index, + new_index, + new_len, + } => { + for i in 0..new_len { + let new_line_range = &new_line_ranges[new_index + i]; + let content = Arc::from(new_content.slice(new_line_range.clone()).as_str()?); + diff_lines.push(DiffLine { + op: Op::Insert, + old_line: old_index, + old_content: None, + new_line: new_index + i, + new_content: Some(content), + }) + } + } - | &similar::DiffOp::Replace { - old_index, - old_len, - new_index, - new_len, - } => Span { - op: Op::Replace, - old_range: old_index..(old_index + old_len), - new_range: new_index..(new_index + new_len), - }, - }) - .collect(); + | &similar::DiffOp::Replace { + old_index, + old_len, + new_index, + new_len, + } => { + for i in 0..new_len.max(old_len) { + let old_line = old_index + i; + let new_line = new_index + i; - ContentDiff { - old_content, - new_content, - spans, - old_line_ranges, - new_line_ranges, + let diff_line = match (old_line_ranges.get(old_line), new_line_ranges.get(new_line)) + { + | (Some(old_range), Some(new_range)) => DiffLine { + op: Op::Replace, + old_line, + old_content: Some(Arc::from(old_content.slice(old_range.clone()).as_str()?)), + new_line: new_index + i, + new_content: Some(Arc::from(new_content.slice(new_range.clone()).as_str()?)), + }, + + | (None, Some(new_range)) => DiffLine { + op: Op::Replace, + old_line: old_index + old_len, + old_content: None, + new_line: new_index + i, + new_content: Some(Arc::from(new_content.slice(new_range.clone()).as_str()?)), + }, + + | (Some(old_range), None) => DiffLine { + op: Op::Replace, + old_line: old_index + i, + old_content: Some(Arc::from(old_content.slice(old_range.clone()).as_str()?)), + new_line: new_index + new_len, + new_content: None, + }, + + | (None, None) => { + // unlickly to happen, but if it does, idk + panic!( + "the unlikely happened: both old & new index of DiffOps::Replace don't point to any line in the parsed line ranges." + ) + } + }; + + diff_lines.push(diff_line); + } + } + + | &similar::DiffOp::Delete { + old_index, + old_len, + new_index, + } => { + for i in 0..old_len { + let old_line_range = &old_line_ranges[old_index]; + let content = Arc::from(old_content.slice(old_line_range.clone()).as_str()?); + diff_lines.push(DiffLine { + op: Op::Delete, + old_line: old_index + i, + old_content: Some(content), + new_line: new_index, + new_content: None, + }) + } + } + } } + + Some(ContentDiff(diff_lines)) } impl ContentDiff { - pub(crate) fn spans(&self) -> &[Span] { - &self.spans + pub(crate) fn len(&self) -> usize { + self.0.len() } - pub(crate) fn old_line_count(&self) -> usize { - self.old_line_ranges.len() + pub(crate) fn get(&self, i: usize) -> &DiffLine { + &self.0[i] } - pub(crate) fn new_line_count(&self) -> usize { - self.new_line_ranges.len() - } - - pub(crate) fn line_slice(&self, side: DiffSide, range: &Range) -> &[u8] { - match side { - | DiffSide::Old => &self.old_content[range.clone()], - | DiffSide::New => &self.new_content[range.clone()], - } - } - - pub(crate) fn line_slice_at(&self, side: DiffSide, line: usize) -> &[u8] { - match side { - | DiffSide::Old => self.line_slice(DiffSide::Old, &self.old_line_ranges[line]), - | DiffSide::New => self.line_slice(DiffSide::New, &self.new_line_ranges[line]), - } - } - - pub(crate) fn rows_for_span(&self, span_index: usize) -> Vec { - let span = &self.spans[span_index]; - let old_len = span.old_range.end.saturating_sub(span.old_range.start); - let new_len = span.new_range.end.saturating_sub(span.new_range.start); - let row_count = old_len.max(new_len); - - let mut rows = Vec::with_capacity(row_count); - for offset in 0..row_count { - let old_line = (offset < old_len).then_some(span.old_range.start + offset); - let new_line = (offset < new_len).then_some(span.new_range.start + offset); - - rows.push(DiffRow { - op_index: span_index, - op: span.op, - old_line, - old_content_range: old_line.map(|line| self.old_line_ranges[line].clone()), - new_line, - new_content_range: new_line.map(|line| self.new_line_ranges[line].clone()), - }); - } - - rows + pub(crate) fn last(&self) -> Option<&DiffLine> { + self.0.last() } } fn line_ranges(content: &[u8]) -> Vec> { - let mut ranges = Vec::new(); - let mut start = 0; - let mut index = 0; + let mut ranges: Vec> = Vec::new(); + let mut line_start: usize = 0; + let mut skip_next = false; - while index < content.len() { - match content[index] { - | b'\r' => { - index += 1; - if index < content.len() && content[index] == b'\n' { - index += 1; - } - ranges.push(start..index); - start = index; - } - | b'\n' => { - index += 1; - ranges.push(start..index); - start = index; - } - | _ => { - index += 1; - } + for i in memchr2_iter(b'\n', b'\r', content) { + if skip_next { + skip_next = false; + continue; + } + + let c = content[i]; + + match (c, content.get(i + 1)) { + | (b'\r', Some(b'\n')) => { + // if \r found, check if its \r\n or if its a lone \r + // if \r\n, then treat as one line break + ranges.push(line_start..i + 1); + // because we already counted the \n byte, the next iter into it needs to be skipped + skip_next = true; + line_start = i + 2; + } + | _ => { + ranges.push(line_start..i); + line_start = i + 1; + } } } - if start < content.len() { - ranges.push(start..content.len()); + if line_start < content.len() { + ranges.push(line_start..content.len()); } ranges diff --git a/src/util/file.rs b/src/util/file.rs index c028939..4e469ab 100644 --- a/src/util/file.rs +++ b/src/util/file.rs @@ -5,18 +5,6 @@ pub(crate) enum ContentType { Binary, } -pub(crate) struct ContentDiff { - old_content: bytes::Bytes, - new_content: bytes::Bytes, -} - -pub(crate) struct LineDiff { - old_line: Option, - old_content_range: std::ops::Range, - new_line: Option, - new_content_range: std::ops::Range, -} - pub(crate) fn classify_content(content: &[u8]) -> ContentType { if content.is_empty() { ContentType::Text @@ -28,9 +16,9 @@ pub(crate) fn classify_content(content: &[u8]) -> ContentType { { ContentType::Text } else { - match memchr(0, &content[0..8192]) { - | None => ContentType::Text, - | Some(_) => ContentType::Binary, + match memchr(0, &content[..content.len().min(8192)]) { + | None => ContentType::Text, + | Some(_) => ContentType::Binary, } } }