From d9677671267b55680d2b37fc5e0948a44a517560 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Tue, 20 Jul 2021 21:15:43 -0400 Subject: [PATCH 01/12] Implement `margin` calculation for uncommenting --- helix-core/src/comment.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 5d564055dbff..2072c77562c9 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -53,6 +53,26 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st changes.reserve((end - start).saturating_sub(skipped.len())); + // determine margin of 0 or 1 for uncommenting; if any comment token is not followed by a space, + // a margin of 0 is used for all lines. + let mut margin = 1; + if commented { + for line in lines.clone() { + if skipped.contains(&line) { + continue; + } + + let pos = text.line_to_char(line) + min; + + if let Some(c) = text.get_char(pos + token.len()) { + if c != ' ' { + margin = 0; + break; + } + } + } + } + for line in lines { if skipped.contains(&line) { continue; @@ -65,7 +85,6 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st changes.push((pos, pos, Some(comment.clone()))) } else { // uncomment line - let margin = 1; // TODO: margin is hardcoded 1 but could easily be 0 changes.push((pos, pos + token.len() + margin, None)) } } From 6055e93c7be12cc9051fbbc7606a3a1dc5d4e297 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Tue, 20 Jul 2021 23:12:36 -0400 Subject: [PATCH 02/12] Move `margin` calculation to `find_line_comment` --- helix-core/src/comment.rs | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 2072c77562c9..b2c1b20ab143 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -8,10 +8,11 @@ fn find_line_comment( token: &str, text: RopeSlice, lines: Range, -) -> (bool, Vec, usize) { +) -> (bool, Vec, usize, usize) { let mut commented = true; let mut skipped = Vec::new(); let mut min = usize::MAX; // minimum col for find_first_non_whitespace_char + let mut margin = 1; for line in lines { let line_slice = text.line(line); if let Some(pos) = find_first_non_whitespace_char(line_slice) { @@ -29,12 +30,18 @@ fn find_line_comment( // considered uncommented. commented = false; } + + // determine margin of 0 or 1 for uncommenting; if any comment token is not followed by a space, + // a margin of 0 is used for all lines. + if matches!(line_slice.get_char(pos + token.len()), Some(c) if c != ' ') { + margin = 0; + } } else { // blank line skipped.push(line); } } - (commented, skipped, min) + (commented, skipped, min, margin) } #[must_use] @@ -49,30 +56,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st let start = text.char_to_line(selection.from()); let end = text.char_to_line(selection.to()); let lines = start..end + 1; - let (commented, skipped, min) = find_line_comment(&token, text, lines.clone()); + let (commented, skipped, min, margin) = find_line_comment(&token, text, lines.clone()); changes.reserve((end - start).saturating_sub(skipped.len())); - // determine margin of 0 or 1 for uncommenting; if any comment token is not followed by a space, - // a margin of 0 is used for all lines. - let mut margin = 1; - if commented { - for line in lines.clone() { - if skipped.contains(&line) { - continue; - } - - let pos = text.line_to_char(line) + min; - - if let Some(c) = text.get_char(pos + token.len()) { - if c != ' ' { - margin = 0; - break; - } - } - } - } - for line in lines { if skipped.contains(&line) { continue; From a7dd731e92f71ba4f2842105d9307d6258a83d08 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Tue, 20 Jul 2021 23:33:59 -0400 Subject: [PATCH 03/12] Fix comment bug with multiple selections on a line --- helix-core/src/comment.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index b2c1b20ab143..9dc9fc6e9ec4 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -49,6 +49,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st let text = doc.slice(..); let mut changes: Vec = Vec::new(); + // keep track of which lines have been affected, so that multiple + // selections on one line don't each try to change it. + let mut affected_lines: Vec = Vec::new(); + let token = token.unwrap_or("//"); let comment = Tendril::from(format!("{} ", token)); @@ -61,9 +65,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st changes.reserve((end - start).saturating_sub(skipped.len())); for line in lines { - if skipped.contains(&line) { + if skipped.contains(&line) || affected_lines.contains(&line) { continue; } + affected_lines.push(line); let pos = text.line_to_char(line) + min; From 4a2ae1d2f195794d36e34883dc1ddf8c74615b3f Mon Sep 17 00:00:00 2001 From: Omnikar Date: Wed, 21 Jul 2021 02:43:44 -0400 Subject: [PATCH 04/12] Fix `find_line_comment` test for new return type --- helix-core/src/comment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 9dc9fc6e9ec4..c54fa8b798eb 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -102,8 +102,8 @@ mod test { let text = state.doc.slice(..); let res = find_line_comment("//", text, 0..3); - // (commented = true, skipped = [line 1], min = col 2) - assert_eq!(res, (false, vec![1], 2)); + // (commented = true, skipped = [line 1], min = col 2, margin = 1) + assert_eq!(res, (false, vec![1], 2, 1)); // comment let transaction = toggle_line_comments(&state.doc, &state.selection, None); From 96b62332266785dd0f818b4d5639aad07ab0c8e6 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Wed, 21 Jul 2021 12:27:25 -0400 Subject: [PATCH 05/12] Generate a single vec of lines for comment toggle `toggle_line_comments` collects the lines covered by all selections into a `Vec`, skipping duplicates. `find_line_comment` now returns the lines to operate on, instead of returning the lines to skip. --- helix-core/src/comment.rs | 54 ++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index c54fa8b798eb..d89b87272346 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -1,16 +1,15 @@ use crate::{ find_first_non_whitespace_char, Change, Rope, RopeSlice, Selection, Tendril, Transaction, }; -use core::ops::Range; use std::borrow::Cow; fn find_line_comment( token: &str, text: RopeSlice, - lines: Range, + lines: impl IntoIterator, ) -> (bool, Vec, usize, usize) { let mut commented = true; - let mut skipped = Vec::new(); + let mut to_change = Vec::new(); let mut min = usize::MAX; // minimum col for find_first_non_whitespace_char let mut margin = 1; for line in lines { @@ -36,12 +35,12 @@ fn find_line_comment( if matches!(line_slice.get_char(pos + token.len()), Some(c) if c != ' ') { margin = 0; } - } else { - // blank line - skipped.push(line); + + // blank lines don't get pushed. + to_change.push(line); } } - (commented, skipped, min, margin) + (commented, to_change, min, margin) } #[must_use] @@ -49,38 +48,35 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st let text = doc.slice(..); let mut changes: Vec = Vec::new(); - // keep track of which lines have been affected, so that multiple - // selections on one line don't each try to change it. - let mut affected_lines: Vec = Vec::new(); - let token = token.unwrap_or("//"); let comment = Tendril::from(format!("{} ", token)); + let mut lines: Vec = Vec::new(); + + let mut min_next_line = 0; for selection in selection { - let start = text.char_to_line(selection.from()); - let end = text.char_to_line(selection.to()); - let lines = start..end + 1; - let (commented, skipped, min, margin) = find_line_comment(&token, text, lines.clone()); + let start = text.char_to_line(selection.from()).max(min_next_line); + let end = text.char_to_line(selection.to()) + 1; + lines.extend(start..end); + min_next_line = end + 1; + } - changes.reserve((end - start).saturating_sub(skipped.len())); + changes.reserve(lines.len()); - for line in lines { - if skipped.contains(&line) || affected_lines.contains(&line) { - continue; - } - affected_lines.push(line); + let (commented, to_change, min, margin) = find_line_comment(&token, text, lines); - let pos = text.line_to_char(line) + min; + for line in to_change { + let pos = text.line_to_char(line) + min; - if !commented { - // comment line - changes.push((pos, pos, Some(comment.clone()))) - } else { - // uncomment line - changes.push((pos, pos + token.len() + margin, None)) - } + if !commented { + // comment line + changes.push((pos, pos, Some(comment.clone()))); + } else { + // uncomment line + changes.push((pos, pos + token.len() + margin, None)); } } + Transaction::change(doc, changes.into_iter()) } From 1032a4be3cb7eaefb95c84855c99618204116b0c Mon Sep 17 00:00:00 2001 From: Omnikar Date: Wed, 21 Jul 2021 12:42:09 -0400 Subject: [PATCH 06/12] Fix test for `find_line_comment` --- helix-core/src/comment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index d89b87272346..f8507e92c92e 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -98,8 +98,8 @@ mod test { let text = state.doc.slice(..); let res = find_line_comment("//", text, 0..3); - // (commented = true, skipped = [line 1], min = col 2, margin = 1) - assert_eq!(res, (false, vec![1], 2, 1)); + // (commented = true, to_change = [line 0, line 2], min = col 2, margin = 1) + assert_eq!(res, (false, vec![0, 2], 2, 1)); // comment let transaction = toggle_line_comments(&state.doc, &state.selection, None); From cc9344acbde30bfe74425fa36366f1dbaa41e00d Mon Sep 17 00:00:00 2001 From: Omnikar Date: Wed, 21 Jul 2021 14:17:54 -0400 Subject: [PATCH 07/12] Reserve length of `to_change` instead of `lines` The length of `lines` includes blank lines which will be skipped, and as such do not need space for a change reserved for them. `to_change` includes only the lines which will be changed. --- helix-core/src/comment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index f8507e92c92e..8e678541489a 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -61,10 +61,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st min_next_line = end + 1; } - changes.reserve(lines.len()); - let (commented, to_change, min, margin) = find_line_comment(&token, text, lines); + changes.reserve(to_change.len()); + for line in to_change { let pos = text.line_to_char(line) + min; From ea7eddb4c9eaa862b724e5b9deb2a4913ca41ffe Mon Sep 17 00:00:00 2001 From: Omnikar Date: Thu, 22 Jul 2021 02:56:25 -0400 Subject: [PATCH 08/12] Use `token.chars().count()` for token char length --- helix-core/src/comment.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 8e678541489a..08b204dc39de 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -12,6 +12,7 @@ fn find_line_comment( let mut to_change = Vec::new(); let mut min = usize::MAX; // minimum col for find_first_non_whitespace_char let mut margin = 1; + let token_len = token.chars().count(); for line in lines { let line_slice = text.line(line); if let Some(pos) = find_first_non_whitespace_char(line_slice) { @@ -32,7 +33,7 @@ fn find_line_comment( // determine margin of 0 or 1 for uncommenting; if any comment token is not followed by a space, // a margin of 0 is used for all lines. - if matches!(line_slice.get_char(pos + token.len()), Some(c) if c != ' ') { + if matches!(line_slice.get_char(pos + token_len), Some(c) if c != ' ') { margin = 0; } From 57c96a06bf59cb9f5ada9bf8af7b379f2924e98e Mon Sep 17 00:00:00 2001 From: Omnikar Date: Thu, 22 Jul 2021 02:59:08 -0400 Subject: [PATCH 09/12] Create `changes` with capacity instead of reserving --- helix-core/src/comment.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 08b204dc39de..349607455bfe 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -47,7 +47,6 @@ fn find_line_comment( #[must_use] pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&str>) -> Transaction { let text = doc.slice(..); - let mut changes: Vec = Vec::new(); let token = token.unwrap_or("//"); let comment = Tendril::from(format!("{} ", token)); @@ -64,7 +63,7 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st let (commented, to_change, min, margin) = find_line_comment(&token, text, lines); - changes.reserve(to_change.len()); + let mut changes: Vec = Vec::with_capacity(to_change.len()); for line in to_change { let pos = text.line_to_char(line) + min; From 3c03cfa06e3b17b7282a25b15d0eafd5fafa97c7 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Fri, 23 Jul 2021 01:25:19 -0400 Subject: [PATCH 10/12] Remove unnecessary clones in `test_find_line_comment` --- helix-core/src/comment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 349607455bfe..462e3ec6026a 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -104,14 +104,14 @@ mod test { // comment let transaction = toggle_line_comments(&state.doc, &state.selection, None); transaction.apply(&mut state.doc); - state.selection = state.selection.clone().map(transaction.changes()); + state.selection = state.selection.map(transaction.changes()); assert_eq!(state.doc, " // 1\n\n // 2\n // 3"); // uncomment let transaction = toggle_line_comments(&state.doc, &state.selection, None); transaction.apply(&mut state.doc); - state.selection = state.selection.clone().map(transaction.changes()); + state.selection = state.selection.map(transaction.changes()); assert_eq!(state.doc, " 1\n\n 2\n 3"); // TODO: account for no margin after comment From 4e7cad2f8a4b8e9b9b5e78f88abee709b42ab71a Mon Sep 17 00:00:00 2001 From: Omnikar Date: Fri, 23 Jul 2021 02:48:53 -0400 Subject: [PATCH 11/12] Add test case for 0 margin comments --- helix-core/src/comment.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 462e3ec6026a..1d636f30e443 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -114,7 +114,16 @@ mod test { state.selection = state.selection.map(transaction.changes()); assert_eq!(state.doc, " 1\n\n 2\n 3"); - // TODO: account for no margin after comment + // 0 margin comments + state.doc = Rope::from(" //1\n\n //2\n //3"); + // reset the selection. + state.selection = Selection::single(0, state.doc.len_chars() - 1); + + let transaction = toggle_line_comments(&state.doc, &state.selection, None); + transaction.apply(&mut state.doc); + state.selection = state.selection.map(transaction.changes()); + assert_eq!(state.doc, " 1\n\n 2\n 3"); + // TODO: account for uncommenting with uneven comment indentation } } From aaabf035de10cb7ac6bf7f825ff4b23992180606 Mon Sep 17 00:00:00 2001 From: Omnikar Date: Fri, 23 Jul 2021 14:31:04 -0400 Subject: [PATCH 12/12] Add comments explaining `find_line_comment` --- helix-core/src/comment.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index 1d636f30e443..fadd88e0592f 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -3,6 +3,15 @@ use crate::{ }; use std::borrow::Cow; +/// Given text, a comment token, and a set of line indices, returns the following: +/// - Whether the given lines should be considered commented +/// - If any of the lines are uncommented, all lines are considered as such. +/// - The lines to change for toggling comments +/// - This is all provided lines excluding blanks lines. +/// - The column of the comment tokens +/// - Column of existing tokens, if the lines are commented; column to place tokens at otherwise. +/// - The margin to the right of the comment tokens +/// - Defaults to `1`. If any existing comment token is not followed by a space, changes to `0`. fn find_line_comment( token: &str, text: RopeSlice,