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 centered view mode #3239

Closed
wants to merge 2 commits into from
Closed

Add centered view mode #3239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e27a2d0
Add toggle_centered_view command and centered-views config
Jul 29, 2022
b785e5a
Update docs
Jul 29, 2022
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
1 change: 1 addition & 0 deletions book/src/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ on unix operating systems.
| `rulers` | List of column positions at which to display the rulers. Can be overridden by language specific `rulers` in `languages.toml` file. | `[]` |
| `bufferline` | Renders a line at the top of the editor displaying open buffers. Can be `always`, `never` or `multiple` (only shown if more than one buffer is in use) | `never` |
| `color-modes` | Whether to color the mode indicator with different colors depending on the mode itself | `false` |
| `centered-views` | Wether to center views by default | `false` |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `centered-views` | Wether to center views by default | `false` |
| `centered-views` | Whether to center views by default | `false` |


### `[editor.statusline]` Section

Expand Down
35 changes: 18 additions & 17 deletions book/src/keymap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,23 +244,24 @@ Accessed by typing `Ctrl-w` in [normal mode](#normal-mode).

This layer is similar to Vim keybindings as Kakoune does not support window.

| Key | Description | Command |
| ----- | ------------- | ------- |
| `w`, `Ctrl-w` | Switch to next window | `rotate_view` |
| `v`, `Ctrl-v` | Vertical right split | `vsplit` |
| `s`, `Ctrl-s` | Horizontal bottom split | `hsplit` |
| `f` | Go to files in the selection in horizontal splits | `goto_file` |
| `F` | Go to files in the selection in vertical splits | `goto_file` |
| `h`, `Ctrl-h`, `Left` | Move to left split | `jump_view_left` |
| `j`, `Ctrl-j`, `Down` | Move to split below | `jump_view_down` |
| `k`, `Ctrl-k`, `Up` | Move to split above | `jump_view_up` |
| `l`, `Ctrl-l`, `Right` | Move to right split | `jump_view_right` |
| `q`, `Ctrl-q` | Close current window | `wclose` |
| `o`, `Ctrl-o` | Only keep the current window, closing all the others | `wonly` |
| `H` | Swap window to the left | `swap_view_left` |
| `J` | Swap window downwards | `swap_view_down` |
| `K` | Swap window upwards | `swap_view_up` |
| `L` | Swap window to the right | `swap_view_right` |
| Key | Description | Command |
| ----- | ------------- | ------- |
| `c` | Toggle centered view | `toggle_centered_view` |
| `w`, `Ctrl-w` | Switch to next window | `rotate_view` |
| `v`, `Ctrl-v` | Vertical right split | `vsplit` |
| `s`, `Ctrl-s` | Horizontal bottom split | `hsplit` |
| `f` | Go to files in the selection in horizontal splits | `goto_file` |
| `F` | Go to files in the selection in vertical splits | `goto_file` |
| `h`, `Ctrl-h`, `Left` | Move to left split | `jump_view_left` |
| `j`, `Ctrl-j`, `Down` | Move to split below | `jump_view_down` |
| `k`, `Ctrl-k`, `Up` | Move to split above | `jump_view_up` |
| `l`, `Ctrl-l`, `Right` | Move to right split | `jump_view_right` |
| `q`, `Ctrl-q` | Close current window | `wclose` |
| `o`, `Ctrl-o` | Only keep the current window, closing all the others | `wonly` |
| `H` | Swap window to the left | `swap_view_left` |
| `J` | Swap window downwards | `swap_view_down` |
| `K` | Swap window upwards | `swap_view_up` |
| `L` | Swap window to the right | `swap_view_right` |

#### Space mode

Expand Down
8 changes: 8 additions & 0 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ impl MappableCommand {
wonly, "Close windows except current",
select_register, "Select register",
insert_register, "Insert register",
toggle_centered_view, "Toggle centered view",
Copy link
Member

Choose a reason for hiding this comment

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

The naming seems confusing since the view isn't "centered" but is instead in a distraction free mode. Tmux also calls this "zoomed (in)" since the view is maximized and replaces the whole tree.

align_view_middle, "Align view middle",
align_view_top, "Align view top",
align_view_center, "Align view center",
Expand Down Expand Up @@ -4571,6 +4572,13 @@ fn insert_register(cx: &mut Context) {
})
}

fn toggle_centered_view(cx: &mut Context) {
let mut view = view_mut!(cx.editor);
view.is_centered = !view.is_centered;

cx.editor.tree.recalculate();
}

fn align_view_top(cx: &mut Context) {
let (view, doc) = current!(cx.editor);
align_view(doc, view, Align::Top);
Expand Down
2 changes: 2 additions & 0 deletions helix-term/src/keymap/default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ pub fn default() -> HashMap<Mode, Keymap> {
"C-d" => half_page_down,

"C-w" => { "Window"
"c" => toggle_centered_view,
"C-w" | "w" => rotate_view,
"C-s" | "s" => hsplit,
"C-v" | "v" => vsplit,
Expand Down Expand Up @@ -242,6 +243,7 @@ pub fn default() -> HashMap<Mode, Keymap> {
"E" => dap_disable_exceptions,
},
"w" => { "Window"
"c" => toggle_centered_view,
"C-w" | "w" => rotate_view,
"C-s" | "s" => hsplit,
"C-v" | "v" => vsplit,
Expand Down
17 changes: 15 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 @@ -274,6 +274,8 @@ pub struct Config {
/// Whether to color modes with different colors. Defaults to `false`.
pub color_modes: bool,
pub soft_wrap: SoftWrap,
/// Whether to center views by default, Defaults to `false`.
pub centered_views: bool,
Comment on lines +277 to +278
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove the config option since it seems unlikely someone would want this always on.

}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -772,6 +774,7 @@ impl Default for Config {
indent_guides: IndentGuidesConfig::default(),
color_modes: false,
soft_wrap: SoftWrap::default(),
centered_views: false,
}
}
}
Expand Down Expand Up @@ -1241,7 +1244,13 @@ impl Editor {
.try_get(self.tree.focus)
.filter(|v| id == v.doc) // Different Document
.cloned()
.unwrap_or_else(|| View::new(id, self.config().gutters.clone()));
.unwrap_or_else(|| {
View::new(
id,
self.config().gutters.clone(),
self.config().centered_views,
)
});
let view_id = self.tree.split(
view,
match action {
Expand Down Expand Up @@ -1398,7 +1407,11 @@ impl Editor {
.map(|(&doc_id, _)| doc_id)
.next()
.unwrap_or_else(|| self.new_document(Document::default(self.config.clone())));
let view = View::new(doc_id, self.config().gutters.clone());
let view = View::new(
doc_id,
self.config().gutters.clone(),
self.config().centered_views,
);
let view_id = self.tree.insert(view);
let doc = doc_mut!(self, &doc_id);
doc.ensure_view_init(view_id);
Expand Down
43 changes: 31 additions & 12 deletions helix-view/src/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,35 @@ impl Tree {

// take the area
// fetch the node
// a) node is view, give it whole area
// a) node is view
// 1) view is centered and the parent container is not a vertical layout or
// with only 1 child (the current view), center the view
// 2) give the whole area
// b) node is container, calculate areas for each child and push them on the stack

while let Some((key, area)) = self.stack.pop() {
let node = &mut self.nodes[key];
let parent = self.nodes[key].parent;

match &mut node.content {
// Parent must always be a container
let (parent_child_count, parent_layout) = match &self.nodes[parent].content {
Content::Container(container) => (container.children.len(), container.layout),
Content::View(_) => unreachable!(),
};

match &mut self.nodes[key].content {
Content::View(view) => {
// debug!!("setting view area {:?}", area);
view.area = area;
if view.is_centered
&& (parent_child_count <= 1 || parent_layout != Layout::Vertical)
{
let width = std::cmp::min(std::cmp::max(area.width / 2, 120), area.width);
let area =
Rect::new(area.width / 2 - width / 2, area.y, width, area.height);

view.area = area;
} else {
view.area = area;
}
Comment on lines +363 to +373
Copy link
Member

Choose a reason for hiding this comment

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

This gives the view the entire area but doesn't affect any of the other views. This means that you're relying on view ordering (this view should be the last one rendered) otherwise other views might get rendered over it. We also pay the performance cost of rendering all other views even though they're not on screen. (Also what happens if I change focus to a different view but this one is still centered?)

I'd rather see this done via a zoom bool flag on the tree. If set to true then the editor should just render the currently focused node and ignore the rest of the tree. The reason we haven't done this yet is because this will likely impact a bunch of other viewport calculations.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, thanks for the feedback, but... to be honest I made this PR 7 months ago, so I don't remember at all why I made things in this way, and I don't have sufficent spare time to invest in this PR anymore :/ if someone is motivated enough to take over my work (or redoing it from scratch ^^') it would be better

} // TODO: call f()
Content::Container(container) => {
// debug!!("setting container area {:?}", area);
Expand Down Expand Up @@ -712,22 +731,22 @@ mod test {
width: 180,
height: 80,
});
let mut view = View::new(DocumentId::default(), GutterConfig::default());
let mut view = View::new(DocumentId::default(), GutterConfig::default(), false);
view.area = Rect::new(0, 0, 180, 80);
tree.insert(view);

let l0 = tree.focus;
let view = View::new(DocumentId::default(), GutterConfig::default());
let view = View::new(DocumentId::default(), GutterConfig::default(), false);
tree.split(view, Layout::Vertical);
let r0 = tree.focus;

tree.focus = l0;
let view = View::new(DocumentId::default(), GutterConfig::default());
let view = View::new(DocumentId::default(), GutterConfig::default(), false);
tree.split(view, Layout::Horizontal);
let l1 = tree.focus;

tree.focus = l0;
let view = View::new(DocumentId::default(), GutterConfig::default());
let view = View::new(DocumentId::default(), GutterConfig::default(), false);
tree.split(view, Layout::Vertical);
let l2 = tree.focus;

Expand Down Expand Up @@ -769,28 +788,28 @@ mod test {
});

let doc_l0 = DocumentId::default();
let mut view = View::new(doc_l0, GutterConfig::default());
let mut view = View::new(doc_l0, GutterConfig::default(), false);
view.area = Rect::new(0, 0, 180, 80);
tree.insert(view);

let l0 = tree.focus;

let doc_r0 = DocumentId::default();
let view = View::new(doc_r0, GutterConfig::default());
let view = View::new(doc_r0, GutterConfig::default(), false);
tree.split(view, Layout::Vertical);
let r0 = tree.focus;

tree.focus = l0;

let doc_l1 = DocumentId::default();
let view = View::new(doc_l1, GutterConfig::default());
let view = View::new(doc_l1, GutterConfig::default(), false);
tree.split(view, Layout::Horizontal);
let l1 = tree.focus;

tree.focus = l0;

let doc_l2 = DocumentId::default();
let view = View::new(doc_l2, GutterConfig::default());
let view = View::new(doc_l2, GutterConfig::default(), false);
tree.split(view, Layout::Vertical);
let l2 = tree.focus;

Expand Down
12 changes: 8 additions & 4 deletions helix-view/src/view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub struct View {
pub id: ViewId,
pub offset: ViewPosition,
pub area: Rect,
pub is_centered: bool,
pub doc: DocumentId,
pub jumps: JumpList,
// documents accessed from this view from the oldest one to last viewed one
Expand Down Expand Up @@ -138,7 +139,7 @@ impl fmt::Debug for View {
}

impl View {
pub fn new(doc: DocumentId, gutters: GutterConfig) -> Self {
pub fn new(doc: DocumentId, gutters: GutterConfig, is_centered: bool) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

The default should be false and we don't want this as a part of the new() method.

Self {
id: ViewId::default(),
doc,
Expand All @@ -148,6 +149,7 @@ impl View {
vertical_offset: 0,
},
area: Rect::default(), // will get calculated upon inserting into tree
is_centered,
jumps: JumpList::new((doc, Selection::point(0))), // TODO: use actual sel
docs_access_history: Vec::new(),
last_modified_docs: [None, None],
Expand Down Expand Up @@ -597,7 +599,7 @@ mod tests {

#[test]
fn test_text_pos_at_screen_coords() {
let mut view = View::new(DocumentId::default(), GutterConfig::default());
let mut view = View::new(DocumentId::default(), GutterConfig::default(), false);
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("abc\n\tdef");
let doc = Document::from(
Expand Down Expand Up @@ -771,6 +773,7 @@ mod tests {
layout: vec![GutterType::Diagnostics],
line_numbers: GutterLineNumbersConfig::default(),
},
false,
);
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("abc\n\tdef");
Expand Down Expand Up @@ -800,6 +803,7 @@ mod tests {
layout: vec![],
line_numbers: GutterLineNumbersConfig::default(),
},
false,
);
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("abc\n\tdef");
Expand All @@ -823,7 +827,7 @@ mod tests {

#[test]
fn test_text_pos_at_screen_coords_cjk() {
let mut view = View::new(DocumentId::default(), GutterConfig::default());
let mut view = View::new(DocumentId::default(), GutterConfig::default(), false);
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("Hi! こんにちは皆さん");
let doc = Document::from(
Expand Down Expand Up @@ -906,7 +910,7 @@ mod tests {

#[test]
fn test_text_pos_at_screen_coords_graphemes() {
let mut view = View::new(DocumentId::default(), GutterConfig::default());
let mut view = View::new(DocumentId::default(), GutterConfig::default(), false);
view.area = Rect::new(40, 40, 40, 40);
let rope = Rope::from_str("Hèl̀l̀ò world!");
let doc = Document::from(
Expand Down