fix: panic as no ranges were inserted

announce.md

@@ -34,3 +34,4 @@ It works in _all_ languages that properly inject comment tokens. For instance, J
 
 Closes https://github.com/helix-editor/helix/issues/7364
 Closes https://github.com/helix-editor/helix/issues/11647
+Related to to https://github.com/helix-editor/helix/issues/9425

helix-core/src/comment.rs

@@ -229,14 +229,14 @@ pub fn find_block_comments(
 pub fn create_block_comment_transaction(
     _doc: &Rope,
     ranges: &[Range],
-    commented: bool,
+    was_commented: bool,
     comment_changes: Vec<CommentChange>,
 ) -> (Vec<Change>, SmallVec<[Range; 1]>) {
     let mut changes: Vec<Change> = Vec::with_capacity(ranges.len() * 2);
     let mut ranges: SmallVec<[Range; 1]> = SmallVec::with_capacity(ranges.len());
     let mut offs = 0;
     for change in comment_changes {
-        if commented {
+        if was_commented {
             if let CommentChange::Commented {
                 range,
                 start_pos,
@@ -248,16 +248,11 @@ pub fn create_block_comment_transaction(
             } = change
             {
                 let from = range.from();
-                changes.push((
+                let keep_from = from + start_pos + start_token.len() + start_margin as usize;
-                    from + start_pos,
+                changes.push((from + start_pos, keep_from, None));
-                    from + start_pos + start_token.len() + start_margin as usize,
+                let keep_until = from + end_pos - end_token.len() - end_margin as usize + 1;
-                    None,
+                changes.push((keep_until, from + end_pos + 1, None));
-                ));
+                ranges.push(Range::new(keep_from, keep_until).with_direction(range.direction()));
-                changes.push((
-                    from + end_pos - end_token.len() - end_margin as usize + 1,
-                    from + end_pos + 1,
-                    None,
-                ));
             }
         } else {
             // uncommented so manually map ranges through changes
@@ -303,13 +298,28 @@ pub fn toggle_block_comments(
     ranges: &Vec<Range>,
     tokens: &[BlockCommentToken],
     selections: &mut SmallVec<[Range; 1]>,
-    added_chars: &mut usize,
+    added_chars: &mut isize,
 ) -> Vec<Change> {
     let text = doc.slice(..);
-    let (commented, comment_changes) = find_block_comments(tokens, text, ranges);
+    let (was_commented, comment_changes) = find_block_comments(tokens, text, ranges);
     let (changes, new_ranges) =
-        create_block_comment_transaction(doc, ranges, commented, comment_changes);
+        create_block_comment_transaction(doc, ranges, was_commented, comment_changes);
-    if commented {
+
+    if was_commented {
+        for (i, range) in new_ranges.iter().enumerate() {
+            // every 2 elements (from, to) in `changes` corresponds
+            // the `from` - `to` represents the range of text that will be deleted.
+            // to 1 element in `new_ranges`
+            let (from, to, _) = changes[i * 2];
+            let (from2, to2, _) = changes[i * 2 + 1];
+            *added_chars -= to2 as isize - from2 as isize;
+            selections.push(Range::new(
+                (range.anchor as isize + *added_chars).try_into().unwrap(),
+                (range.head as isize + *added_chars).try_into().unwrap(),
+            ));
+            *added_chars -= to as isize - from as isize;
+        }
+
         changes
     } else {
         // when we add comment tokens, we want to extend our selection to
@@ -323,9 +333,12 @@ pub fn toggle_block_comments(
             // Will not underflow because the new range must always be
             // at least the same size as the old range, since we're
             // adding comment token characters, never removing.
-            let range = Range::new(range.from() + *added_chars, range.to() + *added_chars);
+            let range = Range::new(
+                range.anchor + *added_chars as usize,
+                range.head + *added_chars as usize,
+            );
             selections.push(range);
-            *added_chars += range.len() - old_range.len();
+            *added_chars += range.len() as isize - old_range.len() as isize;
         }
 
         changes

helix-term/src/commands.rs

@@ -5124,6 +5124,7 @@ fn toggle_comments_impl<'a>(
         .and_then(|lc| lc.block_comment_tokens.as_ref())
         .map(|tc| &tc[..]);
 
+    // TODO: figure out how to avoid this clone
     let syntax = doc.syntax().cloned();
 
     let transaction = comment_transaction(
@@ -5307,8 +5308,6 @@ fn toggle_block_comments(cx: &mut Context) {
         cx,
         Box::new(
             |doc_line_token, doc_block_tokens, rope, selection, mut get_injected_tokens| {
-                // when we add comment tokens, we want to extend our selection to
-                // also include the added tokens.
                 let mut selections = SmallVec::new();
                 let mut added_chars = 0;
                 let transaction = Transaction::change(

helix-term/tests/test/commands.rs

@@ -742,7 +742,6 @@ async fn test_injected_comment_tokens_simple() -> anyhow::Result<()> {
 /// Selections in different regions
 #[tokio::test(flavor = "multi_thread")]
 async fn test_injected_comment_tokens_multiple_selections() -> anyhow::Result<()> {
-    return Ok(());
     // Comments two different injection layers with different comments
     test((
         indoc! {r#"\
@@ -860,7 +859,6 @@ async fn test_injected_comment_tokens_multiple_selections() -> anyhow::Result<()
 /// from the injection with the bigger scope
 #[tokio::test(flavor = "multi_thread")]
 async fn test_injected_comment_tokens_selection_across_different_layers() -> anyhow::Result<()> {
-    return Ok(());
     test((
         indoc! {r#"\
             <p>Comment tog#[|gle on this line should use the HTML comment token(s).</p>