Navigation between hunks

[panglesd]

Pressing n (next) and p (previous) navigates between operations: creation of a file, removal of file, edition, renaming, ...
Each operation contains several hunks of changes, that are displayed one after the other.

It would be nice to be able to only see one hunk, and navigate through them using f (forward) and b (backward).

The top status bar should ideally display the number of the visible hunk, as well as the total number of hunks.

[Kodecheff]

Hello, I'm an Outreachy applicant and would like to be assigned this task. The project repo is running successfully on my local machine. Here is a screenshot:

Thank you.

[Julow]

@Kodecheff Thanks :) You are assigned to this issue.

[Siddhi-agg]

@Kodecheff Hey! Another Outreachy applicant here! Are you still working on this issue? If not, I would love to work on this one. We can also work together if you are blocked:)

[Siddhi-agg]

@Julow Greetings! Since this issue has been inactive for a while now, can I work on this? I don't know if I should ask this, but no other issues were unassigned. I apologize if this is a bad practice.

[Kodecheff]

Hi @Siddhi-agg I am still actively working on this. Thank you

[Julow]

@Siddhi-agg Sorry, I do not want to unassign an issue so fast.

[Kodecheff]

@panglesd Just to clarify, this issue requires binding f and b keys to move to the next and previous hunks respectively. Because n and p currently do that; so I just need to replace the keys. That's about it right?

[Julow]

There's a slight difference between n/p and f/b.

A file can contain several hunks. The example.diff file do not showcase this yet but here's an example:

diff --git a/lib/interactive_viewer.ml b/lib/interactive_viewer.ml
index 79db405..85018ce 100644
--- a/lib/interactive_viewer.ml
+++ b/lib/interactive_viewer.ml
@@ -60,8 +60,6 @@ let navigate z_patches (dir : direction) : unit =
   | Prev -> Lwd.set z_patches (Zipper.prev z)
   | Next -> Lwd.set z_patches (Zipper.next z)
 
-let quit = Lwd.var false
-
 let additions_and_removals lines =
   let add_line (additions, removals) line =
     match line with
@@ -95,6 +93,8 @@ let change_summary z_patches : ui Lwd.t =
   in
   W.string ~attr:Notty.A.(fg lightcyan) operation_count
 
+let quit = Lwd.var false
+
 let view (patches : Patch.t list) =
   let z_patches : 'a Zipper.t Lwd.var =
     match Zipper.zipper_of_list patches with

Files start with a line starting in diff , hunks start with a line starting in @@.

The view function takes a Patch.t list but if you go to the Patch library, you can see that a patch contains hunks : hunk list ;.
n and p moves inside the first list (which is turned into a Zipper in a variable for this purpose) while f and b should move between hunks within the currently viewed file.

[panglesd]

@Kodecheff is it clearer with @Julow's explanations?

Since half of the contribution period has passed and we are missing good first issues, we now require assigned contributors to open a PR not too long after having being assigned a good first issue. The PR does not have to be fully finished, but has to show progress on the issue.

Could you open a PR with your progress, by Wednesday (or reach out if that causes a problem)? If not, we might have to assign the issue to someone else.

[Kodecheff]

Hello @panglesd @Julow sorry for the delay in sending a PR. I am having a problem with the current_hunks variable type. Please see the code below:

open Nottui
module W = Nottui_widgets
open Lwd_infix

let index_v = Lwd.var 0
let index = Lwd.get index_v

let hunk_h = Lwd.var 0
let hunk_index = Lwd.get hunk_h

let current_patch patches =
  let$ index = index in
  List.nth_opt patches index

let string_of_operation = Format.asprintf "%a" (Patch.pp_operation ~git:false)
let string_of_hunk = Format.asprintf "%a" Patch.pp_hunk

let current_operation patches =
  let$ current_patch = current_patch patches in
  match current_patch with
  | Some p -> W.string @@ string_of_operation p.Patch.operation
  | None -> W.string "No operation"

let current_hunks (patches) =
  let$ current_patch = current_patch patches in
  match current_patch with
  | Some p -> List.map (fun p -> string_of_hunk p) p.Patch.hunks
  | None -> []

let current_hunk current_hunks =
  let$ hunk_index = hunk_index in
  List.nth_opt current_hunks hunk_index

let show_hunk (current_hunks) = 
  let$ current_hunk = current_hunk current_hunks in 
  match current_hunk with 
  | Some p -> W.string @@ string_of_hunk p
  | None -> W.string "No operation"


(* let pure_str s = Lwd.pure (W.string s) *)
let quit = Lwd.var false

let view (patches : Patch.t list) =
  W.vbox
    [
      current_operation patches;
      (* W.scrollbox @@ current_hunks patches; *)
      W.scrollbox @@ show_hunk current_hunks;
      Lwd.pure
      @@ Ui.keyboard_area
           (function
             | `ASCII 'q', [] ->
                 Lwd.set quit true;
                 `Handled
             | `ASCII 'n', [] ->
                 Lwd.set index_v (Lwd.peek index_v + 1);
                 `Handled
             | `ASCII 'p', [] ->
                 Lwd.set index_v (Lwd.peek index_v - 1);
                 `Handled
             | `ASCII 'f', [] ->
                 Lwd.set hunk_h (Lwd.peek hunk_h + 1);
                `Handled
             | `ASCII 'b', [] ->
                 Lwd.set hunk_h (Lwd.peek hunk_h - 1);
              `Handled
             | _ -> `Unhandled)
           (W.string "Type 'q' to quit.");
    ]

let start patch = Ui_loop.run ~quit ~tick_period:0.2 (view patch)

show_hunk is expected to receive current_hunks of type Patch.hunk list. But the current_hunks variable is returning type string list Lwd.t instead. Do you have any idea what I am doing wrong?

[Julow]

Before you do anything else make sure to pull the latest version of the repository. The code changed a lot since you started working on this and is now very different from the code you based your modification on.

The current_hunks function returns a something Lwd.t because of this:

  let$ current_patch = current_patch patches in

current_patch patches returns a something Lwd.t too. A value of type _ Lwd.t is "reactive", to consume the value wrapped inside of it (in your case, get the string list out of the string list Lwd.t), you need to make a reactive value too.
let$ does that: current_patch patches returns a reactive value, the local variable current_patch (name is confusing) is not reactive but in exchange, your current_hunks function return a reactive value.

[Julow]

Hi, have you been able to pull the latest changes ?