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 horizontal and vertical split scratch buffers #1763

Merged

Conversation

jharrilim
Copy link
Contributor

Add horizontal and vertical split scratch buffers

This change proposes the addition of two new commands to open scratch buffers in splits. It works out to be very similar to :hsplit src/newfilename.rs where newfilename.rs is a file that doesn't exist, but with the intention of not actually creating a file when you are done with it.

@archseer
Copy link
Member

archseer commented Mar 7, 2022

:new already splits horizontally, so you just need to add the equivalent of vim's :vnew.

@jharrilim
Copy link
Contributor Author

:new already splits horizontally, so you just need to add the equivalent of vim's :vnew.

:new in vim splits horizontally, but in helix it replaces the current view:

fn new_file(
cx: &mut compositor::Context,
_args: &[Cow<str>],
_event: PromptEvent,
) -> anyhow::Result<()> {
cx.editor.new_file(Action::Replace);
Ok(())
}

Should we change :new to split horizontally then with the addition of :vnew?

@archseer
Copy link
Member

archseer commented Mar 7, 2022

Ah yeah, sorry I mixed it up. I'd either change :new to open a horizontal split or add :hnew.

@jharrilim
Copy link
Contributor Author

Went with the second suggestion and changed the aliases to hnew and vnew. The flexibility is nice!

@archseer
Copy link
Member

archseer commented Mar 7, 2022

Sorry, one more rebase is needed: I split the typable commands out to a separate file

@jharrilim jharrilim force-pushed the feat/add-new-splits/jharrilim branch from 7f457f2 to 33faa2a Compare March 7, 2022 19:09
Make subcommand name more descriptive

Fix vsplit completer

Run cargo xtask docgen
@jharrilim jharrilim force-pushed the feat/add-new-splits/jharrilim branch from 33faa2a to 6c717e4 Compare March 7, 2022 19:11
@jharrilim
Copy link
Contributor Author

No problem! updated the PR

@@ -682,6 +682,10 @@ impl Default for Keymaps {
"C-j" | "j" | "down" => jump_view_down,
"C-k" | "k" | "up" => jump_view_up,
"C-l" | "l" | "right" => jump_view_right,
"n" => { "New split scratch buffer"
Copy link
Member

Choose a reason for hiding this comment

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

"Scratch buffer in new split"

I'm also not sure how I feel about these mappings, we can leave them in for now 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 know doom emacs for this space w n will split vertical, not sure if we want so many layers here.

Copy link
Member

@the-mikedavis the-mikedavis Mar 10, 2022

Choose a reason for hiding this comment

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

I am probably biased as I use the scratchpad rarely but I think this would be fine without default keybindings at all. I think the typable commands are more appropriate, especially given how many keypresses these keybindings take.

Copy link
Contributor Author

@jharrilim jharrilim Mar 11, 2022

Choose a reason for hiding this comment

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

It's definitely a tad deep! My argument for keeping it in here would be that when I first installed helix and played around with it, I was very much inclined to pressing space and discovering commands through the dialog. I naturally began to start using space w to find window commands, and felt like this was an appropriate place to put them since the other split commands were here too. Once I got into the flow of navigating this way, it didn't feel too deep IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm good point, the discoverability is nice with the space popups. Although it also possible to discover things through the command picker (#1400), but I don't remember if that shows typable commands. I don't have strong feelings about the bindings, I would feel ok with these 👍

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@archseer archseer merged commit c0dbd6d into helix-editor:master Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants