Merge branch 'master' into debug

11 files changed, 351 insertions, 93 deletions

diff --git a/helix-view/src/clipboard.rs b/helix-view/src/clipboard.rs
index a11224ac..a492652d 100644
--- a/helix-view/src/clipboard.rs
+++ b/helix-view/src/clipboard.rs
@@ -116,7 +116,7 @@ pub fn get_clipboard_provider() -> Box<dyn ClipboardProvider> {
         }
     } else {
         #[cfg(target_os = "windows")]
-        return Box::new(provider::WindowsProvider::new());
+        return Box::new(provider::WindowsProvider::default());
 
         #[cfg(not(target_os = "windows"))]
         return Box::new(provider::NopProvider::new());
@@ -145,15 +145,15 @@ mod provider {
     use anyhow::{bail, Context as _, Result};
     use std::borrow::Cow;
 
+    #[cfg(not(target_os = "windows"))]
     #[derive(Debug)]
     pub struct NopProvider {
         buf: String,
         primary_buf: String,
     }
 
+    #[cfg(not(target_os = "windows"))]
     impl NopProvider {
-        #[allow(dead_code)]
-        // Only dead_code on Windows.
         pub fn new() -> Self {
             Self {
                 buf: String::new(),
@@ -162,6 +162,7 @@ mod provider {
         }
     }
 
+    #[cfg(not(target_os = "windows"))]
     impl ClipboardProvider for NopProvider {
         fn name(&self) -> Cow<str> {
             Cow::Borrowed("none")
@@ -186,19 +187,8 @@ mod provider {
     }
 
     #[cfg(target_os = "windows")]
-    #[derive(Debug)]
-    pub struct WindowsProvider {
-        selection_buf: String,
-    }
-
-    #[cfg(target_os = "windows")]
-    impl WindowsProvider {
-        pub fn new() -> Self {
-            Self {
-                selection_buf: String::new(),
-            }
-        }
-    }
+    #[derive(Default, Debug)]
+    pub struct WindowsProvider;
 
     #[cfg(target_os = "windows")]
     impl ClipboardProvider for WindowsProvider {
diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index 1f1b1f5f..ce5df8ee 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -23,6 +23,8 @@ use crate::{DocumentId, Theme, ViewId};
 /// 8kB of buffer space for encoding and decoding `Rope`s.
 const BUF_SIZE: usize = 8192;
 
+const DEFAULT_INDENT: IndentStyle = IndentStyle::Spaces(4);
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub enum Mode {
     Normal,
@@ -95,6 +97,9 @@ pub struct Document {
     // it back as it separated from the edits. We could split out the parts manually but that will
     // be more troublesome.
     history: Cell<History>,
+
+    pub savepoint: Option<Transaction>,
+
     last_saved_revision: usize,
     version: i32, // should be usize?
 
@@ -306,8 +311,7 @@ where
     T: Default,
     F: FnOnce(T) -> T,
 {
-    let t = mem::take(mut_ref);
-    let _ = mem::replace(mut_ref, f(t));
+    *mut_ref = f(mem::take(mut_ref));
 }
 
 use helix_lsp::lsp;
@@ -325,7 +329,8 @@ impl Document {
             encoding,
             text,
             selections: HashMap::default(),
-            indent_style: IndentStyle::Spaces(4),
+            indent_style: DEFAULT_INDENT,
+            line_ending: DEFAULT_LINE_ENDING,
             mode: Mode::Normal,
             restore_cursor: false,
             syntax: None,
@@ -335,9 +340,9 @@ impl Document {
             diagnostics: Vec::new(),
             version: 0,
             history: Cell::new(History::default()),
+            savepoint: None,
             last_saved_revision: 0,
             language_server: None,
-            line_ending: DEFAULT_LINE_ENDING,
         }
     }
 
@@ -363,7 +368,7 @@ impl Document {
         let mut doc = Self::from(rope, Some(encoding));
 
         // set the path and try detecting the language
-        doc.set_path(path)?;
+        doc.set_path(Some(path))?;
         if let Some(loader) = config_loader {
             doc.detect_language(theme, loader);
         }
@@ -495,17 +500,15 @@ impl Document {
     }
 
     /// Detect the indentation used in the file, or otherwise defaults to the language indentation
-    /// configured in `languages.toml`, with a fallback  back to 2 space indentation if it isn't
+    /// configured in `languages.toml`, with a fallback to 4 space indentation if it isn't
     /// specified. Line ending is likewise auto-detected, and will fallback to the default OS
     /// line ending.
     pub fn detect_indent_and_line_ending(&mut self) {
         self.indent_style = auto_detect_indent_style(&self.text).unwrap_or_else(|| {
-            IndentStyle::from_str(
-                self.language
-                    .as_ref()
-                    .and_then(|config| config.indent.as_ref())
-                    .map_or("  ", |config| config.unit.as_str()), // Fallback to 2 spaces.
-            )
+            self.language
+                .as_ref()
+                .and_then(|config| config.indent.as_ref())
+                .map_or(DEFAULT_INDENT, |config| IndentStyle::from_str(&config.unit))
         });
         self.line_ending = auto_detect_line_ending(&self.text).unwrap_or(DEFAULT_LINE_ENDING);
     }
@@ -550,12 +553,14 @@ impl Document {
         self.encoding
     }
 
-    pub fn set_path(&mut self, path: &Path) -> Result<(), std::io::Error> {
-        let path = helix_core::path::get_canonicalized_path(path)?;
+    pub fn set_path(&mut self, path: Option<&Path>) -> Result<(), std::io::Error> {
+        let path = path
+            .map(helix_core::path::get_canonicalized_path)
+            .transpose()?;
 
         // if parent doesn't exist we still want to open the document
         // and error out when document is saved
-        self.path = Some(path);
+        self.path = path;
 
         Ok(())
     }
@@ -635,6 +640,14 @@ impl Document {
         if !transaction.changes().is_empty() {
             self.version += 1;
 
+            // generate revert to savepoint
+            if self.savepoint.is_some() {
+                take_with(&mut self.savepoint, |prev_revert| {
+                    let revert = transaction.invert(&old_doc);
+                    Some(revert.compose(prev_revert.unwrap()))
+                });
+            }
+
             // update tree-sitter syntax tree
             if let Some(syntax) = &mut self.syntax {
                 // TODO: no unwrap
@@ -644,14 +657,13 @@ impl Document {
             }
 
             // map state.diagnostics over changes::map_pos too
-            // NOTE: seems to do nothing since the language server resends diagnostics on each edit
-            // for diagnostic in &mut self.diagnostics {
-            //     use helix_core::Assoc;
-            //     let changes = transaction.changes();
-            //     diagnostic.range.start = changes.map_pos(diagnostic.range.start, Assoc::After);
-            //     diagnostic.range.end = changes.map_pos(diagnostic.range.end, Assoc::After);
-            //     diagnostic.line = self.text.char_to_line(diagnostic.range.start);
-            // }
+            for diagnostic in &mut self.diagnostics {
+                use helix_core::Assoc;
+                let changes = transaction.changes();
+                diagnostic.range.start = changes.map_pos(diagnostic.range.start, Assoc::After);
+                diagnostic.range.end = changes.map_pos(diagnostic.range.end, Assoc::After);
+                diagnostic.line = self.text.char_to_line(diagnostic.range.start);
+            }
 
             // emit lsp notification
             if let Some(language_server) = self.language_server() {
@@ -692,8 +704,8 @@ impl Document {
         success
     }
 
-    /// Undo the last modification to the [`Document`].
-    pub fn undo(&mut self, view_id: ViewId) {
+    /// Undo the last modification to the [`Document`]. Returns whether the undo was successful.
+    pub fn undo(&mut self, view_id: ViewId) -> bool {
         let mut history = self.history.take();
         let success = if let Some(transaction) = history.undo() {
             self.apply_impl(transaction, view_id)
@@ -706,10 +718,11 @@ impl Document {
             // reset changeset to fix len
             self.changes = ChangeSet::new(self.text());
         }
+        success
     }
 
-    /// Redo the last modification to the [`Document`].
-    pub fn redo(&mut self, view_id: ViewId) {
+    /// Redo the last modification to the [`Document`]. Returns whether the redo was sucessful.
+    pub fn redo(&mut self, view_id: ViewId) -> bool {
         let mut history = self.history.take();
         let success = if let Some(transaction) = history.redo() {
             self.apply_impl(transaction, view_id)
@@ -722,6 +735,17 @@ impl Document {
             // reset changeset to fix len
             self.changes = ChangeSet::new(self.text());
         }
+        success
+    }
+
+    pub fn savepoint(&mut self) {
+        self.savepoint = Some(Transaction::new(self.text()));
+    }
+
+    pub fn restore(&mut self, view_id: ViewId) {
+        if let Some(revert) = self.savepoint.take() {
+            self.apply(&revert, view_id);
+        }
     }
 
     /// Undo modifications to the [`Document`] according to `uk`.
@@ -894,6 +918,9 @@ impl Document {
 
     pub fn set_diagnostics(&mut self, diagnostics: Vec<Diagnostic>) {
         self.diagnostics = diagnostics;
+        // sort by range
+        self.diagnostics
+            .sort_unstable_by_key(|diagnostic| diagnostic.range);
     }
 }
 
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 60864e9e..591e0492 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -2,7 +2,7 @@ use crate::{
     clipboard::{get_clipboard_provider, ClipboardProvider},
     graphics::{CursorKind, Rect},
     theme::{self, Theme},
-    tree::Tree,
+    tree::{self, Tree},
     Document, DocumentId, View, ViewId,
 };
 
@@ -12,6 +12,7 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 
 use std::{
     collections::HashMap,
+    collections::BTreeMap,
     path::{Path, PathBuf},
     pin::Pin,
     sync::Arc,
@@ -19,8 +20,6 @@ use std::{
 
 use tokio::time::{sleep, Duration, Instant, Sleep};
 
-use slotmap::SlotMap;
-
 use anyhow::Error;
 
 pub use helix_core::diagnostic::Severity;
@@ -63,6 +62,9 @@ pub struct Config {
     /// Time in milliseconds since last keypress before idle timers trigger. Used for autocompletion, set to 0 for instant. Defaults to 400ms.
     #[serde(skip_serializing, deserialize_with = "deserialize_duration_millis")]
     pub idle_timeout: Duration,
+    pub completion_trigger_len: u8,
+    /// Whether to display infoboxes. Defaults to true.
+    pub auto_info: bool,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
@@ -92,14 +94,29 @@ impl Default for Config {
             auto_pairs: true,
             auto_completion: true,
             idle_timeout: Duration::from_millis(400),
+            completion_trigger_len: 2,
+            auto_info: true,
         }
     }
 }
 
+pub struct Motion(pub Box<dyn Fn(&mut Editor)>);
+impl Motion {
+    pub fn run(&self, e: &mut Editor) {
+        (self.0)(e)
+    }
+}
+impl std::fmt::Debug for Motion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("motion")
+    }
+}
+
 #[derive(Debug)]
 pub struct Editor {
     pub tree: Tree,
-    pub documents: SlotMap<DocumentId, Document>,
+    pub next_document_id: usize,
+    pub documents: BTreeMap<DocumentId, Document>,
     pub count: Option<std::num::NonZeroUsize>,
     pub selected_register: Option<char>,
     pub registers: Registers,
@@ -124,6 +141,7 @@ pub struct Editor {
     pub config: Config,
 
     pub idle_timer: Pin<Box<Sleep>>,
+    pub last_motion: Option<Motion>,
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -148,7 +166,8 @@ impl Editor {
 
         Self {
             tree: Tree::new(area),
-            documents: SlotMap::with_key(),
+            next_document_id: 0,
+            documents: BTreeMap::new(),
             count: None,
             selected_register: None,
             theme: themes.default(),
@@ -166,6 +185,7 @@ impl Editor {
             clipboard_provider: get_clipboard_provider(),
             status_msg: None,
             idle_timer: Box::pin(sleep(config.idle_timeout)),
+            last_motion: None,
             config,
         }
     }
@@ -221,7 +241,7 @@ impl Editor {
 
     fn _refresh(&mut self) {
         for (view, _) in self.tree.views_mut() {
-            let doc = &self.documents[view.doc];
+            let doc = &self.documents[&view.doc];
             view.ensure_cursor_in_view(doc, self.config.scrolloff)
         }
     }
@@ -230,22 +250,38 @@ impl Editor {
         use crate::tree::Layout;
         use helix_core::Selection;
 
-        if !self.documents.contains_key(id) {
+        if !self.documents.contains_key(&id) {
             log::error!("cannot switch to document that does not exist (anymore)");
             return;
         }
 
         match action {
             Action::Replace => {
-                let view = view!(self);
-                let jump = (
-                    view.doc,
-                    self.documents[view.doc].selection(view.id).clone(),
-                );
-
+                let (view, doc) = current_ref!(self);
+                // If the current view is an empty scratch buffer and is not displayed in any other views, delete it.
+                // Boolean value is determined before the call to `view_mut` because the operation requires a borrow
+                // of `self.tree`, which is mutably borrowed when `view_mut` is called.
+                let remove_empty_scratch = !doc.is_modified()
+                    // If the buffer has no path and is not modified, it is an empty scratch buffer.
+                    && doc.path().is_none()
+                    // If the buffer we are changing to is not this buffer
+                    && id != doc.id
+                    // Ensure the buffer is not displayed in any other splits.
+                    && !self
+                        .tree
+                        .traverse()
+                        .any(|(_, v)| v.doc == doc.id && v.id != view.id);
                 let view = view_mut!(self);
-                view.jumps.push(jump);
-                view.last_accessed_doc = Some(view.doc);
+                if remove_empty_scratch {
+                    // Copy `doc.id` into a variable before calling `self.documents.remove`, which requires a mutable
+                    // borrow, invalidating direct access to `doc.id`.
+                    let id = doc.id;
+                    self.documents.remove(&id);
+                } else {
+                    let jump = (view.doc, doc.selection(view.id).clone());
+                    view.jumps.push(jump);
+                    view.last_accessed_doc = Some(view.doc);
+                }
                 view.doc = id;
                 view.offset = Position::default();
 
@@ -272,14 +308,14 @@ impl Editor {
                 let view = View::new(id);
                 let view_id = self.tree.split(view, Layout::Horizontal);
                 // initialize selection for view
-                let doc = &mut self.documents[id];
+                let doc = self.documents.get_mut(&id).unwrap();
                 doc.selections.insert(view_id, Selection::point(0));
             }
             Action::VerticalSplit => {
                 let view = View::new(id);
                 let view_id = self.tree.split(view, Layout::Vertical);
                 // initialize selection for view
-                let doc = &mut self.documents[id];
+                let doc = self.documents.get_mut(&id).unwrap();
                 doc.selections.insert(view_id, Selection::point(0));
             }
         }
@@ -288,9 +324,11 @@ impl Editor {
     }
 
     pub fn new_file(&mut self, action: Action) -> DocumentId {
-        let doc = Document::default();
-        let id = self.documents.insert(doc);
-        self.documents[id].id = id;
+        let id = DocumentId(self.next_document_id);
+        self.next_document_id += 1;
+        let mut doc = Document::default();
+        doc.id = id;
+        self.documents.insert(id, doc);
         self.switch(id, action);
         id
     }
@@ -313,7 +351,11 @@ impl Editor {
                 self.language_servers
                     .get(language)
                     .map_err(|e| {
-                        log::error!("Failed to get LSP, {}, for `{}`", e, language.scope())
+                        log::error!(
+                            "Failed to initialize the LSP for `{}` {{ {} }}",
+                            language.scope(),
+                            e
+                        )
                     })
                     .ok()
             });
@@ -336,8 +378,10 @@ impl Editor {
                 doc.set_language_server(Some(language_server));
             }
 
-            let id = self.documents.insert(doc);
-            self.documents[id].id = id;
+            let id = DocumentId(self.next_document_id);
+            self.next_document_id += 1;
+            doc.id = id;
+            self.documents.insert(id, doc);
             id
         };
 
@@ -348,16 +392,20 @@ impl Editor {
     pub fn close(&mut self, id: ViewId, close_buffer: bool) {
         let view = self.tree.get(self.tree.focus);
         // remove selection
-        self.documents[view.doc].selections.remove(&id);
+        self.documents
+            .get_mut(&view.doc)
+            .unwrap()
+            .selections
+            .remove(&id);
 
         if close_buffer {
             // get around borrowck issues
-            let doc = &self.documents[view.doc];
+            let doc = &self.documents[&view.doc];
 
             if let Some(language_server) = doc.language_server() {
                 tokio::spawn(language_server.text_document_did_close(doc.identifier()));
             }
-            self.documents.remove(view.doc);
+            self.documents.remove(&view.doc);
         }
 
         self.tree.remove(id);
@@ -374,24 +422,40 @@ impl Editor {
         self.tree.focus_next();
     }
 
+    pub fn focus_right(&mut self) {
+        self.tree.focus_direction(tree::Direction::Right);
+    }
+
+    pub fn focus_left(&mut self) {
+        self.tree.focus_direction(tree::Direction::Left);
+    }
+
+    pub fn focus_up(&mut self) {
+        self.tree.focus_direction(tree::Direction::Up);
+    }
+
+    pub fn focus_down(&mut self) {
+        self.tree.focus_direction(tree::Direction::Down);
+    }
+
     pub fn should_close(&self) -> bool {
         self.tree.is_empty()
     }
 
     pub fn ensure_cursor_in_view(&mut self, id: ViewId) {
         let view = self.tree.get_mut(id);
-        let doc = &self.documents[view.doc];
+        let doc = &self.documents[&view.doc];
         view.ensure_cursor_in_view(doc, self.config.scrolloff)
     }
 
     #[inline]
     pub fn document(&self, id: DocumentId) -> Option<&Document> {
-        self.documents.get(id)
+        self.documents.get(&id)
     }
 
     #[inline]
     pub fn document_mut(&mut self, id: DocumentId) -> Option<&mut Document> {
-        self.documents.get_mut(id)
+        self.documents.get_mut(&id)
     }
 
     #[inline]
@@ -416,7 +480,7 @@ impl Editor {
 
     pub fn cursor(&self) -> (Option<Position>, CursorKind) {
         let view = view!(self);
-        let doc = &self.documents[view.doc];
+        let doc = &self.documents[&view.doc];
         let cursor = doc
             .selection(view.id)
             .primary()
diff --git a/helix-view/src/info.rs b/helix-view/src/info.rs
index 629a3112..b5a002fa 100644
--- a/helix-view/src/info.rs
+++ b/helix-view/src/info.rs
@@ -1,6 +1,6 @@
 use crate::input::KeyEvent;
 use helix_core::unicode::width::UnicodeWidthStr;
-use std::fmt::Write;
+use std::{collections::BTreeSet, fmt::Write};
 
 #[derive(Debug)]
 /// Info box used in editor. Rendering logic will be in other crate.
@@ -16,7 +16,7 @@ pub struct Info {
 }
 
 impl Info {
-    pub fn new(title: &str, body: Vec<(&str, Vec<KeyEvent>)>) -> Info {
+    pub fn new(title: &str, body: Vec<(&str, BTreeSet<KeyEvent>)>) -> Info {
         let body = body
             .into_iter()
             .map(|(desc, events)| {
diff --git a/helix-view/src/input.rs b/helix-view/src/input.rs
index 1e0ddfe2..580204cc 100644
--- a/helix-view/src/input.rs
+++ b/helix-view/src/input.rs
@@ -8,7 +8,7 @@ use crate::keyboard::{KeyCode, KeyModifiers};
 
 /// Represents a key event.
 // We use a newtype here because we want to customize Deserialize and Display.
-#[derive(Debug, PartialEq, Eq, PartialOrd, Clone, Copy, Hash)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
 pub struct KeyEvent {
     pub code: KeyCode,
     pub modifiers: KeyModifiers,
diff --git a/helix-view/src/keyboard.rs b/helix-view/src/keyboard.rs
index 26a4d6d2..810aa063 100644
--- a/helix-view/src/keyboard.rs
+++ b/helix-view/src/keyboard.rs
@@ -54,7 +54,7 @@ impl From<crossterm::event::KeyModifiers> for KeyModifiers {
 }

 

 /// Represents a key.

-#[derive(Debug, PartialOrd, PartialEq, Eq, Clone, Copy, Hash)]

+#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Clone, Copy, Hash)]

 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]

 pub enum KeyCode {

     /// Backspace key.

diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs
index c37474d6..3e779356 100644
--- a/helix-view/src/lib.rs
+++ b/helix-view/src/lib.rs
@@ -12,8 +12,10 @@ pub mod theme;
 pub mod tree;
 pub mod view;
 
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Debug)]
+pub struct DocumentId(usize);
+
 slotmap::new_key_type! {
-    pub struct DocumentId;
     pub struct ViewId;
 }
 
diff --git a/helix-view/src/macros.rs b/helix-view/src/macros.rs
index 0bebd02f..63d76a42 100644
--- a/helix-view/src/macros.rs
+++ b/helix-view/src/macros.rs
@@ -13,7 +13,8 @@
 macro_rules! current {
     ( $( $editor:ident ).+ ) => {{
         let view = $crate::view_mut!( $( $editor ).+ );
-        let doc = &mut $( $editor ).+ .documents[view.doc];
+        let id = view.doc;
+        let doc = $( $editor ).+ .documents.get_mut(&id).unwrap();
         (view, doc)
     }};
 }
@@ -56,7 +57,7 @@ macro_rules! doc {
 macro_rules! current_ref {
     ( $( $editor:ident ).+ ) => {{
         let view = $( $editor ).+ .tree.get($( $editor ).+ .tree.focus);
-        let doc = &$( $editor ).+ .documents[view.doc];
+        let doc = &$( $editor ).+ .documents[&view.doc];
         (view, doc)
     }};
 }
diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index 9c33685b..757316bd 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -1,6 +1,5 @@
 use std::{
     collections::HashMap,
-    convert::TryFrom,
     path::{Path, PathBuf},
 };
 
diff --git a/helix-view/src/tree.rs b/helix-view/src/tree.rs
index 576f64f0..064334b1 100644
--- a/helix-view/src/tree.rs
+++ b/helix-view/src/tree.rs
@@ -47,13 +47,21 @@ impl Node {
 
 // TODO: screen coord to container + container coordinate helpers
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Layout {
     Horizontal,
     Vertical,
     // could explore stacked/tabbed
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum Direction {
+    Up,
+    Down,
+    Left,
+    Right,
+}
+
 #[derive(Debug)]
 pub struct Container {
     layout: Layout,
@@ -150,7 +158,6 @@ impl Tree {
             } => container,
             _ => unreachable!(),
         };
-
         if container.layout == layout {
             // insert node after the current item if there is children already
             let pos = if container.children.is_empty() {
@@ -393,6 +400,112 @@ impl Tree {
         Traverse::new(self)
     }
 
+    // Finds the split in the given direction if it exists
+    pub fn find_split_in_direction(&self, id: ViewId, direction: Direction) -> Option<ViewId> {
+        let parent = self.nodes[id].parent;
+        // Base case, we found the root of the tree
+        if parent == id {
+            return None;
+        }
+        // Parent must always be a container
+        let parent_container = match &self.nodes[parent].content {
+            Content::Container(container) => container,
+            Content::View(_) => unreachable!(),
+        };
+
+        match (direction, parent_container.layout) {
+            (Direction::Up, Layout::Vertical)
+            | (Direction::Left, Layout::Horizontal)
+            | (Direction::Right, Layout::Horizontal)
+            | (Direction::Down, Layout::Vertical) => {
+                // The desired direction of movement is not possible within
+                // the parent container so the search must continue closer to
+                // the root of the split tree.
+                self.find_split_in_direction(parent, direction)
+            }
+            (Direction::Up, Layout::Horizontal)
+            | (Direction::Down, Layout::Horizontal)
+            | (Direction::Left, Layout::Vertical)
+            | (Direction::Right, Layout::Vertical) => {
+                // It's possible to move in the desired direction within
+                // the parent container so an attempt is made to find the
+                // correct child.
+                match self.find_child(id, &parent_container.children, direction) {
+                    // Child is found, search is ended
+                    Some(id) => Some(id),
+                    // A child is not found. This could be because of either two scenarios
+                    // 1. Its not possible to move in the desired direction, and search should end
+                    // 2. A layout like the following with focus at X and desired direction Right
+                    // | _ | x |   |
+                    // | _ _ _ |   |
+                    // | _ _ _ |   |
+                    // The container containing X ends at X so no rightward movement is possible
+                    // however there still exists another view/container to the right that hasn't
+                    // been explored. Thus another search is done here in the parent container
+                    // before concluding it's not possible to move in the desired direction.
+                    None => self.find_split_in_direction(parent, direction),
+                }
+            }
+        }
+    }
+
+    fn find_child(&self, id: ViewId, children: &[ViewId], direction: Direction) -> Option<ViewId> {
+        let mut child_id = match direction {
+            // index wise in the child list the Up and Left represents a -1
+            // thus reversed iterator.
+            Direction::Up | Direction::Left => children
+                .iter()
+                .rev()
+                .skip_while(|i| **i != id)
+                .copied()
+                .nth(1)?,
+            // Down and Right => +1 index wise in the child list
+            Direction::Down | Direction::Right => {
+                children.iter().skip_while(|i| **i != id).copied().nth(1)?
+            }
+        };
+        let (current_x, current_y) = match &self.nodes[self.focus].content {
+            Content::View(current_view) => (current_view.area.left(), current_view.area.top()),
+            Content::Container(_) => unreachable!(),
+        };
+
+        // If the child is a container the search finds the closest container child
+        // visually based on screen location.
+        while let Content::Container(container) = &self.nodes[child_id].content {
+            match (direction, container.layout) {
+                (_, Layout::Vertical) => {
+                    // find closest split based on x because y is irrelevant
+                    // in a vertical container (and already correct based on previous search)
+                    child_id = *container.children.iter().min_by_key(|id| {
+                        let x = match &self.nodes[**id].content {
+                            Content::View(view) => view.inner_area().left(),
+                            Content::Container(container) => container.area.left(),
+                        };
+                        (current_x as i16 - x as i16).abs()
+                    })?;
+                }
+                (_, Layout::Horizontal) => {
+                    // find closest split based on y because x is irrelevant
+                    // in a horizontal container (and already correct based on previous search)
+                    child_id = *container.children.iter().min_by_key(|id| {
+                        let y = match &self.nodes[**id].content {
+                            Content::View(view) => view.inner_area().top(),
+                            Content::Container(container) => container.area.top(),
+                        };
+                        (current_y as i16 - y as i16).abs()
+                    })?;
+                }
+            }
+        }
+        Some(child_id)
+    }
+
+    pub fn focus_direction(&mut self, direction: Direction) {
+        if let Some(id) = self.find_split_in_direction(self.focus, direction) {
+            self.focus = id;
+        }
+    }
+
     pub fn focus_next(&mut self) {
         // This function is very dumb, but that's because we don't store any parent links.
         // (we'd be able to go parent.next_sibling() recursively until we find something)
@@ -420,13 +533,12 @@ impl Tree {
         //   if found = container -> found = first child
         // }
 
-        let iter = self.traverse();
-
-        let mut iter = iter.skip_while(|&(key, _view)| key != self.focus);
-        iter.next(); // take the focused value
-
-        if let Some((key, _)) = iter.next() {
-            self.focus = key;
+        let mut views = self
+            .traverse()
+            .skip_while(|&(id, _view)| id != self.focus)
+            .skip(1); // Skip focused value
+        if let Some((id, _)) = views.next() {
+            self.focus = id;
         } else {
             // extremely crude, take the first item again
             let (key, _) = self.traverse().next().unwrap();
@@ -472,3 +584,64 @@ impl<'a> Iterator for Traverse<'a> {
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::DocumentId;
+
+    #[test]
+    fn find_split_in_direction() {
+        let mut tree = Tree::new(Rect {
+            x: 0,
+            y: 0,
+            width: 180,
+            height: 80,
+        });
+        let mut view = View::new(DocumentId::default());
+        view.area = Rect::new(0, 0, 180, 80);
+        tree.insert(view);
+
+        let l0 = tree.focus;
+        let view = View::new(DocumentId::default());
+        tree.split(view, Layout::Vertical);
+        let r0 = tree.focus;
+
+        tree.focus = l0;
+        let view = View::new(DocumentId::default());
+        tree.split(view, Layout::Horizontal);
+        let l1 = tree.focus;
+
+        tree.focus = l0;
+        let view = View::new(DocumentId::default());
+        tree.split(view, Layout::Vertical);
+        let l2 = tree.focus;
+
+        // Tree in test
+        // | L0  | L2 |    |
+        // |    L1    | R0 |
+        tree.focus = l2;
+        assert_eq!(Some(l0), tree.find_split_in_direction(l2, Direction::Left));
+        assert_eq!(Some(l1), tree.find_split_in_direction(l2, Direction::Down));
+        assert_eq!(Some(r0), tree.find_split_in_direction(l2, Direction::Right));
+        assert_eq!(None, tree.find_split_in_direction(l2, Direction::Up));
+
+        tree.focus = l1;
+        assert_eq!(None, tree.find_split_in_direction(l1, Direction::Left));
+        assert_eq!(None, tree.find_split_in_direction(l1, Direction::Down));
+        assert_eq!(Some(r0), tree.find_split_in_direction(l1, Direction::Right));
+        assert_eq!(Some(l0), tree.find_split_in_direction(l1, Direction::Up));
+
+        tree.focus = l0;
+        assert_eq!(None, tree.find_split_in_direction(l0, Direction::Left));
+        assert_eq!(Some(l1), tree.find_split_in_direction(l0, Direction::Down));
+        assert_eq!(Some(l2), tree.find_split_in_direction(l0, Direction::Right));
+        assert_eq!(None, tree.find_split_in_direction(l0, Direction::Up));
+
+        tree.focus = r0;
+        assert_eq!(Some(l2), tree.find_split_in_direction(r0, Direction::Left));
+        assert_eq!(None, tree.find_split_in_direction(r0, Direction::Down));
+        assert_eq!(None, tree.find_split_in_direction(r0, Direction::Right));
+        assert_eq!(None, tree.find_split_in_direction(r0, Direction::Up));
+    }
+}
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 8a7d3374..ee236e94 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -2,10 +2,9 @@ use std::borrow::Cow;
 
 use crate::{graphics::Rect, Document, DocumentId, ViewId};
 use helix_core::{
-    coords_at_pos,
     graphemes::{grapheme_width, RopeGraphemes},
     line_ending::line_end_char_index,
-    Position, RopeSlice, Selection,
+    visual_coords_at_pos, Position, RopeSlice, Selection,
 };
 
 type Jump = (DocumentId, Selection);
@@ -91,7 +90,10 @@ impl View {
             .selection(self.id)
             .primary()
             .cursor(doc.text().slice(..));
-        let Position { col, row: line } = coords_at_pos(doc.text().slice(..), cursor);
+
+        let Position { col, row: line } =
+            visual_coords_at_pos(doc.text().slice(..), cursor, doc.tab_width());
+
         let inner_area = self.inner_area();
         let last_line = (self.offset.row + inner_area.height as usize).saturating_sub(1);