Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continue line comments #10996

Merged
merged 65 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
3844eb7
Add `continue_comment` function
seanchen1991 Oct 20, 2023
3d31e66
Add test for `get_comment_token` fn
seanchen1991 Oct 20, 2023
129ecae
Fix incorrect assertion
seanchen1991 Oct 20, 2023
2449622
Wire up continue comment functionality
seanchen1991 Oct 20, 2023
036e2bf
Add `comment-tokens` field to languages.toml
seanchen1991 Oct 20, 2023
adda266
Add additional comment tokens for Rust
seanchen1991 Oct 20, 2023
be9224e
Match all comment-tokens fields with comment-token fields
seanchen1991 Oct 20, 2023
e77d15d
Refactor continue comment function to also return position of comment…
seanchen1991 Oct 20, 2023
751d15d
Update calls to find_first_non_whitespace_char
seanchen1991 Oct 22, 2023
4315906
Continue single comments with indentation
seanchen1991 Oct 22, 2023
0d224e5
Implement `count_whitespace_after` fn
seanchen1991 Oct 24, 2023
ee9c015
Get tests for continue comment logic passing
seanchen1991 Oct 24, 2023
e44d55a
Don't count newlines when counting whitespace
seanchen1991 Oct 24, 2023
d816982
Rename a variable
seanchen1991 Oct 24, 2023
e6a492b
Add `continue-comment` config parameter
seanchen1991 Oct 25, 2023
c3a6190
Contiue comments in insert mode based on config
seanchen1991 Oct 25, 2023
d25c959
Merge branch 'helix-editor:master' into feature/continue-comment
seanchen1991 Oct 26, 2023
cbd05dc
Use pre-existing indentation functions to indent comment lines
seanchen1991 Oct 26, 2023
d73cd87
Merge branch 'feature/continue-comment' of https://github.com/seanche…
seanchen1991 Oct 26, 2023
e170d40
Clean up for PR submission
seanchen1991 Oct 30, 2023
d52dae9
Fix merge conflict in languages.toml
seanchen1991 Oct 30, 2023
b03f870
Remove changes in indent.rs
seanchen1991 Oct 30, 2023
57261e5
Remove incorrect unit test
seanchen1991 Nov 1, 2023
e51c288
Merge branch 'feature/continue-comment' of github.com:seanchen1991/he…
TornaxO7 Jun 19, 2024
8fa3cc9
add `has_line_comment` method and fix `starts_with` for RopeSlice
TornaxO7 Jun 20, 2024
c70acf7
make clippy happy
TornaxO7 Jun 20, 2024
345ee7c
Merge branch 'master' of github.com:helix-editor/helix into continue-…
TornaxO7 Jul 15, 2024
38bc94b
continue-line-comment: add explanation
TornaxO7 Jul 20, 2024
fa50e51
single-line-comment: improve performance
TornaxO7 Jul 20, 2024
e318a30
comment: Add invariant to comment-tokens in config
TornaxO7 Aug 6, 2024
76a42d9
continue-comment: improve continue comment when opening new lines wit…
TornaxO7 Aug 6, 2024
b5f3dd4
continue-comment: improve continue comment when writing new line
TornaxO7 Aug 6, 2024
22734da
continue-comment: add hepler function to reduce LOC and improve reada…
TornaxO7 Aug 6, 2024
de5ebe6
comment.rs: revamp tests
TornaxO7 Aug 7, 2024
dc6411d
comments: remove some if-guards
TornaxO7 Aug 7, 2024
ca6ee48
comments: add special case if the text is empty it won't be considere…
TornaxO7 Aug 7, 2024
1a95d40
continue-comment: fix token finding
TornaxO7 Aug 7, 2024
dc92500
make clippy happy
TornaxO7 Aug 7, 2024
1e18f1b
config: fix tests
TornaxO7 Aug 7, 2024
f68cf64
continue-comments: add some docs
TornaxO7 Aug 7, 2024
4cc5148
continue-comment: remove unwraps
TornaxO7 Aug 7, 2024
830f148
Merge branch 'master' of github.com:helix-editor/helix into continue-…
TornaxO7 Aug 9, 2024
da2a444
continue-comment: removing indent_heuristic function
TornaxO7 Aug 9, 2024
1eb963d
Revert "continue-comment: removing indent_heuristic function"
TornaxO7 Aug 9, 2024
9c09141
Reapply "continue-comment: removing indent_heuristic function"
TornaxO7 Aug 9, 2024
d4b1dfb
Revert "continue-comment: removing indent_heuristic function"
TornaxO7 Aug 9, 2024
f53ec12
continue-comment: set default value for continue comments to false
TornaxO7 Aug 9, 2024
372550d
continue-comment: enable by default and use hybrid indentation
TornaxO7 Aug 9, 2024
83093f8
continue-comments: fix indent-logic if continue-comments wants to pla…
TornaxO7 Aug 10, 2024
b842fdb
continue-comment: improve code readability
TornaxO7 Aug 10, 2024
b5003e9
continue-comment: add some comments and remove unused code
TornaxO7 Aug 10, 2024
835b4ba
revert unecessary variable renaming
TornaxO7 Oct 5, 2024
310b6dd
remove config option for continue-comments
TornaxO7 Oct 5, 2024
45f230e
continue-comments: update parameter types of `get_comment_token`
TornaxO7 Oct 6, 2024
ce5d174
continue-comments: restructure `get_comment_token`
TornaxO7 Oct 6, 2024
170c8c2
continue-comments: simplify code
TornaxO7 Oct 6, 2024
843b9b4
style-nit: reduce nested blocks
TornaxO7 Oct 6, 2024
a5d7f52
continue-comments: improve jumping behaviour
TornaxO7 Oct 6, 2024
d7ca450
continue-comments: improve local_offs variable calculation
TornaxO7 Oct 6, 2024
e9ef838
fix warnings
TornaxO7 Oct 6, 2024
aca48e8
improve code format
TornaxO7 Oct 7, 2024
b7f9780
continue-comments: Fix panic case and add test
TornaxO7 Oct 7, 2024
5b9613c
continue-comments: adding test
TornaxO7 Oct 7, 2024
898f231
improve code structure
TornaxO7 Oct 7, 2024
12a8b3e
comment: reverting small bug fix
TornaxO7 Oct 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 123 additions & 46 deletions helix-core/src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ use crate::{
use helix_stdx::rope::RopeSliceExt;
use std::borrow::Cow;

pub const DEFAULT_COMMENT_TOKEN: &str = "//";

/// Returns the longest matching comment token of the given line (if it exists).
pub fn get_comment_token<'a, S: AsRef<str>>(
text: RopeSlice,
tokens: &'a [S],
line_num: usize,
) -> Option<&'a str> {
let line = text.line(line_num);
let start = line.first_non_whitespace_char()?;

tokens
.iter()
.map(AsRef::as_ref)
.filter(|token| line.slice(start..).starts_with(token))
.max_by_key(|token| token.len())
}

/// 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.
Expand All @@ -28,21 +46,20 @@ fn find_line_comment(
let mut min = usize::MAX; // minimum col for 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) = line_slice.first_non_whitespace_char() {
let len = line_slice.len_chars();

if pos < min {
min = pos;
}
min = std::cmp::min(min, pos);

// line can be shorter than pos + token len
let fragment = Cow::from(line_slice.slice(pos..std::cmp::min(pos + token.len(), len)));

// as soon as one of the non-blank lines doesn't have a comment, the whole block is
// considered uncommented.
if fragment != token {
// as soon as one of the non-blank lines doesn't have a comment, the whole block is
// considered uncommented.
commented = false;
}

Expand All @@ -56,14 +73,15 @@ fn find_line_comment(
to_change.push(line);
}
}

(commented, to_change, min, margin)
}

#[must_use]
pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&str>) -> Transaction {
let text = doc.slice(..);

let token = token.unwrap_or("//");
let token = token.unwrap_or(DEFAULT_COMMENT_TOKEN);
let comment = Tendril::from(format!("{} ", token));

let mut lines: Vec<usize> = Vec::with_capacity(selection.len());
Expand Down Expand Up @@ -317,56 +335,87 @@ pub fn split_lines_of_selection(text: RopeSlice, selection: &Selection) -> Selec
mod test {
use super::*;

#[test]
pascalkuthe marked this conversation as resolved.
Show resolved Hide resolved
fn test_find_line_comment() {
// four lines, two space indented, except for line 1 which is blank.
let mut doc = Rope::from(" 1\n\n 2\n 3");
// select whole document
let mut selection = Selection::single(0, doc.len_chars() - 1);
mod find_line_comment {
use super::*;

let text = doc.slice(..);
#[test]
fn not_commented() {
// four lines, two space indented, except for line 1 which is blank.
let doc = Rope::from(" 1\n\n 2\n 3");

let res = find_line_comment("//", text, 0..3);
// (commented = true, to_change = [line 0, line 2], min = col 2, margin = 0)
assert_eq!(res, (false, vec![0, 2], 2, 0));
let text = doc.slice(..);

// comment
let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());
let res = find_line_comment("//", text, 0..3);
// (commented = false, to_change = [line 0, line 2], min = col 2, margin = 0)
assert_eq!(res, (false, vec![0, 2], 2, 0));
}

assert_eq!(doc, " // 1\n\n // 2\n // 3");
#[test]
fn is_commented() {
// three lines where the second line is empty.
let doc = Rope::from("// hello\n\n// there");

// uncomment
let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());
assert_eq!(doc, " 1\n\n 2\n 3");
assert!(selection.len() == 1); // to ignore the selection unused warning
let res = find_line_comment("//", doc.slice(..), 0..3);

// 0 margin comments
doc = Rope::from(" //1\n\n //2\n //3");
// reset the selection.
selection = Selection::single(0, doc.len_chars() - 1);
// (commented = true, to_change = [line 0, line 2], min = col 0, margin = 1)
assert_eq!(res, (true, vec![0, 2], 0, 1));
}
}

let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());
assert_eq!(doc, " 1\n\n 2\n 3");
assert!(selection.len() == 1); // to ignore the selection unused warning
// TODO: account for uncommenting with uneven comment indentation
mod toggle_line_comment {
use super::*;

// 0 margin comments, with no space
doc = Rope::from("//");
// reset the selection.
selection = Selection::single(0, doc.len_chars() - 1);
#[test]
fn comment() {
// four lines, two space indented, except for line 1 which is blank.
let mut doc = Rope::from(" 1\n\n 2\n 3");
// select whole document
let selection = Selection::single(0, doc.len_chars() - 1);

let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());
assert_eq!(doc, "");
assert!(selection.len() == 1); // to ignore the selection unused warning
let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);

assert_eq!(doc, " // 1\n\n // 2\n // 3");
}

// TODO: account for uncommenting with uneven comment indentation
#[test]
fn uncomment() {
let mut doc = Rope::from(" // 1\n\n // 2\n // 3");
let mut selection = Selection::single(0, doc.len_chars() - 1);

let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());

assert_eq!(doc, " 1\n\n 2\n 3");
assert!(selection.len() == 1); // to ignore the selection unused warning
}

#[test]
fn uncomment_0_margin_comments() {
let mut doc = Rope::from(" //1\n\n //2\n //3");
let mut selection = Selection::single(0, doc.len_chars() - 1);

let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());

assert_eq!(doc, " 1\n\n 2\n 3");
assert!(selection.len() == 1); // to ignore the selection unused warning
}

#[test]
fn uncomment_0_margin_comments_with_no_space() {
let mut doc = Rope::from("//");
let mut selection = Selection::single(0, doc.len_chars() - 1);

let transaction = toggle_line_comments(&doc, &selection, None);
transaction.apply(&mut doc);
selection = selection.map(transaction.changes());
assert_eq!(doc, "");
assert!(selection.len() == 1); // to ignore the selection unused warning
}
}

#[test]
Expand Down Expand Up @@ -413,4 +462,32 @@ mod test {
transaction.apply(&mut doc);
assert_eq!(doc, "");
}

/// Test, if `get_comment_tokens` works, even if the content of the file includes chars, whose
/// byte size unequal the amount of chars
#[test]
fn test_get_comment_with_char_boundaries() {
let rope = Rope::from("··");
let tokens = ["//", "///"];

assert_eq!(
super::get_comment_token(rope.slice(..), tokens.as_slice(), 0),
None
);
}

/// Test for `get_comment_token`.
///
/// Assuming the comment tokens are stored as `["///", "//"]`, `get_comment_token` should still
/// return `///` instead of `//` if the user is in a doc-comment section.
#[test]
fn test_use_longest_comment() {
let text = Rope::from(" /// amogus");
let tokens = ["///", "//"];

assert_eq!(
super::get_comment_token(text.slice(..), tokens.as_slice(), 0),
Some("///")
);
}
}
2 changes: 1 addition & 1 deletion helix-stdx/src/rope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'a> RopeSliceExt<'a> for RopeSlice<'a> {
if len < text.len() {
return false;
}
self.get_byte_slice(..len - text.len())
self.get_byte_slice(..text.len())
TornaxO7 marked this conversation as resolved.
Show resolved Hide resolved
.map_or(false, |start| start == text)
}

Expand Down
97 changes: 69 additions & 28 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use helix_core::{
encoding, find_workspace,
graphemes::{self, next_grapheme_boundary, RevRopeGraphemes},
history::UndoKind,
increment, indent,
indent::IndentStyle,
increment,
indent::{self, IndentStyle},
line_ending::{get_line_ending_of_str, line_end_char_index},
match_brackets,
movement::{self, move_vertically_visual, Direction},
Expand Down Expand Up @@ -3424,31 +3424,51 @@ fn open(cx: &mut Context, open: Open) {
)
};

let indent = indent::indent_for_newline(
doc.language_config(),
doc.syntax(),
&doc.config.load().indent_heuristic,
&doc.indent_style,
doc.tab_width(),
text,
line_num,
line_end_index,
cursor_line,
);
let continue_comment_token = doc
.language_config()
.and_then(|config| config.comment_tokens.as_ref())
.and_then(|tokens| comment::get_comment_token(text, tokens, cursor_line));

let line = text.line(cursor_line);
let indent = match line.first_non_whitespace_char() {
Some(pos) if continue_comment_token.is_some() => line.slice(..pos).to_string(),
_ => indent::indent_for_newline(
doc.language_config(),
doc.syntax(),
&doc.config.load().indent_heuristic,
&doc.indent_style,
doc.tab_width(),
text,
line_num,
line_end_index,
cursor_line,
),
};

let indent_len = indent.len();
let mut text = String::with_capacity(1 + indent_len);
text.push_str(doc.line_ending.as_str());
text.push_str(&indent);

if let Some(token) = continue_comment_token {
text.push_str(token);
text.push(' ');
}

let text = text.repeat(count);

// calculate new selection ranges
let pos = offs + line_end_index + line_end_offset_width;
let comment_len = continue_comment_token
.map(|token| token.len() + 1) // `+ 1` for the extra space added
.unwrap_or_default();
for i in 0..count {
// pos -> beginning of reference line,
// + (i * (1+indent_len)) -> beginning of i'th line from pos
// + indent_len -> -> indent for i'th line
ranges.push(Range::point(pos + (i * (1 + indent_len)) + indent_len));
// + (i * (1+indent_len + comment_len)) -> beginning of i'th line from pos (possibly including comment token)
// + indent_len + comment_len -> -> indent for i'th line
ranges.push(Range::point(
pos + (i * (1 + indent_len + comment_len)) + indent_len + comment_len,
));
}

offs += text.chars().count();
Expand Down Expand Up @@ -3886,6 +3906,11 @@ pub mod insert {

let mut new_text = String::new();

let continue_comment_token = doc
.language_config()
.and_then(|config| config.comment_tokens.as_ref())
.and_then(|tokens| comment::get_comment_token(text, tokens, current_line));

// If the current line is all whitespace, insert a line ending at the beginning of
// the current line. This makes the current line empty and the new line contain the
// indentation of the old line.
Expand All @@ -3895,17 +3920,22 @@ pub mod insert {

(line_start, line_start, new_text.chars().count())
} else {
let indent = indent::indent_for_newline(
doc.language_config(),
doc.syntax(),
&doc.config.load().indent_heuristic,
&doc.indent_style,
doc.tab_width(),
text,
current_line,
pos,
current_line,
);
let line = text.line(current_line);

let indent = match line.first_non_whitespace_char() {
Some(pos) if continue_comment_token.is_some() => line.slice(..pos).to_string(),
_ => indent::indent_for_newline(
doc.language_config(),
doc.syntax(),
&doc.config.load().indent_heuristic,
&doc.indent_style,
doc.tab_width(),
text,
current_line,
pos,
current_line,
),
};

// If we are between pairs (such as brackets), we want to
// insert an additional line which is indented one level
Expand All @@ -3915,19 +3945,30 @@ pub mod insert {
.and_then(|pairs| pairs.get(prev))
.map_or(false, |pair| pair.open == prev && pair.close == curr);

let local_offs = if on_auto_pair {
let local_offs = if let Some(token) = continue_comment_token {
new_text.push_str(doc.line_ending.as_str());
new_text.push_str(&indent);
new_text.push_str(token);
new_text.push(' ');
new_text.chars().count()
} else if on_auto_pair {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a limitation we want to have in thr lognterm but usually autopairs within comments are a bit niche and likely hard to fix until some other changss land so this is probably fine for now.

// line where the cursor will be
let inner_indent = indent.clone() + doc.indent_style.as_str();
new_text.reserve_exact(2 + indent.len() + inner_indent.len());
new_text.push_str(doc.line_ending.as_str());
new_text.push_str(&inner_indent);

// line where the matching pair will be
let local_offs = new_text.chars().count();
new_text.push_str(doc.line_ending.as_str());
new_text.push_str(&indent);

local_offs
} else {
new_text.reserve_exact(1 + indent.len());
new_text.push_str(doc.line_ending.as_str());
new_text.push_str(&indent);

new_text.chars().count()
};

Expand Down