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

Added zoom & max-width #9838

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lizclipse
Copy link
Contributor

@lizclipse lizclipse commented Mar 8, 2024

This is based on #3239 (and would close #3203 & close #2262), however, I've made the following changes:

  • Renamed the mode to 'zen' as that's what VSCode called it (and is a fine name I guess) (it's the editor I used before moving to Helix)
  • Moved the flag to the view tree (instead of each view) and have it target the currently-focused view
  • Made it centre all of the child views and only render the currently focused one
  • Config option has been replaced with one that allows setting the maximum width

Some of these no longer apply after these changes

image

@mlemesle
Copy link
Contributor

mlemesle commented Mar 8, 2024

Hey there @lizclipse !
That's a super cool feature that I will definitely use if merged! Thanks for your work!!
Quick thought about your feature. What do you think about a bool in the configuration that, if set to true and helix is started with a single file, will directly open it in zen mode?

@lizclipse
Copy link
Contributor Author

lizclipse commented Mar 9, 2024

That sounds straight-forward enough too add, something like:

[editor.zen-view]
auto-enter = "single-file"

?

Not sure exactly on the option name, but I think that suggestion is self-evident enough?

Thinking about it a bit, though, I believe that having a slightly more flexible option which allows the following enums:

"off"           // Never enter zen-view automatically
"single-file"   // Only automatically enter if Helix is opened with 1 file
"multiple-file" // Only automatically enter if Helix is opened with 1 or more files
"always"        // Always start in zen-view

Could be even more useful? Default it to "off", and then you can decide to enter automatically based on how you use it. e.g. I'd probably set it to "always" since my monitor is fairly spacious and I like to edit text down the middle if I can.

Some input from a maintainer might be good here, since I don't want to be adding new options haphazardly.

@mlemesle
Copy link
Contributor

mlemesle commented Mar 9, 2024

That looks awesome!
I do have a wide monitor as well and I currently use a custom "zen mode" where I just toggle insanely wide gutter space to center the view...
Can't wait to use your feature, thanks again for your work and patience with me!

@lizclipse
Copy link
Contributor Author

No worries, I decided to add this because I want it as well haha. I've added the auto-enter option in its own commit so it can be scrapped easily if it's decided not worth it.

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Mar 9, 2024
@David-Else
Copy link
Contributor

David-Else commented Mar 10, 2024

There was a cool closed pull request #3309 by @the-mikedavis a while back that added a space t toggle setting, I have been using my own limited version of the concept:

[keys.normal.space.t]
f = ":toggle auto-format"
i = ":toggle lsp.display-inlay-hints"
s = ":toggle soft-wrap.enable"
n = ":toggle whitespace.render.newline all none"
z = ":toggle gutters.line-numbers.min-width 48 3" # limited zen mode, doesn't work well disabling line numbers

What do people think about bringing this idea back again and making zen mode part of it?

Also, how about zen mode toggling off the status line and line numbers by default?

@lizclipse
Copy link
Contributor Author

I like the idea of making the line numbers invisible when in zen, although I think that should also be an option since I would prefer to keep them on personally (default to off, though).

The toggle setting mode is cool, although I'd be wary of adding it in this PR, since the last one was closed for what seems to be a good reason. It does seem a good place for this option to go, though, and I think having the line-number toggle under there so you can switch it on-and-off while in zen is probably a good idea.

Would need to consider the interaction between the line-number toggle and zen, as I'm not sure exactly what should happen if you toggle them off in normal view and then switch into zen with the config set to enable line-numbers. Should it follow what the user has set while running, or keep to the config? If you toggle line-numbers off, switch into zen with the config set to not show them, toggle them on, then switch out of zen, should it keep line-numbers off or turn them back on again? Essentially, how separate of a mode should zen be?

My instinct says that it should be quite separate, such that toggling line-numbers in normal or zen shouldn't affect the other mode (and should be persisted while the app is running, such that toggling line-numbers and switching back-and-forth would return back to what was set), but some input on that for the consensus would be good.

@lizclipse lizclipse force-pushed the zen-view branch 3 times, most recently from ed357c8 to ad7ad26 Compare March 16, 2024 02:42
@lizclipse
Copy link
Contributor Author

I've added a new gutters option that allows setting the gutters layout for when in zen-view. By default it's empty, but you can set it up to be identical to the normal gutters if you want (which I'll likely do for myself). Only the layout of the zen-view gutters is configurable, everything else used the base config currently, as it'd require either an unpleasant bodge or a lot of refactoring. Once #9318 is in I can make another PR making the rest of the gutters config separate between normal and zen view.

@lizclipse lizclipse force-pushed the zen-view branch 2 times, most recently from f4d9f6d to 6bae246 Compare March 18, 2024 15:25
@lizclipse
Copy link
Contributor Author

lizclipse commented Mar 18, 2024

From a conversation on Matrix, I've added a 'zoom' mode that uses the same internals as the zen view but doesn't overwrite the width or gutters, meaning it can now hit more open issues on the head. I've also tweaked the max-width calculation to ignore it if set to 0

Edit: oh, I also moved the key binds to the window menu instead, since that made more sense. It's now z for zoom (eg <space>wz and Z for zen (eg C-wZ) (both binds are in each menu these are just egs).

@lizclipse lizclipse changed the title Added zen-view mode Added zoom & zen-view mode Mar 18, 2024
@archseer archseer added this to the next milestone Mar 19, 2024
@lizclipse lizclipse force-pushed the zen-view branch 3 times, most recently from e971c7c to ccf0914 Compare March 26, 2024 15:15
@David-Else
Copy link
Contributor

Hi, thanks for this PR! I think 150 is too long for the default max-width, VS Code sets it to 120 (before the scroll bar appears), You can validate that by pasting in this 120 char string into https://vscode.dev/ :

abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()abcdefghijklmnoABCDEFGHIJKLMNO0123456789pqrs

In VS Code, to toggle Zen Mode you can use the shortcut:

  • On Windows and Linux: Ctrl + K followed by Z
  • On macOS: Cmd + K followed by Z

What do people think? If the default max-width is too long then people won't be getting their code in the center of the screen, but too far to the left.

@lizclipse
Copy link
Contributor Author

@David-Else Funnily enough, the default in-code was already 120! I must've changed it for this exact reason and forgotten to update the config docs.

@paulcomte
Copy link

paulcomte commented Apr 7, 2024

Can we add a different zoom level when entering the zen-mode ?
I'd like to use this feature for possible videos / streams, so by entering into zen-mode I can focus onto the specific code, and then when I leave the zen-mode the zoom level goes back to normal?

Edit: Just saw that there's the toggle zoom feature too, but can we add in the config a variable to auto-toggle zoom?

@paulcomte
Copy link

Oh also, a nice feature, would be to select lines of code and enter the zen mode, and it will scale the window according to these selected lines of code and only display them.
I don't know how long it would take to code though.

@lizclipse
Copy link
Contributor Author

@paulcomte I don't believe that would work purely in helix, since the text size is entirely driven by the terminal and I'm not sure how you would control it without direct integration. You might be able to do this sort of thing via a key-bind that does multiple things, such that it enters zen mode and executes a shell command at the same time, but that would be a setup-specific thing.

@lizclipse lizclipse changed the title Added zoom & zen-view mode Added zoom & max-width Apr 12, 2024
@lizclipse
Copy link
Contributor Author

I asked for some update on the status of review over on Matrix, and I got some good feedback that I've now implemented.

Essentially, the zen-mode is now dropped and the changes reduced down to an MVP to make inclusion easier. All this PR adds now is 2 things:

  • A view can be zoomed to take up the entire area
  • The area can have a max width set

These 2 combined lets you simulate the zen-view using something like the following config:

[keys.normal."+"]
# Creates a basic 'zen-mode' similar to VSCode's
z = ["toggle_zoom", ":set-max-width 120", ":set gutters.layout []"]
Z = ["toggle_zoom", ":set-max-width 0", ':set gutters.layout ["diagnostics","spacer","line-numbers","spacer","diff"]']

It's not perfect, as you need 2 key binds to do what used to be 1, but it comes about naturally from the much more simple implementation. Once the plugin system is finally in, you should also be able to replace this config with a single key bind that toggles between the 2 depending on the zoom/max-width state.

@lizclipse
Copy link
Contributor Author

Just fixed the build issue

@David-Else
Copy link
Contributor

Thanks for this work, nice to get a minimal version and get in merged asap!

Unfortunately when i use the :set-max-width on a single file that has no other split windows I get a nasty grey line appear on the right hand side of the screen as if it were a split. Can we get rid of it?:

image
image

@lizclipse
Copy link
Contributor Author

I've pushed a fix for it now

@lizclipse
Copy link
Contributor Author

lizclipse commented Apr 19, 2024

Rebased to fix the confict

Edit: again

@@ -360,7 +366,27 @@ impl Tree {
return;
}

self.stack.push((self.root, self.area));
let area = if self.max_width > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really useful behavior for non-zoom state? I would expect this option to affect only zoomed window.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is, since it's a fairly distinct feature and could have possible uses outside just in a zoomed mode, such as for a wide-screen, or for centring the editor with vertical splits. It's not guaranteed that this will be set when zoomed, either, so it's not always going to be used. It also does not have a config option (they've been removed from this PR now), so it won't ever be set when helix starts anyway.

My main feeling with it, though, is that if it was made zoom-only, then the question would start being asked "why not when not zoomed?" and I wouldn't have a good answer. Having it work outside the zoom makes the feature more flexible, which in turn makes it more useful.

@lizclipse lizclipse force-pushed the zen-view branch 2 times, most recently from c4673a0 to ca55729 Compare May 10, 2024 14:43
@David-Else
Copy link
Contributor

Could this be reviewed and merged? Some kind of centering of the text is needed for writing without neck injury. Cheers :)

@David-Else
Copy link
Contributor

Hi, thanks for this PR! I think 150 is too long for the default max-width, VS Code sets it to 120 (before the scroll bar appears), You can validate that by pasting in this 120 char string into https://vscode.dev/ :

abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()abcdefghijklmnoABCDEFGHIJKLMNO0123456789pqrs

In VS Code, to toggle Zen Mode you can use the shortcut:

* On Windows and Linux: `Ctrl` + `K` followed by `Z`

* On macOS: `Cmd` + `K` followed by `Z`

What do people think? If the default max-width is too long then people won't be getting their code in the center of the screen, but too far to the left.

I checked again and it is now 94 characters length in Zen Mode! I don't know if they changed it or something changed on my system. On reflection I think 120 is too long and it should be 80 or 100.

@lizclipse
Copy link
Contributor Author

I've been away from projects for a while, and I think I got a bit bored of rebasing and working on things (skill issue on my part)... I'll try to put some time aside in the next few days to update this PR so that it's in a mergeable state again, but it actually shouldnt be that bad.

I've personally been using this PR merged into a custom branch with some other PRs for myself, so I can say that it's still working pretty much I think, just needs cleaning. There is a bug with mouse clicks currently, which is that if you have say 3 vertical splits, and then zoom in on the middle, any clicks will be directed to the left-most split and the focus is changed over. I think it's because the current implementation centers all splits into the same location and then selectively renders the focused one, but the mouse handling doesnt know about it.

Not sure if the way I've done this is the best, but I'll fix that but when I come to clean things up again if there isnt an obviously better way to do things.

I checked again and it is now 94 characters length in Zen Mode! I don't know if they changed it or something changed on my system. On reflection I think 120 is too long and it should be 80 or 100.

I dont really mind, I think I liked 120 because it gave some space on the right-hand-side to exceed the usual 80/100 line length limit (eg for a longer comment), but I can set the default to whatever makes the most sense for most people and edit my own config. 100 feels like a nice round number close to VSCode's and hits a common line length limit.

@David-Else
Copy link
Contributor

Since Helix 25.01 we now we have diagnostics appearing on the right of soft wrapped lines, can they flow to the edge of the screen and outside of the zen window in this PR?

Just as a reference, this is with soft wrap at 80 chars and a make shift zen mode of 80 chars. Diagnostics can go to the edges with this hack.

[keys.normal.space.t]
z = ":toggle gutters.line-numbers.min-width 52 3"

image

Maybe make the line length a config value?

@lizclipse lizclipse force-pushed the zen-view branch 2 times, most recently from 96875ef to 0bb11da Compare January 22, 2025 00:05
@lizclipse
Copy link
Contributor Author

I think it's been that long since I last looked at this that I forgot that I removed the config, so there isnt a value to set. It was reworked into a command, :set-max-width, and that will set it at runtime.

I've just rebased my changes, and included (a) a fix for the mouse click bug, and (2) a 2nd argument to :set-max-width <width> [alt width] to allow proper toggling. The way I made that work is that if the current max width is different to the first argument, then that value is set, otherwise it will set the max width to the 2nd argument. The :toggle command only works on config values, and I dont think there was any other way to replicate that behaviour for a command, so this instead allows it to work as a single toggle keybind.

Havent added the config option back in, but I could do it that works better? When I did the rework before it was because adding a new config option was a fairly big thing, and this feature doesnt really need it, so this way makes it more simple and easier to get in. Not sure how much these things still apply, but I've assumed they do for now.

@noahbald
Copy link

Havent added the config option back in, but I could do it that works better?

Personally I’d rather use it as a set it and forget it situation since I prefer to always have the buffer centered.
I’m not sure how it works, but as a config option could it call the set-max-width command on startup? Maybe that would help make it simpler?

@David-Else
Copy link
Contributor

Awesome work! Few problems:

  1. The ugly vertical line is back again that you fixed before.
  2. When set to 80 max width it actually 73 chars (see bottom right for char count). I understand this might include the gutter, but in the case below I think the gutter takes 3 characters, so that would be 76 total? Could you take the gutter into account and make max width the actual number of characters available, the same as soft wrap?

image

  1. In you z = ["toggle_zoom", ":set-max-width 120 0", ":set gutters.layout []"] example the line numbers stay off when unzoomed.

@David-Else
Copy link
Contributor

One more thing, I think it would be better if the error messages could extend outside the window to the right rather than be cut off. This is how things look with the current hack approach:

z   = ":toggle gutters.line-numbers.min-width 52 3"

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements
Projects
None yet
8 participants