From cd767dfde9ceaf4511c3f0c1339789101b9c2443 Mon Sep 17 00:00:00 2001 From: Eli Dowling Date: Tue, 18 Jun 2024 09:14:29 +1000 Subject: [PATCH] Try to prevent re-rending multiple times when updating --- forks/lwd/lib/lwd/lwd.ml | 1 + jj_tui/bin/file_view.ml | 8 ++-- jj_tui/bin/global_funcs.ml | 42 ++++++++++++++------ jj_tui/bin/global_vars.ml | 81 ++++++++++++++++++++++---------------- jj_tui/bin/jj_ui.ml | 4 +- jj_tui/bin/main.ml | 5 ++- jj_tui/lib/widgets.ml | 25 ++++++------ 7 files changed, 101 insertions(+), 65 deletions(-) diff --git a/forks/lwd/lib/lwd/lwd.ml b/forks/lwd/lib/lwd/lwd.ml index 8b829f8..74bdce1 100644 --- a/forks/lwd/lib/lwd/lwd.ml +++ b/forks/lwd/lib/lwd/lwd.ml @@ -88,6 +88,7 @@ let map2 x y ~f = inj ( | x, y -> operator (Map2 (x, y, f)) ) + let pair x y = inj ( match prj x, prj y with | Pure vx, Pure vy -> Pure (vx, vy) diff --git a/jj_tui/bin/file_view.ml b/jj_tui/bin/file_view.ml index 3dbd502..60a113e 100644 --- a/jj_tui/bin/file_view.ml +++ b/jj_tui/bin/file_view.ml @@ -39,7 +39,7 @@ module Make (Vars : Global_vars.Vars) = struct ] ;; - let file_view () = + let file_view sw () = let file_uis = let$ files = Lwd.get Vars.ui_state.jj_change_files in files @@ -47,7 +47,9 @@ module Make (Vars : Global_vars.Vars) = struct Wd.{ data = file; ui = Wd.selectable_item (W.string file) }) in Wd.selection_list_custom - ~on_selection_change:(Lwd.set selected_file) + ~on_selection_change:(fun x -> + Eio.Fiber.fork ~sw @@ fun _ -> + Vars.update_ui_state @@ fun _ -> Lwd.set selected_file x) ~custom_handler:(fun _ _ key -> match key with `ASCII k, [] -> handleInputs command_mapping k | _ -> `Unhandled) file_uis @@ -56,6 +58,6 @@ module Make (Vars : Global_vars.Vars) = struct (**Get the status for the currently selected file*) let file_status () = let$ selected = Lwd.get selected_file in - jj_no_log [ "diff"; selected ] + if selected != "" then jj_no_log [ "diff"; selected ] else "" ;; end diff --git a/jj_tui/bin/global_funcs.ml b/jj_tui/bin/global_funcs.ml index 7775529..6e420c7 100644 --- a/jj_tui/bin/global_funcs.ml +++ b/jj_tui/bin/global_funcs.ml @@ -21,21 +21,37 @@ let list_files ?(rev = "@") () = else None) ;; + (**Updates the state; Without snapshotting the working copy by default *) let on_change ?(cause_snapshot = false) () = Eio.Switch.run @@ fun sw -> - Eio.Fiber.fork ~sw (fun _ -> - let res = - jj_no_log ~snapshot:cause_snapshot [ "show"; "-s"; "--color-words" ] |> colored_string - in - Vars.ui_state.jj_show $= res); - (*From now on we use ignore-working-copy so we don't re-snapshot the state*) - Eio.Fiber.fork ~sw (fun _ -> - let res = jj_no_log ~snapshot:false [ "log" ] |> colored_string in - Vars.ui_state.jj_tree $= res); + let log_res = + jj_no_log ~snapshot:cause_snapshot [ "show"; "-s"; "--color-words" ] |> colored_string + in + (* From now on we use ignore-working-copy so we don't re-snapshot the state and so + we can operate in paralell *) + let tree = + Eio.Fiber.fork_promise ~sw (fun _ -> + jj_no_log ~snapshot:false [ "log" ] |> colored_string) (* TODO: stop using dop last twice *) - Eio.Fiber.fork ~sw (fun _ -> - let res = jj_no_log ~snapshot:false [ "branch"; "list"; "-a" ] |> colored_string in - Vars.ui_state.jj_branches $= res); - Eio.Fiber.fork ~sw (fun _ -> Vars.ui_state.jj_change_files $= list_files ()) + and branches = + Eio.Fiber.fork_promise ~sw (fun _ -> + jj_no_log ~snapshot:false [ "branch"; "list"; "-a" ] |> colored_string) + and files_list = + Eio.Fiber.fork_promise ~sw (fun _ -> + list_files ()) + in + (*wait for all our tasks*) + let tree = Eio.Promise.await_exn tree + and files_list = Eio.Promise.await_exn files_list + and branches = Eio.Promise.await_exn branches in + (*now we can assign our results*) + Vars.render_mutex|>Eio.Mutex.lock; + + Vars.ui_state.jj_show $= log_res; + Vars.ui_state.jj_branches $= branches; + Vars.ui_state.jj_tree $= tree; + Vars.ui_state.jj_change_files $= files_list; + + Vars.render_mutex|>Eio.Mutex.unlock; ;; diff --git a/jj_tui/bin/global_vars.ml b/jj_tui/bin/global_vars.ml index d8e714e..815cb3c 100644 --- a/jj_tui/bin/global_vars.ml +++ b/jj_tui/bin/global_vars.ml @@ -5,67 +5,81 @@ open Eio.Std type cmd_args = string list type ui_state_t = { - view : - [ `Main - | `Cmd_I of cmd_args - | `RunCmd of - cmd_args - (* | `Prompt of string * [ `Cmd of cmd_args | `Cmd_I of cmd_args ] *) - ] - Lwd.var; - input : [ `Normal | `Mode of char -> Ui.may_handle ] Lwd.var; - show_popup : (ui Lwd.t * string) option Lwd.var; - show_prompt : - (string * string * ([ `Finished of string | `Closed ] -> unit)) option Lwd.var; - command_log : string list Lwd.var; - jj_tree : I.t Lwd.var; - jj_show : I.t Lwd.var; - jj_branches : I.t Lwd.var; - jj_change_files : (string * string) list Lwd.var; + view : + [ `Main + | `Cmd_I of cmd_args + | `RunCmd of + cmd_args + (* | `Prompt of string * [ `Cmd of cmd_args | `Cmd_I of cmd_args ] *) + ] + Lwd.var + ; input : [ `Normal | `Mode of char -> Ui.may_handle ] Lwd.var + ; show_popup : (ui Lwd.t * string) option Lwd.var + ; show_prompt : + (string * string * ([ `Finished of string | `Closed ] -> unit)) option Lwd.var + ; command_log : string list Lwd.var + ; jj_tree : I.t Lwd.var + ; jj_show : I.t Lwd.var + ; jj_branches : I.t Lwd.var + ; jj_change_files : (string * string) list Lwd.var } (** Global variables for the ui*) module type Vars = sig type eio_vars = { - env : Eio_unix.Stdenv.base; - mgr : Eio_unix.Process.mgr_ty Eio.Resource.t; - cwd : Eio.Fs.dir_ty Eio.Path.t; + env : Eio_unix.Stdenv.base + ; mgr : Eio_unix.Process.mgr_ty Eio.Resource.t + ; cwd : Eio.Fs.dir_ty Eio.Path.t } val quit : bool Lwd.var val term : Notty_unix.Term.t option ref val term_width_height : (int * int) Lwd.var val get_eio_env : unit -> Eio_unix.Stdenv.base - val set_eio_env :Eio_unix.Stdenv.base->unit + val set_eio_env : Eio_unix.Stdenv.base -> unit val get_eio_vars : unit -> eio_vars val get_term : unit -> Notty_unix.Term.t val ui_state : ui_state_t + val update_ui_state: (ui_state_t->unit)->unit + + val render_mutex : Eio.Mutex.t end module Vars : Vars = struct type eio_vars = { - env : Eio_unix.Stdenv.base; - mgr : Eio_unix.Process.mgr_ty Eio.Resource.t; - cwd : Eio.Fs.dir_ty Eio.Path.t; + env : Eio_unix.Stdenv.base + ; mgr : Eio_unix.Process.mgr_ty Eio.Resource.t + ; cwd : Eio.Fs.dir_ty Eio.Path.t } let quit = Lwd.var false let eio = ref None + (** DONT DIRECTLY SET FROM EIO FIBERS!! When setting variables within a fiber use update_ui_state*) let ui_state = { - view = Lwd.var `Main; - jj_tree = Lwd.var I.empty; - jj_show = Lwd.var I.empty; - jj_branches = Lwd.var I.empty; - jj_change_files = Lwd.var []; - input = Lwd.var `Normal; - show_popup = Lwd.var None; - show_prompt = Lwd.var None; - command_log = Lwd.var []; + view = Lwd.var `Main + ; jj_tree = Lwd.var I.empty + ; jj_show = Lwd.var I.empty + ; jj_branches = Lwd.var I.empty + ; jj_change_files = Lwd.var [] + ; input = Lwd.var `Normal + ; show_popup = Lwd.var None + ; show_prompt = Lwd.var None + ; command_log = Lwd.var [] } ;; + (** allows other fibers to set lwd vars only when not during rendering*) + let render_mutex = Eio.Mutex.create () + + (** Safely ensures your update doesn't occur during a render*) + let update_ui_state f = + Eio.Mutex.lock render_mutex; + f ui_state; + Eio.Mutex.unlock render_mutex + ;; + let set_eio_env env = eio := Some { env; mgr = Eio.Stdenv.process_mgr env; cwd = Eio.Stdenv.cwd env } ;; @@ -75,5 +89,4 @@ module Vars : Vars = struct let get_eio_env () = (Option.get !eio).env let get_eio_vars () = Option.get !eio let get_term () = Option.get !term - end diff --git a/jj_tui/bin/jj_ui.ml b/jj_tui/bin/jj_ui.ml index 7999936..360d284 100644 --- a/jj_tui/bin/jj_ui.ml +++ b/jj_tui/bin/jj_ui.ml @@ -134,7 +134,7 @@ module Make (Vars : Global_vars.Vars) = struct let clock = Eio.Stdenv.clock env in while true do Eio.Time.sleep clock 5.0; - on_change () + on_change ~cause_snapshot:true (); done; `Stop_daemon); let$* running = Lwd.get ui_state.view in @@ -153,7 +153,7 @@ module Make (Vars : Global_vars.Vars) = struct W.h_pane (W.vbox [ - File_view.file_view () + File_view.file_view sw () |>$ Ui.resize ~w:10 ~sw:1 |> Wd.border_box_focusable ~focus:file_focus ~pad_h:0 ; Wd.v_scroll_area (ui_state.jj_tree $-> Ui.atom) diff --git a/jj_tui/bin/main.ml b/jj_tui/bin/main.ml index ec8c3e5..61607f8 100644 --- a/jj_tui/bin/main.ml +++ b/jj_tui/bin/main.ml @@ -26,14 +26,17 @@ let ui_loop ~quit ~term root = let prev_term_width, prev_term_height = Lwd.peek Vars.term_width_height in if term_width <> prev_term_width || term_height <> prev_term_height then Lwd.set Vars.term_width_height (term_width, term_height); + Vars.render_mutex|>Eio.Mutex.lock; Nottui.Ui_loop.step ~process_event:true ~timeout:0.05 ~renderer term (Lwd.observe @@ root); + Vars.render_mutex|>Eio.Mutex.unlock; Eio.Fiber.yield (); - loop ()) + loop () + ) in loop () ;; diff --git a/jj_tui/lib/widgets.ml b/jj_tui/lib/widgets.ml index 8f6370a..f44373b 100644 --- a/jj_tui/lib/widgets.ml +++ b/jj_tui/lib/widgets.ml @@ -5,7 +5,6 @@ open! Util open! Widgets_citty module W = Nottui_widgets - let dynamic_width = dynamic_width let dynamic_size ?(w = 10) ~sw ?(h = 10) ~sh f = @@ -615,7 +614,7 @@ let general_prompt ?(focus = Focus.make ()) ?(char_count = false) ~show_prompt_v ~on_submit:(fun (str, _) -> on_exit (`Finished str)) ] in - let$* prompt_val, _ = prompt_val + let$* prompt_val, _ = prompt_val and$ focus_status = focus |> Focus.status in let label_bottom = if char_count @@ -797,7 +796,6 @@ let v_scroll_area ui = ui |> W.vscroll_area ~change:(fun _ x -> state $= x) ~state:(Lwd.get state) ;; - (**This is a simple popup that can show ontop of *) let popup ~show_popup_var ui = let popup_ui = @@ -886,15 +884,18 @@ let selection_list_custom (*handle selections*) let render_items = let$ focus = focus |> Focus.status - and$ items = items - and$ selected = - Lwd.get selected_var in - (* First ensure if our list has gotten shorter we haven't selected off the list*) - (* We do this here to ensure that the selected var is updated before we render to avoid double rendering*) - let max_selected = Int.max 0 (List.length items - 1 ) in - if Int.min selected max_selected <> selected then selected_var $= max_selected; - let selected = Lwd.peek selected_var in - List.nth_opt items selected |> Option.iter (fun x -> on_selection_change x.data); + and$ items, selected = + (* This doesn't depend on changes in focus so we run it with just the items and selected*) + let$ items = items + and$ selected = Lwd.get selected_var in + (* First ensure if our list has gotten shorter we haven't selected off the list*) + (* We do this here to ensure that the selected var is updated before we render to avoid double rendering*) + let max_selected = Int.max 0 (List.length items - 1) in + if Int.min selected max_selected <> selected then selected_var $= max_selected; + let selected = Lwd.peek selected_var in + List.nth_opt items selected |> Option.iter (fun x -> on_selection_change x.data); + items, selected + in items |> List.mapi (fun i x -> if selected == i