Windows close in the wrong order

[lukepighetti]

Summary

If you open a bunch of windows, then start closing them, it always navigates the cursor to the first window, instead of the last selected window. Ideally the focus path travelled during window creation should be followed during window teardown

Reproduction Steps

Screen.Recording.2022-10-31.at.8.49.11.AM-HD.720p.mov

Helix log

~/.cache/helix/helix.log

please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines

Platform

macOS

Terminal Emulator

iTerm2

Helix Version

helix 22.08.1 (6752c7d)

[wes-adams]

i'd like to address this issue

[wes-adams]

i've made some progress, but i'm kinda stuck

keep in mind i'm brand new to Rust. i'm really stumbling my way through trying to learn.

@lukepighetti - feel free to dig into this a little

good case:

bad case:

split view, and then try to close original view

---

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', helix-view/src/tree.rs:286:29
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:48:5
   3: core::option::Option<T>::unwrap
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/option.rs:755:21
   4: helix_view::tree::Tree::get
             at ./helix-view/src/tree.rs:286:9
   5: helix_view::editor::Editor::cursor
             at ./helix-view/src/editor.rs:1316:27
   6: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::cursor
             at ./helix-term/src/ui/editor.rs:1553:15
   7: helix_term::compositor::Compositor::cursor
             at ./helix-term/src/compositor.rs:173:40
   8: helix_term::application::Application::render
             at ./helix-term/src/application.rs:298:27
   9: helix_term::application::Application::handle_terminal_events
             at ./helix-term/src/application.rs:599:13
  10: helix_term::application::Application::event_loop_until_idle::{{closure}}
             at ./helix-term/src/application.rs:332:21
  11: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
  12: helix_term::application::Application::event_loop::{{closure}}
             at ./helix-term/src/application.rs:311:57
  13: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
  14: helix_term::application::Application::run::{{closure}}
             at ./helix-term/src/application.rs:1009:38
  15: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
  16: hx::main_impl::{{closure}}
             at ./helix-term/src/main.rs:155:53
  17: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
  18: tokio::park::thread::CachedParkThread::block_on::{{closure}}
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/park/thread.rs:267:54
  19: tokio::coop::with_budget::{{closure}}
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:102:9
  20: std::thread::local::LocalKey<T>::try_with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:442:16
  21: std::thread::local::LocalKey<T>::with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:418:9
  22: tokio::coop::with_budget
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:95:5
  23: tokio::coop::budget
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:72:5
  24: tokio::park::thread::CachedParkThread::block_on
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/park/thread.rs:267:31
  25: tokio::runtime::enter::Enter::block_on
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/runtime/enter.rs:152:13
  26: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/multi_thread/mod.rs:79:9
  27: tokio::runtime::Runtime::block_on
             at /home/wadams/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-1.21.2/src/runtime/mod.rs:492:44
  28: hx::main_impl
             at ./helix-term/src/main.rs:157:5
  29: hx::main
             at ./helix-term/src/main.rs:37:21
  30: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5

[wes-adams]

i think after 755c933, the feature is complete

it would be great to get some help testing - is there a typical way to recruit help testing?

[lukepighetti]

Can this be modeled as a stack?

Numbers are pane index, last index is selected pane
open: [0]
split left: [0,1]
split down [0,1,2]
go to 1 [0,2,1]
go to 2 [0, 1, 2]
go to 0 [1, 2, 0]
close [1,2]
close [1]
close quit

[wes-adams]

my attempt adds a field to the Node struct to keep track of which view spawned the new node. when a view is closed, the focus goes back to the view that spawned it (if it still exists - if not, focus returns to the next view in an arbitrary list).

i can try some implementation with a stack as well

i've learned quite a bit about how views are added and removed, so another clean attempt with a stack might end up being better.

after some thought, i don't know if a stack is a great approach. a user can close any view at any time. a stack works on the assumption of FIFO or LIFO.

[lukepighetti]

With the stack (LIFO) approach, if the user selects a view, it is moved to the top of the stack.

Top of the stack is what's visible.

Closing a view pops it off the top of the stack.

[wes-adams]

i've re-worked the pull request in a different direction based on feedback from @archseer
it seems to work. it seems stable. its ready for further review and testing

[the-mikedavis]

Closed by #4766