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 configuration for min width of line-numbers gutter #4724

Merged
merged 22 commits into from
Jan 21, 2023
Merged
Show file tree
Hide file tree
Changes from 16 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
52 changes: 52 additions & 0 deletions book/src/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,55 @@ render = true
character = "╎" # Some characters that work well: "▏", "┆", "┊", "⸽"
skip-levels = 1
```

### `[editor.gutters]` Section

For simplicity, `editor.gutters` accepts an array of gutter types, which will
use default settings for all gutter components.

```toml
[editor]
gutters = ["diff", "diagnostics", "line-numbers", "spacer"]
```

To customize the behavior of gutters, the `[editor.gutters]` section must
be used. This section contains top level settings, as well as settings for
specific gutter components as sub-sections.

| Key | Description | Default |
| --- | --- | --- |
| `layout` | A vector of gutters to display | `["diagnostics", "spacer", "line-numbers", "spacer", "diff"]` |

Example:

```toml
[editor.gutters]
layout = ["diff", "diagnostics", "line-numbers", "spacer"]
```

#### `[editor.gutters.line-numbers]` Section

Options for the line number gutter

| Key | Description | Default |
| --- | --- | --- |
| `min-width` | The minimum number of characters to use | `3` |

Example:

```toml
[editor.gutters.line-numbers]
min-width = 1
```

#### `[editor.gutters.diagnotics]` Section

Currently unused

#### `[editor.gutters.diff]` Section

Currently unused

#### `[editor.gutters.spacer]` Section

Currently unused
Copy link
Contributor

Choose a reason for hiding this comment

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

What about [editor.gutters.breakpoints]? I’m mentioning this because of what I’m doing in #5371 (I might eventually rebase on your work).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now the sub-sections are styled after the names that can be provided to editor.gutters. There's no real reason it must be like this, but it think there's some discoverability benefit to being consistent here.

Following that style, both breakpoints and diagnostics would be configured in editors.gutters.diagnostics. I think it's best to follow this precedent, but I'd say this is a call for some of the more core contributors like @the-mikedavis


Looking into the future a bit further, I think the underlying issue here is that diagnostics and breakpoints are a bit oddly coupled - likely a solution from before gutters were configurable. I think there's a case for decoupling diagnostics and breakpoints and trying to tackle that use case more generically. I think it would be interesting to be able to layer gutters as part of the layout. Perhaps something like:

[editor.gutters]
layout = [ [ "diagnostics", "breakpoints" ], "line-numbers" ]

Which would provide the current behavior. I think that needs more discussion before jumping to something like that, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m reimplementing #5371 rebased atop your PR then.

114 changes: 105 additions & 9 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
io::stdin,
marker::PhantomData,
num::NonZeroUsize,
path::{Path, PathBuf},
pin::Pin,
Expand Down Expand Up @@ -71,6 +72,105 @@ where
)
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct GutterConfig {
/// Gutter Layout
pub layout: Vec<GutterType>,
/// Options specific to the "line-numbers" gutter
pub line_numbers: GutterLineNumbersConfig,
}

impl Default for GutterConfig {
fn default() -> Self {
Self {
layout: vec![
GutterType::Diagnostics,
GutterType::Spacer,
GutterType::LineNumbers,
GutterType::Spacer,
GutterType::Diff,
],
line_numbers: GutterLineNumbersConfig::default(),
}
}
}

impl From<Vec<GutterType>> for GutterConfig {
fn from(x: Vec<GutterType>) -> Self {
GutterConfig {
layout: x,
..Default::default()
}
}
}

fn deserialize_gutter_seq_or_struct<'de, T, D>(deserializer: D) -> Result<T, D::Error>
where
T: Deserialize<'de> + From<Vec<GutterType>>,
D: Deserializer<'de>,
{
// This is a Visitor that forwards string types to T's `FromStr` impl and
// forwards map types to T's `Deserialize` impl. The `PhantomData` is to
// keep the compiler from complaining about T being an unused generic type
// parameter. We need T in order to know the Value type for the Visitor
// impl.
struct GutterSeqOrStruct<T>(PhantomData<fn() -> T>);
dgkf marked this conversation as resolved.
Show resolved Hide resolved

impl<'de, T> serde::de::Visitor<'de> for GutterSeqOrStruct<T>
where
T: Deserialize<'de> + From<Vec<GutterType>>,
{
type Value = T;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
formatter,
"an array of gutter names or a detailed gutter configuration"
)
}

fn visit_seq<S>(self, mut seq: S) -> Result<Self::Value, S::Error>
where
S: serde::de::SeqAccess<'de>,
{
let mut gutters = Vec::new();
while let Some(gutter) = seq.next_element::<&str>()? {
gutters.push(
gutter
.parse::<GutterType>()
.map_err(serde::de::Error::custom)?,
)
}

Ok(gutters.into())
}

fn visit_map<M>(self, map: M) -> Result<Self::Value, M::Error>
where
M: serde::de::MapAccess<'de>,
{
let deserializer = serde::de::value::MapAccessDeserializer::new(map);
Deserialize::deserialize(deserializer)
}
}

deserializer.deserialize_any(GutterSeqOrStruct(PhantomData))
}
dgkf marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct GutterLineNumbersConfig {
/// Minimum number of characters to use for line number gutter. Defaults to 3.
pub min_width: usize,
}

impl Default for GutterLineNumbersConfig {
fn default() -> Self {
Self { min_width: 3 }
}
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct FilePickerConfig {
Expand Down Expand Up @@ -132,8 +232,8 @@ pub struct Config {
pub cursorline: bool,
/// Highlight the columns cursors are currently on. Defaults to false.
pub cursorcolumn: bool,
/// Gutters. Default ["diagnostics", "line-numbers"]
pub gutters: Vec<GutterType>,
#[serde(deserialize_with = "deserialize_gutter_seq_or_struct")]
pub gutters: GutterConfig,
/// Middle click paste support. Defaults to true.
pub middle_click_paste: bool,
/// Automatic insertion of pairs to parentheses, brackets,
Expand Down Expand Up @@ -603,13 +703,7 @@ impl Default for Config {
line_number: LineNumber::Absolute,
cursorline: false,
cursorcolumn: false,
gutters: vec![
GutterType::Diagnostics,
GutterType::Spacer,
GutterType::LineNumbers,
GutterType::Spacer,
GutterType::Diff,
],
gutters: GutterConfig::default(),
middle_click_paste: true,
auto_pairs: AutoPairConfig::default(),
auto_completion: true,
Expand Down Expand Up @@ -841,6 +935,7 @@ impl Editor {
let config = self.config();
self.auto_pairs = (&config.auto_pairs).into();
self.reset_idle_timer();
self._refresh();
}

pub fn clear_idle_timer(&mut self) {
Expand Down Expand Up @@ -981,6 +1076,7 @@ impl Editor {
for (view, _) in self.tree.views_mut() {
let doc = doc_mut!(self, &view.doc);
view.sync_changes(doc);
view.gutters = config.gutters.clone();
dgkf marked this conversation as resolved.
Show resolved Hide resolved
view.ensure_cursor_in_view(doc, config.scrolloff)
}
}
Expand Down
109 changes: 101 additions & 8 deletions helix-view/src/gutter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cmp::max;
use std::fmt::Write;

use crate::{
Expand Down Expand Up @@ -140,12 +141,18 @@ pub fn line_numbers<'doc>(
is_focused: bool,
) -> GutterFn<'doc> {
let text = doc.text().slice(..);
let last_line = view.last_line(doc);
let width = GutterType::LineNumbers.width(view, doc);

let last_line = text.len_lines().saturating_sub(1);
let last_line_in_view = view.last_line(doc);

// Whether to draw the line number for the last line of the
// document or not. We only draw it if it's not an empty line.
let draw_last = text.line_to_byte(last_line) < text.len_bytes();
let draw_last = text.line_to_byte(last_line_in_view) < text.len_bytes();
let last_drawn_line = if draw_last { last_line + 1 } else { last_line };

// characters used to display last line, and settings for min/max
let n_last_line = count_digits(last_drawn_line);
let n_min = view.gutters.line_numbers.min_width;

let linenr = theme.get("ui.linenr");
let linenr_select = theme.get("ui.linenr.selected");
Expand All @@ -158,7 +165,8 @@ pub fn line_numbers<'doc>(
let mode = editor.mode;

Box::new(move |line: usize, selected: bool, out: &mut String| {
if line == last_line && !draw_last {
if line == last_line_in_view && !draw_last {
let width = max(n_last_line, n_min);
dgkf marked this conversation as resolved.
Show resolved Hide resolved
write!(out, "{:>1$}", '~', width).unwrap();
Some(linenr)
} else {
Expand All @@ -181,20 +189,26 @@ pub fn line_numbers<'doc>(
linenr
};

let width = max(n_last_line, n_min);
write!(out, "{:>1$}", display_num, width).unwrap();
Some(style)
}
})
}

pub fn line_numbers_width(_view: &View, doc: &Document) -> usize {
/// The width of a "line-numbers" gutter
///
/// The width of the gutter depends on the number of lines in the document,
/// whether there is content on the last line (the `~` line), and the
/// `editor.gutters.line-numbers.min-width` settings.
pub fn line_numbers_width(view: &View, doc: &Document) -> usize {
the-mikedavis marked this conversation as resolved.
Show resolved Hide resolved
let text = doc.text();
let last_line = text.len_lines().saturating_sub(1);
let draw_last = text.line_to_byte(last_line) < text.len_bytes();
let last_drawn = if draw_last { last_line + 1 } else { last_line };

// set a lower bound to 2-chars to minimize ambiguous relative line numbers
std::cmp::max(count_digits(last_drawn), 2)
let digits = count_digits(last_drawn);
let n_min = view.gutters.line_numbers.min_width;
max(digits, n_min)
}

pub fn padding<'doc>(
Expand Down Expand Up @@ -282,3 +296,82 @@ pub fn diagnostics_or_breakpoints<'doc>(
breakpoints(line, selected, out).or_else(|| diagnostics(line, selected, out))
})
}

#[cfg(test)]
mod tests {
use super::*;
use crate::document::Document;
use crate::editor::{GutterConfig, GutterLineNumbersConfig};
use crate::graphics::Rect;
use crate::DocumentId;
use helix_core::Rope;

#[test]
fn test_default_gutter_widths() {
let mut view = View::new(DocumentId::default(), GutterConfig::default());
view.area = Rect::new(40, 40, 40, 40);

let rope = Rope::from_str("abc\n\tdef");
let doc = Document::from(rope, None);

assert_eq!(view.gutters.layout.len(), 5);
assert_eq!(view.gutters.layout[0].width(&view, &doc), 1);
assert_eq!(view.gutters.layout[1].width(&view, &doc), 1);
assert_eq!(view.gutters.layout[2].width(&view, &doc), 3);
assert_eq!(view.gutters.layout[3].width(&view, &doc), 1);
assert_eq!(view.gutters.layout[4].width(&view, &doc), 1);
}

#[test]
fn test_configured_gutter_widths() {
let gutters = GutterConfig {
layout: vec![GutterType::Diagnostics],
..Default::default()
};

let mut view = View::new(DocumentId::default(), gutters);
view.area = Rect::new(40, 40, 40, 40);

let rope = Rope::from_str("abc\n\tdef");
let doc = Document::from(rope, None);

assert_eq!(view.gutters.layout.len(), 1);
assert_eq!(view.gutters.layout[0].width(&view, &doc), 1);

let gutters = GutterConfig {
layout: vec![GutterType::Diagnostics, GutterType::LineNumbers],
line_numbers: GutterLineNumbersConfig { min_width: 10 },
};

let mut view = View::new(DocumentId::default(), gutters);
view.area = Rect::new(40, 40, 40, 40);

let rope = Rope::from_str("abc\n\tdef");
let doc = Document::from(rope, None);

assert_eq!(view.gutters.layout.len(), 2);
assert_eq!(view.gutters.layout[0].width(&view, &doc), 1);
assert_eq!(view.gutters.layout[1].width(&view, &doc), 10);
}

#[test]
fn test_line_numbers_gutter_width_resizes() {
let gutters = GutterConfig {
layout: vec![GutterType::Diagnostics, GutterType::LineNumbers],
line_numbers: GutterLineNumbersConfig { min_width: 1 },
};

let mut view = View::new(DocumentId::default(), gutters);
view.area = Rect::new(40, 40, 40, 40);

let rope = Rope::from_str("a\nb");
let doc_short = Document::from(rope, None);

let rope = Rope::from_str("a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np");
let doc_long = Document::from(rope, None);

assert_eq!(view.gutters.layout.len(), 2);
assert_eq!(view.gutters.layout[1].width(&view, &doc_short), 1);
assert_eq!(view.gutters.layout[1].width(&view, &doc_long), 2);
}
}
Loading