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.

Sign up for GitHub

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

Jump to bottom

Unify usage of indent_style #5918

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Unify usage of indent_style #5918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 32 additions & 7 deletions helix-core/src/indent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ impl IndentStyle {
}
}
}

#[inline]
pub fn indent_width(&self, tab_width: usize) -> usize {
match *self {
IndentStyle::Tabs => tab_width,
IndentStyle::Spaces(width) => width as usize,
}
}
}

/// Attempts to detect the indentation style used in a document.
Expand Down Expand Up @@ -177,7 +185,7 @@ pub fn auto_detect_indent_style(document_text: &Rope) -> Option<IndentStyle> {

/// To determine indentation of a newly inserted line, figure out the indentation at the last col
/// of the previous line.
pub fn indent_level_for_line(line: RopeSlice, tab_width: usize) -> usize {
pub fn indent_level_for_line(line: RopeSlice, tab_width: usize, indent_width: usize) -> usize {
let mut len = 0;
for ch in line.chars() {
match ch {
Expand All @@ -187,7 +195,7 @@ pub fn indent_level_for_line(line: RopeSlice, tab_width: usize) -> usize {
}
}

len / tab_width
len / indent_width
}

/// Computes for node and all ancestors whether they are the first node on their line.
Expand Down Expand Up @@ -466,6 +474,7 @@ fn extend_nodes<'a>(
text: RopeSlice,
line: usize,
tab_width: usize,
indent_width: usize,
) {
let mut stop_extend = false;

Expand All @@ -490,10 +499,12 @@ fn extend_nodes<'a>(
if deepest_preceding.end_position().row == line {
extend_node = true;
} else {
let cursor_indent = indent_level_for_line(text.line(line), tab_width);
let cursor_indent =
indent_level_for_line(text.line(line), tab_width, indent_width);
let node_indent = indent_level_for_line(
text.line(deepest_preceding.start_position().row),
tab_width,
indent_width,
);
if cursor_indent > node_indent {
extend_node = true;
Expand Down Expand Up @@ -562,6 +573,7 @@ pub fn treesitter_indent_for_pos(
syntax: &Syntax,
indent_style: &IndentStyle,
tab_width: usize,
indent_width: usize,
text: RopeSlice,
line: usize,
pos: usize,
Expand Down Expand Up @@ -622,6 +634,7 @@ pub fn treesitter_indent_for_pos(
text,
line,
tab_width,
indent_width,
);
}
let mut first_in_line = get_first_in_line(node, new_line.then_some(byte_pos));
Expand Down Expand Up @@ -709,6 +722,7 @@ pub fn indent_for_newline(
line_before_end_pos: usize,
current_line: usize,
) -> String {
let indent_width = indent_style.indent_width(tab_width);
if let (Some(query), Some(syntax)) = (
language_config.and_then(|config| config.indent_query()),
syntax,
Expand All @@ -718,6 +732,7 @@ pub fn indent_for_newline(
syntax,
indent_style,
tab_width,
indent_width,
text,
line_before,
line_before_end_pos,
Expand All @@ -726,7 +741,7 @@ pub fn indent_for_newline(
return indent;
};
}
let indent_level = indent_level_for_line(text.line(current_line), tab_width);
let indent_level = indent_level_for_line(text.line(current_line), tab_width, indent_width);
indent_style.as_str().repeat(indent_level)
}

Expand Down Expand Up @@ -763,12 +778,22 @@ mod test {
#[test]
fn test_indent_level() {
let tab_width = 4;
let indent_width = 4;
let line = Rope::from(" fn new"); // 8 spaces
assert_eq!(indent_level_for_line(line.slice(..), tab_width), 2);
assert_eq!(
indent_level_for_line(line.slice(..), tab_width, indent_width),
2
);
let line = Rope::from("\t\t\tfn new"); // 3 tabs
assert_eq!(indent_level_for_line(line.slice(..), tab_width), 3);
assert_eq!(
indent_level_for_line(line.slice(..), tab_width, indent_width),
3
);
// mixed indentation
let line = Rope::from("\t \tfn new"); // 1 tab, 4 spaces, tab
assert_eq!(indent_level_for_line(line.slice(..), tab_width), 3);
assert_eq!(
indent_level_for_line(line.slice(..), tab_width, indent_width),
3
);
}
}
6 changes: 4 additions & 2 deletions helix-core/tests/indent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ fn test_treesitter_indent(file_name: &str, lang_scope: &str) {
for i in 0..doc.len_lines() {
let line = text.line(i);
if let Some(pos) = helix_core::find_first_non_whitespace_char(line) {
let tab_and_indent_width: usize = 4;
let suggested_indent = treesitter_indent_for_pos(
indent_query,
&syntax,
&IndentStyle::Spaces(4),
4,
&IndentStyle::Spaces(tab_and_indent_width as u8),
tab_and_indent_width,
tab_and_indent_width,
text,
i,
text.line_to_char(i) + pos,
Expand Down
19 changes: 6 additions & 13 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3365,8 +3365,8 @@ pub mod insert {
let count = cx.count();
let (view, doc) = current_ref!(cx.editor);
let text = doc.text().slice(..);
let indent_unit = doc.indent_style.as_str();
let tab_size = doc.tab_width();
let tab_width = doc.tab_width();
let indent_width = doc.indent_width();
let auto_pairs = doc.auto_pairs(cx.editor);

let transaction =
Expand All @@ -3387,28 +3387,21 @@ pub mod insert {
None,
)
} else {
let unit_len = indent_unit.chars().count();
// NOTE: indent_unit always contains 'only spaces' or 'only tab' according to `IndentStyle` definition.
let unit_size = if indent_unit.starts_with('\t') {
tab_size * unit_len
} else {
unit_len
};
let width: usize = fragment
.chars()
.map(|ch| {
if ch == '\t' {
tab_size
tab_width
} else {
// it can be none if it still meet control characters other than '\t'
// here just set the width to 1 (or some value better?).
ch.width().unwrap_or(1)
}
})
.sum();
let mut drop = width % unit_size; // round down to nearest unit
let mut drop = width % indent_width; // round down to nearest unit
if drop == 0 {
drop = unit_size
drop = indent_width
}; // if it's already at a unit, consume a whole unit
let mut chars = fragment.chars().rev();
let mut start = pos;
Expand Down Expand Up @@ -3950,7 +3943,7 @@ fn unindent(cx: &mut Context) {
let lines = get_lines(doc, view.id);
let mut changes = Vec::with_capacity(lines.len());
let tab_width = doc.tab_width();
let indent_width = count * tab_width;
let indent_width = count * doc.indent_width();

for line_idx in lines {
let line = doc.text().line(line_idx);
Expand Down
7 changes: 6 additions & 1 deletion helix-view/src/document.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,13 +1122,18 @@ impl Document {
self.syntax.as_ref()
}

Copy link
Member

@pascalkuthe pascalkuthe Feb 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

With this config removed tab-width is not updated on :config-reload anymore. I actually think that we should keep this function around and not make IndentStyle a struct at all.

Rather we should add a second function to the document to compute the indent_width and then use that in the right places.

Suggested change
/// Tab size in columns.
pub fn tab_width(&self) -> usize {
self.language_config()
.and_then(|config| config.indent.as_ref())
.map_or(4, |config| config.tab_width) // fallback to 4 columns
}
/// Indentation size in columns
pub fn indent_width(&self) -> usize {
match self.indent_style{
IndentStyle::Tab => self.tab_width(),
IndentStyle::Spaces(width) => width as usize
}
}

/// Tab size in columns.
/// The width that the tab character is rendered at
pub fn tab_width(&self) -> usize {
self.language_config()
.and_then(|config| config.indent.as_ref())
.map_or(4, |config| config.tab_width) // fallback to 4 columns
}

// The width (in spaces) of a level of indentation.
pub fn indent_width(&self) -> usize {
self.indent_style.indent_width(self.tab_width())
}

pub fn changes(&self) -> &ChangeSet {
&self.changes
}
Expand Down