From 143cfe13e05b17840a9f2c69417ed98bc3b8cb0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Tue, 16 Mar 2021 15:30:29 +0900 Subject: [PATCH] minor: TODO comment cleanup --- TODO.md | 2 ++ helix-lsp/src/client.rs | 22 ++++++---------------- helix-term/src/application.rs | 12 ------------ helix-term/src/commands.rs | 13 ------------- helix-term/src/compositor.rs | 3 --- helix-term/src/keymap.rs | 1 - helix-term/src/ui/editor.rs | 10 +++------- helix-view/src/document.rs | 4 +--- helix-view/src/view.rs | 9 +++------ 9 files changed, 15 insertions(+), 61 deletions(-) diff --git a/TODO.md b/TODO.md index a3152f19f..0411c12be 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,8 @@ - [ ] buffers should sit on editor.buffers, view simply refs them - [ ] yank on delete +- [ ] load toml configs, themes, tabsize/identation + - [ ] draw separator line between views - [ ] lsp: signature help diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index ae858ce07..edec10a92 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -192,7 +192,7 @@ impl Client { text_document: Some(lsp::TextDocumentClientCapabilities { completion: Some(lsp::CompletionClientCapabilities { completion_item: Some(lsp::CompletionItemCapability { - snippet_support: Some(false), // TODO + snippet_support: Some(false), ..Default::default() }), completion_item_kind: Some(lsp::CompletionItemKindCapability { @@ -284,7 +284,6 @@ impl Client { mut character, } = pos; - // TODO: there should be a better way here for ch in text.chars() { if ch == '\n' { line += 1; @@ -299,8 +298,6 @@ impl Client { let old_text = old_text.slice(..); let new_text = new_text.slice(..); - // TODO: verify this function, specifically line num counting - while let Some(change) = iter.next() { let len = match change { Delete(i) | Retain(i) => *i, @@ -355,7 +352,6 @@ impl Client { changes } - // TODO: trigger any time history.commit_revision happens pub async fn text_document_did_change( &self, text_document: lsp::VersionedTextDocumentIdentifier, @@ -365,7 +361,7 @@ impl Client { ) -> Result<()> { // figure out what kind of sync the server supports - let capabilities = self.capabilities.as_ref().unwrap(); // TODO: needs post init + let capabilities = self.capabilities.as_ref().unwrap(); let sync_capabilities = match capabilities.text_document_sync { Some(lsp::TextDocumentSyncCapability::Kind(kind)) => kind, @@ -384,7 +380,7 @@ impl Client { range: None, //Some(Range) range_length: None, // u64 apparently deprecated text: "".to_string(), - }] // TODO: probably need old_state here too? + }] } lsp::TextDocumentSyncKind::Incremental => { Self::changeset_to_changes(old_text, new_text, changes) @@ -416,7 +412,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, text: &Rope, ) -> Result<()> { - let capabilities = self.capabilities.as_ref().unwrap(); // TODO: needs post init + let capabilities = self.capabilities.as_ref().unwrap(); let include_text = match &capabilities.text_document_sync { Some(lsp::TextDocumentSyncCapability::Options(lsp::TextDocumentSyncOptions { @@ -446,8 +442,6 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, ) -> Result> { - // TODO: figure out what should happen when you complete with multiple cursors - let params = lsp::CompletionParams { text_document_position: lsp::TextDocumentPositionParams { text_document, @@ -489,7 +483,6 @@ impl Client { text_document, position, }, - // TODO: support these tokens work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token: None, }, @@ -514,7 +507,6 @@ impl Client { text_document, position, }, - // TODO: support these tokens work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token: None, }, @@ -533,7 +525,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, options: lsp::FormattingOptions, ) -> anyhow::Result> { - let capabilities = self.capabilities.as_ref().unwrap(); // TODO: needs post init + let capabilities = self.capabilities.as_ref().unwrap(); // check if we're able to format let _capabilities = match capabilities.document_formatting_provider { @@ -547,7 +539,6 @@ impl Client { let params = lsp::DocumentFormattingParams { text_document, options, - // TODO: support these tokens work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token: None, }, @@ -564,7 +555,7 @@ impl Client { range: lsp::Range, options: lsp::FormattingOptions, ) -> anyhow::Result> { - let capabilities = self.capabilities.as_ref().unwrap(); // TODO: needs post init + let capabilities = self.capabilities.as_ref().unwrap(); // check if we're able to format let _capabilities = match capabilities.document_range_formatting_provider { @@ -579,7 +570,6 @@ impl Client { text_document, range, options, - // TODO: support these tokens work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token: None, }, diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index c22cf9967..f239a2f02 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -221,15 +221,3 @@ impl Application { Ok(()) } } - -// TODO: language configs: -// tabSize, fileExtension etc, mapping to tree sitter parser -// themes: -// map tree sitter highlights to color values -// -// TODO: expand highlight thing so we're able to render only viewport range -// TODO: async: maybe pre-cache scopes as empty so we render all graphemes initially as regular -////text until calc finishes -// TODO: scope matching: biggest union match? [string] & [html, string], [string, html] & [ string, html] -// can do this by sorting our theme matches based on array len (longest first) then stopping at the -// first rule that matches (rule.all(|scope| scopes.contains(scope))) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 507e5be65..16f43591b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -460,19 +460,6 @@ pub fn select_regex(cx: &mut Context) { } pub fn split_selection(cx: &mut Context) { - // TODO: this needs to store initial selection state, revert on esc, confirm on enter - // needs to also call the callback function per input change, not just final time. - // could cheat and put it into completion_fn - // - // kakoune does it like this: - // # save state to register - // { - // # restore state from register - // # if event == abort, return early - // # add to history if enabled - // # update state - // } - let prompt = ui::regex_prompt(cx, "split:".to_string(), |doc, regex| { let text = doc.text().slice(..); let selection = selection::split_on_matches(text, doc.selection(), ®ex); diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index ba8453f38..d7ca6f260 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -54,9 +54,6 @@ pub trait Component { /// May be used by the parent component to compute the child area. /// viewport is the maximum allowed area, and the child should stay within those bounds. fn required_size(&mut self, viewport: (u16, u16)) -> Option<(u16, u16)> { - // TODO: the compositor should trigger this on push_layer too so that we can use it as an - // initializer there too. - // // TODO: for scrolling, the scroll wrapper should place a size + offset on the Context // that way render can use it None diff --git a/helix-term/src/keymap.rs b/helix-term/src/keymap.rs index 35b83b1a5..674900035 100644 --- a/helix-term/src/keymap.rs +++ b/helix-term/src/keymap.rs @@ -90,7 +90,6 @@ use std::collections::HashMap; // #[cfg(feature = "term")] pub use crossterm::event::{KeyCode, KeyEvent as Key, KeyModifiers as Modifiers}; -// TODO: could be trie based pub type Keymap = HashMap; pub type Keymaps = HashMap; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 31a19649a..eb951a26b 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -124,6 +124,9 @@ impl EditorView { use helix_core::graphemes::{grapheme_width, RopeGraphemes}; + // TODO: scope matching: biggest union match? [string] & [html, string], [string, html] & [ string, html] + // can do this by sorting our theme matches based on array len (longest first) then stopping at the + // first rule that matches (rule.all(|scope| scopes.contains(scope))) let style = match spans.first() { Some(span) => theme.get(theme.scopes()[span.0].as_str()), None => Style::default().fg(Color::Rgb(164, 160, 232)), // lavender @@ -138,8 +141,6 @@ impl EditorView { // iterate over range char by char for grapheme in RopeGraphemes::new(text) { - // TODO: track current char_index - if grapheme == "\n" { visual_x = 0; line += 1; @@ -433,9 +434,7 @@ impl Component for EditorView { Event::Key(event) => { let view = cx.editor.view_mut(); - // TODO: sequences (`gg`) let mode = view.doc.mode(); - // TODO: handle count other than 1 let mut cxt = commands::Context { executor: cx.executor, editor: &mut cx.editor, @@ -479,8 +478,6 @@ impl Component for EditorView { if let Some(command) = self.keymap[&mode].get(&event) { command(&mut cxt); - - // TODO: simplistic ensure cursor in view for now } } } @@ -503,7 +500,6 @@ impl Component for EditorView { fn render(&self, mut area: Rect, surface: &mut Surface, cx: &mut Context) { for (view, is_focused) in cx.editor.tree.views() { - // TODO: use parent area self.render_view(view, view.area, surface, &cx.editor.theme, is_focused); } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 033a35930..82258bde9 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -255,8 +255,6 @@ impl Document { return; } - // TODO: change -> change -> undo -> change -> change fails, probably old_state needs reset - let new_changeset = ChangeSet::new(self.text()); let changes = std::mem::replace(&mut self.changes, new_changeset); // Instead of doing this messy merge we could always commit, and based on transaction @@ -319,7 +317,7 @@ impl Document { // self.state.doc.slice // } - // TODO: transact(Fn) ? + // transact(Fn) ? // -- LSP methods diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 5b269a9a7..b406b7564 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -13,15 +13,12 @@ use tui::layout::Rect; pub const PADDING: usize = 5; -// TODO: view should be View { doc: Document(state, history,..) } -// since we can have multiple views into the same file pub struct View { pub id: Key, pub doc: Document, pub first_line: usize, pub area: Rect, } -// TODO: popups should be a thing on the view with a rect + text impl View { pub fn new(doc: Document) -> Result { @@ -54,9 +51,9 @@ impl View { /// Calculates the last visible line on screen #[inline] pub fn last_line(&self) -> usize { - let viewport = Rect::new(6, 0, self.area.width, self.area.height.saturating_sub(1)); // - 1 for statusline + let height = self.area.height.saturating_sub(1); // - 1 for statusline std::cmp::min( - self.first_line + (viewport.height as usize), + self.first_line + height as usize, self.doc.text().len_lines() - 1, ) } @@ -67,7 +64,7 @@ impl View { pub fn screen_coords_at_pos(&self, text: RopeSlice, pos: usize) -> Option { let line = text.char_to_line(pos); - if line < self.first_line as usize || line > self.last_line() { + if line < self.first_line || line > self.last_line() { // Line is not visible on screen return None; }