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

Add a conf option to show no line number #769

Closed
wants to merge 1 commit into from
Closed

Add a conf option to show no line number #769

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
23 changes: 15 additions & 8 deletions helix-term/src/ui/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,12 @@ impl EditorView {
) {
let text = doc.text().slice(..);
let last_line = view.last_line(doc);
let last_line_width = match config.line_number {
LineNumber::Absolute | LineNumber::Relative => {
(last_line + 1).to_string().chars().count()
}
LineNumber::None => 0,
};

let linenr = theme.get("ui.linenr");
let linenr_select: Style = theme.try_get("ui.linenr.selected").unwrap_or(linenr);
Expand Down Expand Up @@ -457,25 +463,26 @@ impl EditorView {
let selected = cursors.contains(&line);

let text = if line == last_line && !draw_last {
" ~".into()
format!("{:>1$}", "~", last_line_width)
} else {
let line = match config.line_number {
LineNumber::Absolute => line + 1,
match config.line_number {
LineNumber::Absolute => format!("{:>1$}", line + 1, last_line_width),
LineNumber::Relative => {
if current_line == line {
line + 1
format!("{:>1$}", line + 1, last_line_width)
} else {
abs_diff(current_line, line)
format!("{:>1$}", abs_diff(current_line, line), last_line_width)
}
}
};
format!("{:>5}", line)
// if the line numbers are disabled we can entirely ignore the line width and return nothing
LineNumber::None => String::new(),
}
};
surface.set_stringn(
viewport.x + 1,
viewport.y + i as u16,
text,
5,
last_line_width,
if selected && is_focused {
linenr_select
} else {
Expand Down
7 changes: 5 additions & 2 deletions helix-view/src/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ pub enum LineNumber {

/// Show relative line number to the primary cursor
Relative,

/// Do not show the line number
None,
}

impl Default for Config {
Expand Down Expand Up @@ -215,14 +218,14 @@ impl Editor {
return;
}
Action::HorizontalSplit => {
let view = View::new(id);
let view = View::new(id, self.config.clone());
let view_id = self.tree.split(view, Layout::Horizontal);
// initialize selection for view
let doc = &mut self.documents[id];
doc.selections.insert(view_id, Selection::point(0));
}
Action::VerticalSplit => {
let view = View::new(id);
let view = View::new(id, self.config.clone());
let view_id = self.tree.split(view, Layout::Vertical);
// initialize selection for view
let doc = &mut self.documents[id];
Expand Down
48 changes: 29 additions & 19 deletions helix-view/src/view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::borrow::Cow;

use crate::{graphics::Rect, Document, DocumentId, ViewId};
use crate::{editor::LineNumber, graphics::Rect, Document, DocumentId, ViewId};
use helix_core::{
coords_at_pos,
graphemes::{grapheme_width, RopeGraphemes},
Expand Down Expand Up @@ -66,24 +66,32 @@ pub struct View {
pub jumps: JumpList,
/// the last accessed file before the current one
pub last_accessed_doc: Option<DocumentId>,
pub config: crate::editor::Config,
}

impl View {
pub fn new(doc: DocumentId) -> Self {
pub fn new(doc: DocumentId, config: crate::editor::Config) -> Self {
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 config should be shared, best without using RefCell<Mutex<Config>> for #798

Self {
id: ViewId::default(),
doc,
offset: Position::new(0, 0),
area: Rect::default(), // will get calculated upon inserting into tree
jumps: JumpList::new((doc, Selection::point(0))), // TODO: use actual sel
last_accessed_doc: None,
config,
}
}

pub fn inner_area(&self) -> Rect {
// TODO: not ideal
const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter
self.area.clip_left(OFFSET).clip_bottom(1) // -1 for statusline
const OFFSET: usize = 1 + 1; // 1 diagnostic + 1 gutter
let last_line = self.offset.row + self.area.height as usize - 1; // -1 for statusline
let offset = match self.config.line_number {
LineNumber::Absolute | LineNumber::Relative => {
last_line.to_string().chars().count() + OFFSET
Copy link
Contributor

Choose a reason for hiding this comment

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

I just opened a log file with a few million lines and was searching for an issue about cut-off line numbers, nice to see that it’s fixed here!
I think computing the offset is more efficient with

/// Computes the number of decimal digits needed to print a number.
fn digits10(i: usize) -> usize {
    ((i + 1) as f64).log10().ceil() as usize
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huum I'm not sure that using float and log10 would be any fast 🤔

What do you think of this function?

/// Computes the number of decimal digits needed to print a number.
fn digits10(n: usize) -> usize {
    std::iter::successors(Some(n), |n| {
        let n = n / 10;
        (n != 0).then(|| n)
    }).count()
}

I'll run a little benchmark just to get an idea but thanks for the suggestion.
I don't think this PR will be merged any time soon however because I don't really have any time to invest in helix currently so you can take it if you want and have the time 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is my result with a bunch of numbers:

log10/0                     time:   [6.8428 ns 6.8473 ns 6.8525 ns]                            
divide/0                    time:   [1.2312 ns 1.2334 ns 1.2362 ns]                             

log10/100                   time:   [8.8163 ns 8.8380 ns 8.8643 ns]                              
divide/100                  time:   [1.6460 ns 1.6472 ns 1.6485 ns]                               

log10/1000                  time:   [9.0255 ns 9.0385 ns 9.0530 ns]                               
divide/1000                 time:   [1.9341 ns 1.9369 ns 1.9403 ns]                                

log10/4000                  time:   [9.3187 ns 9.3493 ns 9.3821 ns]                               
divide/4000                 time:   [1.8846 ns 1.8853 ns 1.8860 ns]                                

log10/10000                 time:   [8.7732 ns 8.7818 ns 8.7922 ns]                                
divide/10000                time:   [2.0882 ns 2.0900 ns 2.0922 ns]                                 

log10/50000                 time:   [9.1107 ns 9.1342 ns 9.1616 ns]                                
divide/50000                time:   [2.1692 ns 2.1741 ns 2.1798 ns]                                 

log10/100000                time:   [9.0641 ns 9.0708 ns 9.0776 ns]                                 
divide/100000               time:   [2.4120 ns 2.4160 ns 2.4204 ns]                                  

log10/500000                time:   [9.3445 ns 9.3639 ns 9.3847 ns]                                 
divide/500000               time:   [2.3472 ns 2.3540 ns 2.3627 ns]                                  

log10/18446744073709551615  time:   [7.7829 ns 7.7914 ns 7.8008 ns]
divide/18446744073709551615 time:   [10.872 ns 10.879 ns 10.887 ns]

log10 is your function and divide is my function. The number after the / is which number was given to our function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, that’s a lot faster!

}
LineNumber::None => OFFSET,
};
self.area.clip_left(offset as u16).clip_bottom(1) // -1 for statusline
}

pub fn ensure_cursor_in_view(&mut self, doc: &Document, scrolloff: usize) {
Expand Down Expand Up @@ -242,13 +250,15 @@ impl View {

#[cfg(test)]
mod tests {
use crate::editor::Config;

use super::*;
use helix_core::Rope;
const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter

#[test]
fn test_text_pos_at_screen_coords() {
let mut view = View::new(DocumentId::default());
let mut view = View::new(DocumentId::default(), Config::default());
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("abc\n\tdef");
let text = rope.slice(..);
Expand Down Expand Up @@ -281,7 +291,7 @@ mod tests {

assert_eq!(
view.text_pos_at_screen_coords(&text, 41, 40 + OFFSET + 4, 4),
Some(5)
Some(8)
);

assert_eq!(
Expand All @@ -294,67 +304,67 @@ mod tests {

#[test]
fn test_text_pos_at_screen_coords_cjk() {
let mut view = View::new(DocumentId::default());
let mut view = View::new(DocumentId::default(), Config::default());
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("Hi! こんにちは皆さん");
let text = rope.slice(..);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 0, 4),
Some(0)
Some(3)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 5, 4),
Some(5)
Some(6)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 6, 4),
Some(5)
Some(7)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 7, 4),
Some(6)
Some(7)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 8, 4),
Some(6)
Some(8)
);
}

#[test]
fn test_text_pos_at_screen_coords_graphemes() {
let mut view = View::new(DocumentId::default());
let mut view = View::new(DocumentId::default(), Config::default());
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("Hèl̀l̀ò world!");
let text = rope.slice(..);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 0, 4),
Some(0)
Some(5)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 1, 4),
Some(1)
Some(7)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 2, 4),
Some(3)
Some(9)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 3, 4),
Some(5)
Some(10)
);

assert_eq!(
view.text_pos_at_screen_coords(&text, 40, 40 + OFFSET + 4, 4),
Some(7)
Some(11)
);
}
}