Radish alpha
r
radicle-tui
rad:z39mP9rQAaGmERfUMPULfPUi473tY
Radicle terminal user interface
Radicle
Git
Patches › 757743b8
Review: Fix hunk comments
Merged did:key:z6MkgFq6...nBGz opened 1 year ago

This patch fixes broken comment rendering on hunks.

Activity Changes +818 -79
5 files changed +818 -79 c5930ad4 → 9aa7fb20
modified bin/commands/patch/review.rs
@@ -343,11 +343,6 @@ impl<'a> App<'a> {
        let hunk = selected.and_then(|selected| hunks.get(selected));



        if let Some(hunk) = hunk {
- 
            let empty_text = hunk
- 
                .inner()
- 
                .hunk_text()
- 
                .unwrap_or(Text::raw("Nothing to show.").dark_gray());
- 


            let mut cursor = selected

                .and_then(|selected| state.view_state(selected))

                .map(|state| state.cursor)
@@ -364,6 +359,10 @@ impl<'a> App<'a> {
                        })

                    }

                } else {
+ 
                    let empty_text = hunk
+ 
                        .inner()
+ 
                        .hunk_text()
+ 
                        .unwrap_or(Text::raw("Nothing to show.").dark_gray());

                    ui.centered_text_view(frame, empty_text, Some(Borders::BottomSides));

                }

            });
modified bin/git.rs
@@ -283,6 +283,18 @@ impl HunkDiff {
        }

    }
+ 
    pub fn path(&self) -> &PathBuf {
+ 
        match self {
+ 
            HunkDiff::Added { path, .. } => path,
+ 
            HunkDiff::Modified { path, .. } => path,
+ 
            HunkDiff::Deleted { path, .. } => path,
+ 
            HunkDiff::Copied { copied } => &copied.new_path,
+ 
            HunkDiff::Moved { moved } => &moved.new_path,
+ 
            HunkDiff::ModeChanged { path, .. } => path,
+ 
            HunkDiff::EofChanged { path, .. } => path,
+ 
        }
+ 
    }
+ 


    pub fn paths(&self) -> FilePaths {

        match self {

            Self::Added { path, new, .. } => (None, Some((path, new.oid))),
modified bin/test.rs
@@ -118,7 +118,7 @@ pub mod setup {
}



pub mod fixtures {
- 
    use std::path::Path;
+ 
    use std::path::{Path, PathBuf};



    use anyhow::Result;
@@ -133,6 +133,11 @@ pub mod fixtures {
    use radicle::storage::ReadStorage;

    use radicle::test::setup::{BranchWith, Node};

    use radicle::Storage;
+ 
    use radicle_cli::git::unified_diff::FileHeader;
+ 
    use radicle_git_ext::Oid;
+ 
    use radicle_surf::diff::{self, DiffFile, Hunk, Line, Modification};
+
+ 
    use crate::git::HunkDiff;



    use super::setup::{NodeRepo, NodeRepoCheckout, NodeWithRepo};
@@ -280,4 +285,186 @@ fn main() {


        (repo, oid)

    }
+
+ 
    /// @@ -3,8 +3,7 @@
+ 
    /// 3   3     // or if you prefer to use your keyboard, you can use the "Ctrl + Enter"
+ 
    /// 4   4     // shortcut.
+ 
    /// 5   5
+ 
    /// 6       - // This code is editable, feel free to hack it!
+ 
    /// 7       - // You can always return to the original code by clicking the "Reset" button ->
+ 
    ///     6   + // This is still a comment.
+ 
    /// 8   7
+ 
    /// 9   8     // This is the main function.
+ 
    /// 10  9     fn main() {
+ 
    pub fn simple_modified_hunk_diff(path: &PathBuf, commit: Oid) -> Result<HunkDiff> {
+ 
        let diff = DiffFile {
+ 
            oid: commit,
+ 
            mode: diff::FileMode::Blob,
+ 
        };
+
+ 
        Ok(HunkDiff::Modified {
+ 
            path: path.clone(),
+ 
            header: FileHeader::Modified {
+ 
                path: path.to_path_buf(),
+ 
                old: diff.clone(),
+ 
                new: diff.clone(),
+ 
                binary: false,
+ 
            },
+ 
            old: diff.clone(),
+ 
            new: diff,
+ 
            hunk: Some(Hunk {
+ 
                header: Line::from(b"@@ -3,8 +3,7 @@\n".to_vec()),
+ 
                lines: vec![
+ 
                    Modification::context(
+ 
                        b"// or if you prefer to use your keyboard, you can use the \"Ctrl + Enter\"\n"
+ 
                            .to_vec(),
+ 
                        3,
+ 
                        3,
+ 
                    ),
+ 
                    Modification::context(b"// shortcut.\n".to_vec(), 4, 4),
+ 
                    Modification::context(b"\n".to_vec(), 5, 5),
+ 
                    Modification::deletion(
+ 
                        b"// This code is editable, feel free to hack it!\n".to_vec(),
+ 
                        6,
+ 
                    ),
+ 
                    Modification::deletion(
+ 
                        b"// You can always return to the original code by clicking the \"Reset\" button ->\n".to_vec(),
+ 
                        7,
+ 
                    ),
+ 
                    Modification::addition(b"// This is still a comment.\n".to_vec(), 6),
+ 
                    Modification::context(b"\n".to_vec(), 8, 7),
+ 
                    Modification::context(b"// This is the main function.\n".to_vec(), 9, 8),
+ 
                    Modification::context(b"fn main() {\n".to_vec(), 10, 9),
+ 
                ],
+ 
                old: 3..11,
+ 
                new: 3..10,
+ 
            }),
+ 
            _stats: None,
+ 
        })
+ 
    }
+
+ 
    /// @@ -1,17 +1,15 @@
+ 
    /// 1       - use radicle::issue::IssueId;
+ 
    /// 2       - use tui::ui::state::ItemState;
+ 
    /// 3       - use tui::SelectionExit;
+ 
    /// 4   1     use tuirealm::command::{Cmd, CmdResult, Direction as MoveDirection};
+ 
    /// 5   2     use tuirealm::event::{Event, Key, KeyEvent};
+ 
    /// 6   3     use tuirealm::{MockComponent, NoUserEvent};
+ 
    /// 7   4
+ 
    /// 8   5     use radicle_tui as tui;
+ 
    /// 9   6
+ 
    ///     7   + use tui::ui::state::ItemState;
+ 
    /// 10  8     use tui::ui::widget::container::{AppHeader, GlobalListener, LabeledContainer};
+ 
    /// 11  9     use tui::ui::widget::context::{ContextBar, Shortcuts};
+ 
    /// 12  10    use tui::ui::widget::list::PropertyList;
+ 
    /// 13      -
+ 
    /// 14  11    use tui::ui::widget::Widget;
+ 
    ///     12  + use tui::{Id, SelectionExit};
+ 
    /// 15  13
+ 
    /// 16  14    use super::ui::{IdSelect, OperationSelect};
+ 
    /// 17  15    use super::{IssueOperation, Message};
+ 
    pub fn complex_modified_hunk_diff(path: &PathBuf, commit: Oid) -> Result<HunkDiff> {
+ 
        let diff = DiffFile {
+ 
            oid: commit,
+ 
            mode: diff::FileMode::Blob,
+ 
        };
+
+ 
        Ok(HunkDiff::Modified {
+ 
            path: path.clone(),
+ 
            header: FileHeader::Modified {
+ 
                path: path.to_path_buf(),
+ 
                old: diff.clone(),
+ 
                new: diff.clone(),
+ 
                binary: false,
+ 
            },
+ 
            old: diff.clone(),
+ 
            new: diff,
+ 
            hunk: Some(Hunk {
+ 
                header: Line::from(b"@@ -1,17 +1,15 @@\n".to_vec()),
+ 
                lines: vec![
+ 
                    Modification::deletion(b"use radicle::issue::IssueId;\n".to_vec(), 1),
+ 
                    Modification::deletion(b"use tui::ui::state::ItemState;\n".to_vec(), 2),
+ 
                    Modification::deletion(b"use tui::SelectionExit;\n".to_vec(), 3),
+ 
                    Modification::context(
+ 
                        b"use tuirealm::command::{Cmd, CmdResult, Direction as MoveDirection};\n"
+ 
                            .to_vec(),
+ 
                        4,
+ 
                        1,
+ 
                    ),
+ 
                    Modification::context(
+ 
                        b"use tuirealm::event::{Event, Key, KeyEvent};\n".to_vec(),
+ 
                        5,
+ 
                        2,
+ 
                    ),
+ 
                    Modification::context(
+ 
                        b"use tuirealm::{MockComponent, NoUserEvent};\n".to_vec(),
+ 
                        6,
+ 
                        3,
+ 
                    ),
+ 
                    Modification::context(b"\n".to_vec(), 7, 4),
+ 
                    Modification::context(b"use radicle_tui as tui;\n".to_vec(), 8, 5),
+ 
                    Modification::context(b"\n".to_vec(), 9, 6),
+ 
                    Modification::addition(b"use tui::ui::state::ItemState;\n".to_vec(), 7),
+ 
                    Modification::context(b"use tui::ui::widget::container::{AppHeader, GlobalListener, LabeledContainer};\n"
+ 
                        .to_vec(),
+ 
                        10,
+ 
                        8,
+ 
                    ),
+ 
                    Modification::context(
+ 
                        b"use tui::ui::widget::context::{ContextBar, Shortcuts};\n".to_vec(),
+ 
                        11,
+ 
                        9,
+ 
                    ),
+ 
                    Modification::context(
+ 
                        b"use tui::ui::widget::list::PropertyList;\n".to_vec(),
+ 
                        12,
+ 
                        10,
+ 
                    ),
+ 
                    Modification::deletion(b"\n".to_vec(), 13),
+ 
                    Modification::context(b"use tui::ui::widget::Widget;\n".to_vec(), 14, 11),
+ 
                    Modification::addition(b"use tui::{Id, SelectionExit};\n".to_vec(), 12),
+ 
                    Modification::context(b"\n".to_vec(), 15, 13),
+ 
                    Modification::context(
+ 
                        b"use super::ui::{IdSelect, OperationSelect};\n".to_vec(),
+ 
                        16,
+ 
                        14,
+ 
                    ),
+ 
                    Modification::context(
+ 
                        b"use super::{IssueOperation, Message};\n".to_vec(),
+ 
                        17,
+ 
                        15,
+ 
                    ),
+ 
                ],
+ 
                old: 1..18,
+ 
                new: 1..16,
+ 
            }),
+ 
            _stats: None,
+ 
        })
+ 
    }
+
+ 
    /// @@ -1,1 +0,0 @@
+ 
    /// - TBD
+ 
    pub fn deleted_hunk_diff(path: &PathBuf, commit: Oid) -> Result<HunkDiff> {
+ 
        let diff = DiffFile {
+ 
            oid: commit,
+ 
            mode: diff::FileMode::Blob,
+ 
        };
+
+ 
        Ok(HunkDiff::Deleted {
+ 
            path: path.clone(),
+ 
            header: FileHeader::Deleted {
+ 
                path: path.to_path_buf(),
+ 
                old: diff.clone(),
+ 
                binary: false,
+ 
            },
+ 
            old: diff.clone(),
+ 
            hunk: Some(Hunk {
+ 
                header: Line::from(b"@@ -1,1 +0,0 @@\n".to_vec()),
+ 
                lines: vec![Modification::deletion(b"TBD\n".to_vec(), 1)],
+ 
                old: 1..2,
+ 
                new: 0..0,
+ 
            }),
+ 
            _stats: None,
+ 
        })
+ 
    }

}
modified bin/ui/items.rs
@@ -1,6 +1,7 @@
use std::collections::HashMap;

use std::fmt;

use std::fmt::Debug;
+ 
use std::ops::Range;

use std::str::FromStr;



use nom::bytes::complete::{tag, take};
@@ -18,13 +19,14 @@ use radicle::issue;
use radicle::issue::{CloseReason, Issue, IssueId, Issues};

use radicle::node::notifications::{Notification, NotificationId, NotificationKind};

use radicle::node::{Alias, AliasStore, NodeId};
- 
use radicle::patch::{self, Review};
- 
use radicle::patch::{Patch, PatchId, Patches};
+ 
use radicle::patch;
+ 
use radicle::patch::{Patch, PatchId, Patches, Review};

use radicle::storage::git::Repository;

use radicle::storage::{ReadRepository, ReadStorage, RefUpdate, WriteRepository};

use radicle::Profile;
- 
use radicle_surf::diff::{self, Hunk, Modification};
+ 
use radicle_surf::diff;
+ 
use radicle_surf::diff::{Hunk, Modification};



use radicle_cli::git::unified_diff::{Decode, HunkHeader};

use radicle_cli::terminal;
@@ -39,7 +41,7 @@ use tui_tree_widget::TreeItem;
use radicle_tui as tui;



use tui::ui::theme::style;
- 
use tui::ui::utils::LineMerger;
+ 
use tui::ui::utils::{LineMerger, MergeLocation};

use tui::ui::{span, Column};

use tui::ui::{ToRow, ToTree};
@@ -1041,40 +1043,171 @@ impl<'a> From<TermLine> for Line<'a> {
    }

}
- 
/// All comments per hunk, indexed by their starting line.
- 
#[derive(Clone, Debug)]
- 
pub struct HunkComments {
- 
    /// All comments. Can be unsorted.
- 
    comments: HashMap<usize, Vec<(EntryId, Comment<CodeLocation>)>>,
+ 
/// Represents the old and new ranges of a unified diff.
+ 
pub struct DiffLineRanges {
+ 
    old: Range<u32>,
+ 
    new: Range<u32>,

}
- 
impl HunkComments {
- 
    pub fn all(&self) -> &HashMap<usize, Vec<(EntryId, Comment<CodeLocation>)>> {
- 
        &self.comments
+ 
impl From<&Hunk<Modification>> for DiffLineRanges {
+ 
    fn from(hunk: &Hunk<Modification>) -> Self {
+ 
        Self {
+ 
            old: hunk.old.clone(),
+ 
            new: hunk.new.clone(),
+ 
        }

    }
+ 
}
- 
    pub fn is_empty(&self) -> bool {
- 
        self.comments.is_empty()
+ 
/// Identifies a line in a unified diff by its old and new line number.
+ 
#[derive(Clone, Debug, Default, Hash, Eq, PartialEq)]
+ 
pub struct DiffLineIndex {
+ 
    old: Option<u32>,
+ 
    new: Option<u32>,
+ 
}
+
+ 
impl DiffLineIndex {
+ 
    pub fn is_start_of(&self, ranges: &DiffLineRanges) -> bool {
+ 
        // TODO(erikli): Find out, why comments inserted right before or after
+ 
        // the hunk header can have such weird values.
+ 
        let old = self
+ 
            .old
+ 
            .map(|o| self.new.is_none() && o >= 4294967294)
+ 
            .unwrap_or_default();
+ 
        let new = self
+ 
            .new
+ 
            .map(|n| n == u32::MAX.saturating_sub(1) || n == ranges.new.end)
+ 
            .unwrap_or_default();
+
+ 
        old || new

    }
- 
    pub fn len(&self) -> usize {
- 
        self.comments.values().fold(0_usize, |mut count, comments| {
- 
            count += comments.len();
- 
            count
- 
        })
+ 
    pub fn is_end_of(&self, ranges: &DiffLineRanges) -> bool {
+ 
        let old = self
+ 
            .old
+ 
            .map(|o| o == ranges.old.end.saturating_sub(1))
+ 
            .unwrap_or_default();
+ 
        let new = self
+ 
            .new
+ 
            .map(|n| n == ranges.new.end.saturating_sub(1))
+ 
            .unwrap_or_default();
+
+ 
        old || new
+ 
    }
+
+ 
    pub fn is_inside_of(&self, ranges: &DiffLineRanges) -> bool {
+ 
        let old = self
+ 
            .old
+ 
            .map(|o| o >= ranges.old.start && o < ranges.old.end.saturating_sub(1))
+ 
            .unwrap_or_default();
+ 
        let new = self
+ 
            .new
+ 
            .map(|n| n >= ranges.new.start && n < ranges.new.end.saturating_sub(1))
+ 
            .unwrap_or_default();
+
+ 
        old || new
+ 
    }
+ 
}
+
+ 
impl From<&CodeLocation> for DiffLineIndex {
+ 
    fn from(location: &CodeLocation) -> Self {
+ 
        Self {
+ 
            old: location.old.as_ref().map(|r| match r {
+ 
                CodeRange::Lines { range } => range.end.saturating_sub(1) as u32,
+ 
                CodeRange::Chars { line, range: _ } => line.saturating_sub(1) as u32,
+ 
            }),
+ 
            new: location.new.as_ref().map(|r| match r {
+ 
                CodeRange::Lines { range } => range.end.saturating_sub(1) as u32,
+ 
                CodeRange::Chars { line, range: _ } => line.saturating_sub(1) as u32,
+ 
            }),
+ 
        }

    }

}
- 
impl From<Vec<(EntryId, Comment<CodeLocation>)>> for HunkComments {
- 
    fn from(comments: Vec<(EntryId, Comment<CodeLocation>)>) -> Self {
- 
        let mut line_comments: HashMap<usize, Vec<(EntryId, Comment<CodeLocation>)>> =
+ 
/// A type that can map a line index to a line number in a unified diff.
+ 
#[derive(Debug)]
+ 
pub struct IndexedDiffLines {
+ 
    lines: HashMap<DiffLineIndex, u32>,
+ 
}
+
+ 
impl IndexedDiffLines {
+ 
    pub fn new(diff: &HunkDiff) -> Self {
+ 
        let mut indexed = HashMap::new();
+
+ 
        if let Some(hunk) = diff.hunk() {
+ 
            for (index, line) in hunk.lines.iter().enumerate() {
+ 
                let line_index = match line {
+ 
                    Modification::Addition(addition) => DiffLineIndex {
+ 
                        old: None,
+ 
                        new: Some(addition.line_no),
+ 
                    },
+ 
                    Modification::Deletion(deletion) => DiffLineIndex {
+ 
                        old: Some(deletion.line_no),
+ 
                        new: None,
+ 
                    },
+ 
                    Modification::Context {
+ 
                        line: _,
+ 
                        line_no_old,
+ 
                        line_no_new,
+ 
                    } => DiffLineIndex {
+ 
                        old: Some(*line_no_old),
+ 
                        new: Some(*line_no_new),
+ 
                    },
+ 
                };
+
+ 
                indexed.insert(line_index, index as u32);
+ 
            }
+ 
        }
+
+ 
        Self { lines: indexed }
+ 
    }
+
+ 
    pub fn line(&self, index: DiffLineIndex) -> Option<u32> {
+ 
        self.lines.get(&index).copied()
+ 
    }
+ 
}
+
+ 
/// All comments per hunk, indexed by their merge location: start, line or end.
+ 
#[derive(Clone, Debug)]
+ 
pub struct HunkComments {
+ 
    /// All comments. Can be unsorted.
+ 
    comments: HashMap<MergeLocation, Vec<(EntryId, Comment<CodeLocation>)>>,
+ 
}
+
+ 
impl HunkComments {
+ 
    pub fn new(diff: &HunkDiff, comments: Vec<(EntryId, Comment<CodeLocation>)>) -> Self {
+ 
        let mut line_comments: HashMap<MergeLocation, Vec<(EntryId, Comment<CodeLocation>)>> =

            HashMap::new();
+ 
        let indexed = IndexedDiffLines::new(diff);



        for comment in comments {
- 
            // TODO(erikli): Check why we need range end instead of range start.
- 
            let line = match comment.1.location().as_ref().unwrap().new.as_ref().unwrap() {
- 
                CodeRange::Lines { range } => range.end,
- 
                _ => 0,
+ 
            let line = if let Some(location) = comment.1.location() {
+ 
                if let Some(hunk) = diff.hunk() {
+ 
                    let ranges = DiffLineRanges::from(hunk);
+ 
                    let index = DiffLineIndex::from(location);
+
+ 
                    if index.is_start_of(&ranges) {
+ 
                        MergeLocation::Start
+ 
                    } else if index.is_end_of(&ranges) {
+ 
                        MergeLocation::End
+ 
                    } else {
+ 
                        let mut line = indexed
+ 
                            .line(index.clone())
+ 
                            .map(|line| MergeLocation::Line(line as usize));
+
+ 
                        // TODO(erikli): Properly fix index lookup rules for addition:
+ 
                        // old line number need to be ignored.
+ 
                        if line.is_none() {
+ 
                            line = indexed
+ 
                                .line(DiffLineIndex { old: None, ..index })
+ 
                                .map(|line| MergeLocation::Line(line as usize))
+ 
                        }
+ 
                        line.unwrap_or_default()
+ 
                    }
+ 
                } else {
+ 
                    MergeLocation::Unknown
+ 
                }
+ 
            } else {
+ 
                MergeLocation::Unknown

            };



            if let Some(comments) = line_comments.get_mut(&line) {
@@ -1088,6 +1221,21 @@ impl From<Vec<(EntryId, Comment<CodeLocation>)>> for HunkComments {
            comments: line_comments,

        }

    }
+
+ 
    pub fn all(&self) -> &HashMap<MergeLocation, Vec<(EntryId, Comment<CodeLocation>)>> {
+ 
        &self.comments
+ 
    }
+
+ 
    pub fn is_empty(&self) -> bool {
+ 
        self.comments.is_empty()
+ 
    }
+
+ 
    pub fn len(&self) -> usize {
+ 
        self.comments.values().fold(0_usize, |mut count, comments| {
+ 
            count += comments.len();
+ 
            count
+ 
        })
+ 
    }

}



/// A [`HunkItem`] that can be rendered. Hunk items are indexed sequentially and
@@ -1107,34 +1255,45 @@ impl<'a> From<(&Repository, &Review, &HunkDiff)> for HunkItem<'a> {
    fn from(value: (&Repository, &Review, &HunkDiff)) -> Self {

        let (repo, review, item) = value;

        let hi = Highlighter::default();
- 
        // let hunk = item.hunk();
-
- 
        let path = match &item {
- 
            HunkDiff::Added { path, .. } => path,
- 
            HunkDiff::Modified { path, .. } => path,
- 
            HunkDiff::Deleted { path, .. } => path,
- 
            HunkDiff::Copied { copied } => &copied.new_path,
- 
            HunkDiff::Moved { moved } => &moved.new_path,
- 
            HunkDiff::ModeChanged { path, .. } => path,
- 
            HunkDiff::EofChanged { path, .. } => path,
- 
        };



        // TODO(erikli): Start with raw, non-highlighted lines and

        // move highlighting to separate task / thread, e.g. here:

        // `let lines = blobs.raw()`

        let blobs = item.clone().blobs(repo.raw());

        let lines = blobs.highlight(hi);
+
+ 
        // Filter comments and include them, if:
+ 
        // - comment has a code location
+ 
        // - comment path matches hunk path
+ 
        // - comment code location is inside hunk code range

        let comments = review

            .comments()
- 
            .filter(|(_, comment)| comment.location().is_some())
- 
            .filter(|(_, comment)| comment.location().unwrap().path == *path)
+ 
            .filter(|(_, comment)| {
+ 
                if let Some(location) = comment.location() {
+ 
                    if location.path == *item.path() {
+ 
                        if let Some(hunk) = item.hunk() {
+ 
                            let ranges = DiffLineRanges::from(hunk);
+ 
                            let index = DiffLineIndex::from(location);
+
+ 
                            log::warn!("Checking comment {comment:?} at {index:?}");
+
+ 
                            return index.is_start_of(&ranges)
+ 
                                || index.is_inside_of(&ranges)
+ 
                                || index.is_end_of(&ranges);
+ 
                        } else {
+ 
                            return true;
+ 
                        }
+ 
                    }
+ 
                }
+ 
                false
+ 
            })

            .map(|(id, comment)| (*id, comment.clone()))

            .collect::<Vec<_>>();



        Self {

            diff: item.clone(),

            lines,
- 
            comments: HunkComments::from(comments),
+ 
            comments: HunkComments::new(item, comments),

        }

    }

}
@@ -1512,24 +1671,18 @@ impl<'a> HunkItem<'a> {
                let mut lines = hunk

                    .as_ref()

                    .map(|hunk| Text::from(hunk.to_text(&self.lines)));
- 
                let start = hunk
- 
                    .as_ref()
- 
                    .map(|hunk| hunk.new.start as usize)
- 
                    .unwrap_or_default();



                lines = lines.map(|lines| {
- 
                    let mut mixins = HashMap::new();
- 


                    let divider = span::default(&"─".to_string().repeat(500)).gray().dim();
+ 
                    let mut merge = HashMap::new();

                    for (line, comments) in self.comments.all() {
- 
                        mixins.insert(
- 
                            *line,
+ 
                        merge.insert(
+ 
                            line.clone(),

                            comments

                                .iter()

                                .enumerate()

                                .map(|(idx, comment)| {
- 
                                    // let body = span::default(comment.1.body()).gray();

                                    let timestamp =

                                        span::timestamp(&format::timestamp(&comment.1.timestamp()));

                                    let author =
@@ -1571,8 +1724,7 @@ impl<'a> HunkItem<'a> {
                                .collect(),

                        );

                    }
- 
                    let merged = LineMerger::merge(lines.lines.clone(), mixins, start);
-
+ 
                    let merged = LineMerger::new(lines.lines).merge(merge, None);

                    Text::from(merged)

                });
@@ -1809,7 +1961,12 @@ impl<'a> ToText<'a> for Hunk<Modification> {


#[cfg(test)]

mod tests {
+ 
    use std::path::PathBuf;
+ 


    use anyhow::Result;
+ 
    use radicle::crypto::Signer;
+
+ 
    use crate::test;



    use super::*;
@@ -1876,4 +2033,351 @@ mod tests {


        Ok(())

    }
+
+ 
    #[test]
+ 
    fn diff_line_index_checks_ranges_correctly() -> Result<()> {
+ 
        let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
+ 
        let path = PathBuf::from_str("main.rs").unwrap();
+
+ 
        // --------------------------------------------------------------------
+ 
        // At the top.
+ 
        // --------------------------------------------------------------------
+ 
        // @@ -3,8 +3,7 @@
+ 
        // 3   3     // or if you prefer to use your keyboard, you can use the "Ctrl + Enter"
+ 
        // 4   4     // shortcut.
+ 
        // 5   5
+ 
        // 6       - // This code is editable, feel free to hack it!
+ 
        // 7       - // You can always return to the original code by clicking the "Reset" button ->
+ 
        //     6   + // This is still a comment.
+ 
        // --------------------------------------------------------------------
+ 
        // In the middle.
+ 
        // --------------------------------------------------------------------
+ 
        // 8   7
+ 
        // 9   8     // This is the main function.
+ 
        // 10  9     fn main() {
+ 
        // ---------------------------------------------------------------------
+ 
        // At the end.
+ 
        // ---------------------------------------------------------------------
+ 
        let diff = test::fixtures::simple_modified_hunk_diff(&path, commit)?;
+ 
        let ranges = DiffLineRanges::from(diff.hunk().unwrap());
+
+ 
        let start = CodeLocation {
+ 
            commit,
+ 
            path: path.clone(),
+ 
            old: Some(CodeRange::Lines { range: 3..12 }),
+ 
            new: Some(CodeRange::Lines { range: 3..11 }),
+ 
        };
+ 
        assert!(DiffLineIndex::from(&start).is_start_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&start).is_inside_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&start).is_end_of(&ranges));
+
+ 
        let inside = CodeLocation {
+ 
            commit,
+ 
            path: path.clone(),
+ 
            old: Some(CodeRange::Lines { range: 3..8 }),
+ 
            new: Some(CodeRange::Lines { range: 3..7 }),
+ 
        };
+ 
        assert!(DiffLineIndex::from(&inside).is_inside_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&inside).is_start_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&inside).is_end_of(&ranges));
+
+ 
        let end = CodeLocation {
+ 
            commit,
+ 
            path: path.clone(),
+ 
            old: Some(CodeRange::Lines { range: 3..11 }),
+ 
            new: Some(CodeRange::Lines { range: 3..10 }),
+ 
        };
+ 
        assert!(DiffLineIndex::from(&end).is_end_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&end).is_start_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&end).is_inside_of(&ranges));
+
+ 
        let outside = CodeLocation {
+ 
            commit,
+ 
            path: path.clone(),
+ 
            old: Some(CodeRange::Lines { range: 125..127 }),
+ 
            new: Some(CodeRange::Lines { range: 125..128 }),
+ 
        };
+ 
        assert!(!DiffLineIndex::from(&outside).is_start_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&outside).is_inside_of(&ranges));
+ 
        assert!(!DiffLineIndex::from(&outside).is_end_of(&ranges));
+
+ 
        Ok(())
+ 
    }
+
+ 
    #[test]
+ 
    fn hunk_comments_on_modified_simple_are_inserted_correctly() -> Result<()> {
+ 
        let alice = test::fixtures::node_with_repo();
+
+ 
        let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
+ 
        let path = PathBuf::from_str("main.rs").unwrap();
+
+ 
        // --------------------------------------------------------------------
+ 
        // At the top.
+ 
        // --------------------------------------------------------------------
+ 
        // @@ -3,8 +3,7 @@
+ 
        // 3   3     // or if you prefer to use your keyboard, you can use the "Ctrl + Enter"
+ 
        // 4   4     // shortcut.
+ 
        // 5   5
+ 
        // 6       - // This code is editable, feel free to hack it!
+ 
        // 7       - // You can always return to the original code by clicking the "Reset" button ->
+ 
        //     6   + // This is still a comment.
+ 
        // --------------------------------------------------------------------
+ 
        // In the middle.
+ 
        // --------------------------------------------------------------------
+ 
        // 8   7
+ 
        // 9   8     // This is the main function.
+ 
        // 10  9     fn main() {
+ 
        // ---------------------------------------------------------------------
+ 
        // At the end.
+ 
        // ---------------------------------------------------------------------
+ 
        let diff = test::fixtures::simple_modified_hunk_diff(&path, commit)?;
+
+ 
        let top = (
+ 
            Oid::from_str("05ac6202655dcde6c2613702fec07c2e2fe8f382").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "At the top.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 3..12 }),
+ 
                    new: Some(CodeRange::Lines { range: 3..11 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+ 
        let middle = (
+ 
            Oid::from_str("2d09104bf2d6ad328aa72594b679d2d6c5a61865").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "In the middle.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 3..8 }),
+ 
                    new: Some(CodeRange::Lines { range: 3..7 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+ 
        let end = (
+ 
            Oid::from_str("8280317b308ba1bf2cef04533efb15d920431e86").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "At the end.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 3..11 }),
+ 
                    new: Some(CodeRange::Lines { range: 3..10 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+
+ 
        let comments = {
+ 
            let comments = [top.clone(), middle.clone(), end.clone()];
+ 
            HunkComments::new(&diff, comments.to_vec())
+ 
        };
+
+ 
        for expected in [
+ 
            (top, MergeLocation::Start),
+ 
            (middle, MergeLocation::Line(5)),
+ 
            (end, MergeLocation::End),
+ 
        ] {
+ 
            let (line, expected) = (expected.1, expected.0);
+ 
            let actual = comments.all().get(&line);
+ 
            assert_ne!(actual, None, "No comment found at {line:?}");
+
+ 
            let actual = actual.unwrap().first().unwrap();
+ 
            assert_eq!(actual.0, expected.0);
+ 
        }
+
+ 
        Ok(())
+ 
    }
+
+ 
    #[test]
+ 
    fn hunk_comments_on_modified_complex_are_inserted_correctly() -> Result<()> {
+ 
        let alice = test::fixtures::node_with_repo();
+
+ 
        let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
+ 
        let path = PathBuf::from_str("main.rs").unwrap();
+
+ 
        // --------------------------------------------------------------------
+ 
        // At the top.
+ 
        // --------------------------------------------------------------------
+ 
        // @@ -1,17 +1,15 @@
+ 
        // 1       - use radicle::issue::IssueId;
+ 
        // 2       - use tui::ui::state::ItemState;
+ 
        // 3       - use tui::SelectionExit;
+ 
        // --------------------------------------------------------------------
+ 
        // After deletion.
+ 
        // --------------------------------------------------------------------
+ 
        // 4   1     use tuirealm::command::{Cmd, CmdResult, Direction as MoveDirection};
+ 
        // 5   2     use tuirealm::event::{Event, Key, KeyEvent};
+ 
        // 6   3     use tuirealm::{MockComponent, NoUserEvent};
+ 
        // 7   4
+ 
        // 8   5     use radicle_tui as tui;
+ 
        // 9   6
+ 
        //     7   + use tui::ui::state::ItemState;
+ 
        // 10  8     use tui::ui::widget::container::{AppHeader, GlobalListener, LabeledContainer};
+ 
        // 11  9     use tui::ui::widget::context::{ContextBar, Shortcuts};
+ 
        // 12  10    use tui::ui::widget::list::PropertyList;
+ 
        // 13      -
+ 
        // 14  11    use tui::ui::widget::Widget;
+ 
        //     12  + use tui::{Id, SelectionExit};
+ 
        // 15  13
+ 
        // 16  14    use super::ui::{IdSelect, OperationSelect};
+ 
        // --------------------------------------------------------------------
+ 
        // Before last line.
+ 
        // --------------------------------------------------------------------
+ 
        // 17  15    use super::{IssueOperation, Message};
+ 
        let diff = test::fixtures::complex_modified_hunk_diff(&path, commit)?;
+
+ 
        let top = (
+ 
            Oid::from_str("05ac6202655dcde6c2613702fec07c2e2fe8f382").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "At the top.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 1..18 }),
+ 
                    new: Some(CodeRange::Lines { range: 1..17 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+ 
        let after_deletion = (
+ 
            Oid::from_str("2d09104bf2d6ad328aa72594b679d2d6c5a61865").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "After deletion.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 1..4 }),
+ 
                    new: None,
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+ 
        let before_last_line = (
+ 
            Oid::from_str("60972bca0c9e686e76b0a5123acb3c8c60c38b1e").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "Before last line".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 1..17 }),
+ 
                    new: Some(CodeRange::Lines { range: 1..15 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+
+ 
        let comments = {
+ 
            let comments = [
+ 
                top.clone(),
+ 
                after_deletion.clone(),
+ 
                before_last_line.clone(),
+ 
            ];
+ 
            HunkComments::new(&diff, comments.to_vec())
+ 
        };
+
+ 
        for expected in [
+ 
            (top, MergeLocation::Start),
+ 
            (after_deletion, MergeLocation::Line(2)),
+ 
            (before_last_line, MergeLocation::Line(17)),
+ 
        ] {
+ 
            let (line, expected) = (expected.1, expected.0);
+ 
            let actual = comments.all().get(&line);
+ 
            assert_ne!(actual, None, "No comment found at {line:?}");
+
+ 
            let actual = actual.unwrap().first().unwrap();
+ 
            assert_eq!(actual.0, expected.0);
+ 
        }
+
+ 
        Ok(())
+ 
    }
+
+ 
    #[test]
+ 
    fn hunk_comments_on_deleted_simple_are_inserted_correctly() -> Result<()> {
+ 
        let alice = test::fixtures::node_with_repo();
+
+ 
        let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
+ 
        let path = PathBuf::from_str("README.md").unwrap();
+
+ 
        // --------------------------------------------------------------------
+ 
        // At the top.
+ 
        // --------------------------------------------------------------------
+ 
        // @@ -1,1 +0,0 @@
+ 
        //  -TBD
+ 
        // --------------------------------------------------------------------
+ 
        // At the end.
+ 
        // --------------------------------------------------------------------
+ 
        let diff = test::fixtures::deleted_hunk_diff(&path, commit)?;
+
+ 
        let top = (
+ 
            Oid::from_str("05ac6202655dcde6c2613702fec07c2e2fe8f382").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "At the top.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 1..3 }),
+ 
                    new: Some(CodeRange::Lines { range: 0..1 }),
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+ 
        let end = (
+ 
            Oid::from_str("8280317b308ba1bf2cef04533efb15d920431e86").unwrap(),
+ 
            Comment::new(
+ 
                *alice.node.signer.public_key(),
+ 
                "At the end.".to_string(),
+ 
                None,
+ 
                Some(CodeLocation {
+ 
                    commit,
+ 
                    path: path.clone(),
+ 
                    old: Some(CodeRange::Lines { range: 1..2 }),
+ 
                    new: None,
+ 
                }),
+ 
                vec![],
+ 
                Timestamp::from_secs(0),
+ 
            ),
+ 
        );
+
+ 
        let comments = {
+ 
            let comments = [top.clone(), end.clone()];
+ 
            HunkComments::new(&diff, comments.to_vec())
+ 
        };
+
+ 
        for expected in [(top, MergeLocation::Start), (end, MergeLocation::End)] {
+ 
            let (line, expected) = (expected.1, expected.0);
+ 
            let actual = comments.all().get(&line);
+ 
            assert_ne!(actual, None, "No comment found at {line:?}");
+
+ 
            let actual = actual.unwrap().first().unwrap();
+ 
            assert_eq!(actual.0, expected.0);
+ 
        }
+
+ 
        Ok(())
+ 
    }

}
modified src/ui/utils.rs
@@ -1,25 +1,64 @@
use std::collections::HashMap;
- 
pub struct LineMerger;
+ 
/// A type that specifies the location of a line merge.
+ 
/// The lines to merge will be either added before, within or after
+ 
/// the base lines.
+ 
#[derive(Default, Clone, Debug, Hash, Eq, PartialEq)]
+ 
pub enum MergeLocation {
+ 
    Start,
+ 
    Line(usize),
+ 
    End,
+ 
    #[default]
+ 
    Unknown,
+ 
}
+
+ 
/// A type that can merge lines based on their merge location.
+ 
#[derive(Default)]
+ 
pub struct LineMerger<T> {
+ 
    /// Base lines that other lines will be merged with.
+ 
    lines: Vec<T>,
+ 
}
+
+ 
impl<T: Clone> LineMerger<T> {
+ 
    pub fn new(lines: impl IntoIterator<Item = T>) -> Self {
+ 
        Self {
+ 
            lines: lines.into_iter().collect::<Vec<_>>(),
+ 
        }
+ 
    }
- 
impl LineMerger {
- 
    pub fn merge<T: Clone>(
- 
        lines: Vec<T>,
- 
        mixins: HashMap<usize, Vec<Vec<T>>>,
- 
        start: usize,
+ 
    pub fn merge(
+ 
        &self,
+ 
        merge: HashMap<MergeLocation, Vec<Vec<T>>>,
+ 
        start: Option<usize>,

    ) -> Vec<T> {

        let mut merged = vec![];
- 
        for (idx, line) in lines.iter().enumerate() {
- 
            merged.push(line.clone());
+ 
        for (idx, line) in self.lines.iter().enumerate() {
+ 
            let location = if idx == 0 {
+ 
                MergeLocation::Start
+ 
            } else if idx == self.lines.len().saturating_sub(1) {
+ 
                MergeLocation::End
+ 
            } else {
+ 
                let idx = idx
+ 
                    .saturating_add(start.unwrap_or_default())
+ 
                    .saturating_sub(1);
+ 
                MergeLocation::Line(idx)
+ 
            };
+
+ 
            if location != MergeLocation::Start {
+ 
                merged.push(line.clone());
+ 
            }
- 
            let actual_idx = idx.saturating_add(start);
- 
            if let Some(mixins) = mixins.get(&actual_idx) {
- 
                for mixin in mixins {
- 
                    for mixin_line in mixin {
- 
                        merged.push(mixin_line.clone());
+ 
            if let Some(merges) = merge.get(&location) {
+ 
                for merge in merges {
+ 
                    for line in merge {
+ 
                        merged.push(line.clone());

                    }

                }

            }
+
+ 
            if location == MergeLocation::Start {
+ 
                merged.push(line.clone());
+ 
            }

        }



        merged
@@ -32,7 +71,7 @@ mod test {


    use pretty_assertions::assert_eq;
- 
    use crate::ui::utils::LineMerger;
+ 
    use crate::ui::utils::{LineMerger, MergeLocation};



    #[test]

    fn lines_should_be_merged_correctly() -> anyhow::Result<()> {
@@ -53,10 +92,9 @@ Is this needed?
            .to_string();

        let comment = comment.lines().collect::<Vec<_>>();
- 
        let merged = LineMerger::merge(
- 
            diff.lines().collect(),
- 
            HashMap::from([(3_usize, vec![comment])]),
- 
            1,
+ 
        let merged = LineMerger::new(diff.lines()).merge(
+ 
            HashMap::from([(MergeLocation::Line(2), vec![comment])]),
+ 
            Some(1),

        );

        let actual = build_string(merged);
@@ -100,10 +138,9 @@ Is this needed?
            .to_string();

        let comment = comment.lines().collect::<Vec<_>>();
- 
        let merged = LineMerger::merge(
- 
            diff.lines().collect(),
- 
            HashMap::from([(104_usize, vec![comment])]),
- 
            100,
+ 
        let merged = LineMerger::new(diff.lines()).merge(
+ 
            HashMap::from([(MergeLocation::Line(103), vec![comment])]),
+ 
            Some(100),

        );

        let actual = build_string(merged);