Proposal: Visual selection of lines to be staged

[anselstetter]

This project is getting better and better. Thank you for your work on it. :heart:

Coming from Magit and using evil mode, I've used visual mode to highlight the lines I wanted to stage. So after pressing s, everything highlighted was staged at once, without the need to individual select each line.

Did you consider something like this?

This was really convenient.

[altsem]

Sorry for the late reply. Yes I think this should be in there! I can't remember how lines are selected in evil-magit. But preferably we'd just copy the behaviour imo.

[altsem]

Had a peek in Doom Emacs' Magit.

Reverse is bound to - Revert is bound to _ v and V enter VISUAL LINE mode.

Whereas in gitu, v is reverse and V Revert.

I guess we may want to mimic those.

[anselstetter]

Awesome. Nice to hear.

[jonathanj]

In the configuration of magit I use, holding shift while moving up/down will add the lines to selection (like you would expect in a GUI editor).

This would be a fantastic feature for gitu!

[jonathanj]

@altsem I started trying to implement this, my approach was to add ItemData::Multiple<Vec<ItemData>> and then have ops to enter/exit visual mode, then get_selection_item returns ItemData::Multiple if there's a visual selection, and ops that care about multiple things (such as stage) can unpack it, and ops that don't care (such as commit) can ignore it.

My implementation for stage is basically "iterate the multiple items, call self.get_action for each one then manually run action(app, term)", but the problem I've run into is that if you stage two adjacent lines that is two stage_line actions run separately but without the UI update between them to rebuild the ItemData resulting in a failed patch:

$ git apply --cached --recount
$ git apply --cached --recount
error: patch failed: src/ops/editor.rs:308
error: src/ops/editor.rs: patch does not apply
! 'git apply --cached --recount' exited with code: 1

I have no idea how magit does this internally, but it seems like there might need to be a way to combine consecutive ItemData things together when they can be combined. Not sure if this is the right track, or if this whole approach is bad.

[altsem]

@jonathanj I think this function will help you assemble the patch. You just need to pass it a file_i, hunk_i and a range of lines: https://github.com/altsem/gitu/blame/3ccaa5178369f6c000a94182656ae55360ba92e0/src/git/diff.rs?plain=1#L50-L56

    pub(crate) fn format_line_patch(
        &self,
        file_i: usize,
        hunk_i: usize,
        line_range: Range<usize>,
        mode: PatchMode,
    ) -> String {

ItemData store indices like this: https://github.com/altsem/gitu/blame/3ccaa5178369f6c000a94182656ae55360ba92e0/src/item_data.rs?plain=1#L22-L37

    Delta {
        diff: Rc<Diff>,
        file_i: usize,
    },
    Hunk {
        diff: Rc<Diff>,
        file_i: usize,
        hunk_i: usize,
    },
    HunkLine {
        diff: Rc<Diff>,
        file_i: usize,
        hunk_i: usize,
        line_i: usize,
        line_range: Range<usize>,
    },

line_range in both 🤔. I think line_range in HunkLine might refer a range of bytes that resemble the line in the original diff text. Whereas line_range in format_line_patch actually denote which lines you want to include in the patch.

But I suppose it does become more complicated as there's files / hunks / lines selected in various combinations. I think we should generate a single patch to apply though, instead of applying things 1-by-1.