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

Add command to inc/dec number under cursor #1027

Merged
merged 18 commits into from
Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions book/src/keymap.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
| `=` | Format selection | `format_selections` |
| `d` | Delete selection | `delete_selection` |
| `c` | Change selection (delete and enter insert mode) | `change_selection` |
| `Ctrl-a` | Increment number under cursor | `increment_number` |
| `Ctrl-x` | Decrement number under cursor | `decrement_number` |

#### Shell

Expand Down
1 change: 1 addition & 0 deletions helix-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod line_ending;
pub mod macros;
pub mod match_brackets;
pub mod movement;
pub mod numbers;
pub mod object;
pub mod path;
mod position;
Expand Down
144 changes: 144 additions & 0 deletions helix-core/src/numbers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
use std::borrow::Cow;

use ropey::{Rope, RopeSlice};

pub struct NumberInfo {
pub start: usize,
pub end: usize,
pub value: i64,
pub radix: u32,
}
jasonrhansen marked this conversation as resolved.
Show resolved Hide resolved

/// If there is a number at `char_idx`, return the text range, value and radix.
pub fn number_at(text: &Rope, char_idx: usize) -> Option<NumberInfo> {
let line = text.char_to_line(char_idx);
let line_start = text.line_to_char(line);
let line = text.line(line);
let line_len = line.len_chars();

let mut pos = char_idx - line_start;
let mut range_and_radix = None;

// Search from the cursor until a number is found or we reach the end of the line.
while range_and_radix.is_none() && pos < line_len {
pos += line.chars_at(pos).take_while(|c| !c.is_digit(16)).count();

range_and_radix = if let Some((start, end)) = hex_number_range(&line, pos) {
Some((start, end, 16))
} else if let Some((start, end)) = octal_number_range(&line, pos) {
Some((start, end, 8))
} else if let Some((start, end)) = binary_number_range(&line, pos) {
Some((start, end, 2))
} else if let Some((start, end)) = decimal_number_range(&line, pos) {
// We don't want to treat the '0' of the prefixes "0x", "0o", and "0b" as a number itself, so check for that here.
if end - start == 1 && line.char(start) == '0' && start + 2 < line_len {
let (c1, c2) = (line.char(start + 1), line.char(start + 2));
if c1 == 'x' && c2.is_digit(16)
|| c1 == 'o' && c2.is_digit(8)
|| c1 == 'b' && c2.is_digit(2)
{
pos += 2;
continue;
}
}

Some((start, end, 10))
} else {
pos += 1;
None
};
}

if let Some((start, end, radix)) = range_and_radix {
let number_text: Cow<str> = line.slice(start..end).into();
let value = i128::from_str_radix(&number_text, radix).ok()?;
if (value.is_positive() && value.leading_zeros() < 64)
|| (value.is_negative() && value.leading_ones() < 64)
{
return None;
}
Comment on lines +60 to +64
Copy link
Contributor

@pickfire pickfire Nov 14, 2021

Choose a reason for hiding this comment

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

I don't understand what does this do. Can you please put a comment here? Is it because the integer may be too large? Or is it to handle the cast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Vim's implementation, decrementing 0x0 results in 0xffffffffffffffff, which is -1 for i64. However, if we try to parse that with i64::from_str_radix we get an error because it tries to parse it as a positive number and overflows. If I parse it as an i128, I can cast it to i64 to get the negative value.

This check just makes sure the number fits in an i64. You may be wondering why I didn't use try_from to convert to i64, and the reason is it wouldn't work for parsing negative numbers that aren't base 10.

let value = i128::from_str_radix("ffffffffffffffff", 16).unwrap();
let value = value as i64;
// `value` now is -1

but

let value = i128::from_str_radix("ffffffffffffffff", 16).unwrap();
let value = i64::try_from(value).unwrap();

This version will get a TryFromIntError error because the original i128 is actually a positive number that's too large to fit in an i64.

Can you think of a better way to handle this, or do you think adding a comment is the best option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the best thing to do here is change the behavior from what Vim does.

Vim treats 0xFFFFFFFFFFFFFFFF as -1, because that's the bit pattern of -1 for a 64 bit two-complement integer. Vim only handles the minus sign for base 10 numbers.

So in Vim -0x01 gets incremented to -0x02 instead of 0x00 because it ignores the minus sign. We could make helix only interpret the number as negative if it has a minus sign.

Also in Vim 0x00 gets decremented to 0xffffffffffffffff, but we could have helix decrement it to -0x01.

But this would also mean that 0xffffffffffffffff would be too large and wouldn't be parsed as an i64.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the easiest way here might be just store it as i128 but either way, this is already merged so it should be good.

let value = value as i64;
Some(NumberInfo {
start: line_start + start,
end: line_start + end,
value,
radix,
})
} else {
None
}
}

/// Return the start and end of the decimal number at `pos` if there is one.
fn decimal_number_range(text: &RopeSlice, pos: usize) -> Option<(usize, usize)> {
if pos >= text.len_chars() {
return None;
}
let pos = pos + 1;
let mut chars = text.chars_at(pos);
chars.reverse();
let decimal_start = pos - chars.take_while(|c| c.is_digit(10)).count();

if decimal_start < pos {
let decimal_end = decimal_start
+ text
.chars_at(decimal_start)
.take_while(|c| c.is_digit(10))
.count();

// Handle negative numbers
if decimal_start > 0 && text.char(decimal_start - 1) == '-' {
Some((decimal_start - 1, decimal_end))
} else {
Some((decimal_start, decimal_end))
}
} else {
None
}
}

/// Return the start and end of the hexidecimal number at `pos` if there is one.
/// Hexidecimal numbers must be prefixed with "0x". The prefix will not be included in the range.
fn hex_number_range(text: &RopeSlice, pos: usize) -> Option<(usize, usize)> {
prefixed_number_range(text, pos, 16, 'x')
}

/// Return the start and end of the octal number at `pos` if there is one.
/// Octal numbers must be prefixed with "0o". The prefix will not be included in the range.
fn octal_number_range(text: &RopeSlice, pos: usize) -> Option<(usize, usize)> {
prefixed_number_range(text, pos, 8, 'o')
}

/// Return the start and end of the binary number at `pos` if there is one.
/// Binary numbers must be prefixed with "0b". The prefix will not be included in the range.
fn binary_number_range(text: &RopeSlice, pos: usize) -> Option<(usize, usize)> {
prefixed_number_range(text, pos, 2, 'b')
}

/// Return the start and end of the number at `pos` if there is one with the given `radix` and `prefix_char`.
/// The number must be prefixed with `'0' + prefix_char`. The prefix will not be included in the range.
fn prefixed_number_range(
text: &RopeSlice,
pos: usize,
radix: u32,
prefix_char: char,
) -> Option<(usize, usize)> {
if pos >= text.len_chars() {
return None;
}
let pos = pos + 1;
let mut chars = text.chars_at(pos);
chars.reverse();
let start = pos - chars.take_while(|c| c.is_digit(radix)).count();
let is_num = start < pos
&& start >= 2
&& text.char(start - 2) == '0'
&& text.char(start - 1) == prefix_char;

if is_num {
let end = pos + text.chars_at(pos).take_while(|c| c.is_digit(radix)).count();
Some((start, end))
} else {
None
}
}
95 changes: 95 additions & 0 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use helix_core::{
line_ending::{get_line_ending_of_str, line_end_char_index, str_is_line_ending},
match_brackets,
movement::{self, Direction},
numbers::{number_at, NumberInfo},
object, pos_at_coords,
regex::{self, Regex, RegexBuilder},
register::Register,
Expand Down Expand Up @@ -340,6 +341,8 @@ impl Command {
shell_keep_pipe, "Filter selections with shell predicate",
suspend, "Suspend",
rename_symbol, "Rename symbol",
increment_number, "Increment number under cursor",
decrement_number, "Decrement number under cursor",
);
}

Expand Down Expand Up @@ -5100,3 +5103,95 @@ fn rename_symbol(cx: &mut Context) {
);
cx.push_layer(Box::new(prompt));
}

/// If there is a number at the cursor, increment it by the count.
fn increment_number(cx: &mut Context) {
increment_number_impl(cx, cx.count() as i64);
}

/// If there is a number at the cursor, decrement it by the count.
fn decrement_number(cx: &mut Context) {
increment_number_impl(cx, -(cx.count() as i64));
}

/// If there is a number at the cursor, increment it by `amount`.
fn increment_number_impl(cx: &mut Context, amount: i64) {
let (view, doc) = current!(cx.editor);

let selection = doc.selection(view.id);
if selection.len() != 1 {
return;
}

let primary_selection = selection.primary();
if primary_selection.to() - primary_selection.from() != 1 {
return;
}

let text = doc.text();
let cursor = primary_selection.cursor(text.slice(..));

if let Some(NumberInfo {
start,
end,
value: old_value,
radix,
}) = number_at(text, cursor)
pickfire marked this conversation as resolved.
Show resolved Hide resolved
{
let number_text: Cow<str> = text.slice(start..end).into();
let number_text = number_text.strip_prefix('-').unwrap_or(&number_text);

let new_value = old_value.wrapping_add(amount);
let old_length = end - start;

let (replacement, new_length) = {
let mut replacement = match radix {
2 => format!("{:b}", new_value),
8 => format!("{:o}", new_value),
10 => format!("{}", new_value.abs()),
16 => {
let lower_count = number_text.chars().filter(char::is_ascii_lowercase).count();
let upper_count = number_text.chars().filter(char::is_ascii_uppercase).count();
if upper_count > lower_count {
format!("{:X}", new_value)
} else {
format!("{:x}", new_value)
}
}
_ => unimplemented!("radix not supported: {}", radix),
};

let mut new_length = replacement.chars().count();

let old_length_no_sign = number_text.chars().count();
if new_length < old_length_no_sign && (radix != 10 || number_text.starts_with('0')) {
replacement = "0".repeat(old_length_no_sign - new_length) + &replacement;
new_length = old_length_no_sign;
}

if radix == 10 && new_value.is_negative() {
replacement = format!("-{}", replacement);
new_length += 1;
}

(replacement, new_length)
};

let changes = std::iter::once((start, end, Some(replacement.into())));
let mut transaction = Transaction::change(text, changes);

// Move cursor to the last character of the number.
let mut selection = doc.selection(view.id).clone();
let mut primary = selection.primary_mut();
primary.anchor = if new_length < old_length {
end - 1 - old_length + new_length
} else {
end - 1 + new_length - old_length
};
primary.head = primary.anchor;
jasonrhansen marked this conversation as resolved.
Show resolved Hide resolved
transaction = transaction.with_selection(selection);

doc.apply(&transaction, view.id);
doc.append_changes_to_history(view.id);
}
}
3 changes: 3 additions & 0 deletions helix-term/src/keymap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@ impl Default for Keymaps {
"A-!" => shell_append_output,
"$" => shell_keep_pipe,
"C-z" => suspend,

"C-a" => increment_number,
"C-x" => decrement_number,
pickfire marked this conversation as resolved.
Show resolved Hide resolved
});
let mut select = normal.clone();
select.merge_nodes(keymap!({ "Select mode"
Expand Down