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

Layout transient state refactor #6763

Closed
quicknir opened this issue Aug 4, 2016 · 9 comments
Closed

Layout transient state refactor #6763

quicknir opened this issue Aug 4, 2016 · 9 comments
Labels
- Mailling list - Discussion Spacemacs Layouts stale marked as a stale issue/pr (usually by a bot)

Comments

@quicknir
Copy link
Contributor

quicknir commented Aug 4, 2016

I was taking a real look at this for the first time today, and I think the current design is a bit off/inconsistent. First off, it's an enormous transient state, large enough that by default it's keybindings are hidden. Within that though, well over half of the commands immediately exit the transient state. The entire point though of a transient state as opposed to just a prefix binding, is that you don't immediately exit it. Second, the design where SPC l immediately enters the transient state is quite different from some of the most functionally similar prefixes, SPC w and SPC b.

These two together, to me at least, suggest a pretty straightforward course: change SPC l from a transient state, into a normal prefix key. Move all, or almost all, of the bindings inside the current transient state to normal bindings under this prefix. For instance, b which goes to a buffer associated with the current layout, would just become SPC l b. So the keystroke for it would be the same. Meanwhile, move the transient state to SPC l ., to be consistent with SPC w . and SPC b .. That transient state would then have many fewer commands; mostly those that don't exist and a few others deemed appropriate.

One random nitpick: is SPC l D supposed to be "close other layout" or "close other layouts"? The former would be a bit surprising; if you have more than two layouts it's not that clear which will be closed. I think we should aim for the latter functionality, and try to bring the same functionality to SPC w D and SPC b D (but that should be discusssed elsewhere, just mentioning it here for the sake of consistency).

Final thought, to go with my recent train of buffer/window/layout thoughts: I think we should reserve SPC F for frame related commands. Hopefully in the future we can support SPC F 2 to go to the second frame, and so on.

@travisbhartwell
Copy link
Contributor

I agree with @quicknir here. This is something I might try to tackle, especially if others concur and have related suggestions.

@bmag
Copy link
Contributor

bmag commented Aug 5, 2016

For reference, here are commands that exit the transient state (TS): (source)

  • <number>: switch to layout according to number
  • <return>: do nothing, used for exiting the TS
  • a: add buffer
  • A: import buffers (add all buffer from a layout)
  • b: spacemacs/persp-helm-mini (switch buffer)
  • D: close other layout (the user chooses interactively which one to close)
  • h: go to default layout
  • l: switch layout with helm/ivy
  • L: load layout from file
  • o: open custom layout
  • r: remove buffer from layout
  • R: rename layout
  • s: save all layouts to file
  • S: save some layout to file
  • t: show a buffer without adding it to current layout
  • w: workspaces TS
  • X: kill other layouts and their buffers (some or all of the other layouts, user chooses interactively which layout to kill)

And here are the commands that don't exit:

  • C-<number>: switch to layout according to number
  • <tab>: switch to last layout (similar to C-<tab>)
  • n/C-l: next layout
  • N/p/C-h: previous layout
  • x: kill current layout and its buffers

It does seem to me that we can move the TS SPC l .. I think that if we move the TS, most of its commands can become non-exiting commands and not be removed from the TS.
SPC l D is supposed to close just one layout.

@travisbhartwell not really related, but I noticed that SPC l b doesn't restrict the buffer choice to the current layout when Ivy is enabled instead of Helm. I haven't got to tracking it down yet.

@CestDiego are you still around? I think we'd benefit from hearing your input. (and you're still listed in the go-to persons page)

@bmag
Copy link
Contributor

bmag commented Aug 5, 2016

@travisbhartwell here's a related suggestion: we're missing a command to add all of the current project buffers to the current layout - this is very handy after opening a project layout with SPC p l. I have a naive command in my config that can be made more robust:

(defun bm-add-project-buffers-to-layout ()
  "Add all of current project's buffers to current layout.
Handy after creating project layout with [SPC p l]."
  (interactive)
  (let ((persp-switch-to-added-buffer nil))
    (mapc #'persp-add-buffer (projectile-project-buffers))))

This got me thinking that we're missing commands for the one-project-per-layout workflow, such as:

  • add project buffers to layout, or open project layout + add existing buffers
  • save project layout (shortcut for SPC l S + choose project-unique name), or "close project" that saves and closes the project's layout
  • load project layout (shortcut for SPC l L + choose project-unique name)
  • maybe re-wire SPC p p to load a project layout if there's already a saved one

@Andre0991
Copy link
Contributor

Andre0991 commented Sep 6, 2016

IMO tab should exit the TS. It makes sense in my workflow as I generally want to switch to the previous layout and edit it.

On the transient state x which-key issue, on the one hand the TS is too big, on the other I think using a TS looks nicer than which-key for this sort of thing, as it's possible to organize the commands by columns (which is currently not optimal actually, as there are just two columns, "Go to" and "Actions").

@syl20bnr
Copy link
Owner

syl20bnr commented Sep 6, 2016

But the whole point of the TS is to display the layouts. If we can find another way to show the layouts when pressing SPC l then we can remove the need of the TS and put it on SPC l ..

@syl20bnr
Copy link
Owner

syl20bnr commented Sep 6, 2016

Indeed SPC tab should exit the state.

@Andre0991
Copy link
Contributor

Also SPC l l exits the TS with helm, but it doesn't with ivy.

@quicknir
Copy link
Contributor Author

quicknir commented Oct 21, 2016

@travisbhartwell Are you still working on this? If not, I think I'll pick it up.

@Andre0991 I'm not really sure what's fundamentally different about layouts, compared to buffers or windows ("for this sort of thing"). I don't have any strong feelings about TS vs which-key to be honest. It's just a matter of consistency. In an alternative universe spacemacs could just have used TS's everywhere (or much more), but that's not what happened. It's pretty jarring to get used to which key for all of the prefixes, and suddenly get this TS for layouts.

@syl20bnr I'm not sure why it's strictly necessary to see the list of layers. When you do e.g. SPC b n, you don't see the list of any buffers at any point, you just hit the shortcut key and go to the next one. Why is that different from SPC l n? Don't get me wrong, I think showing the layouts is nice too, that's why should keep the transient state, I think though there are plenty of useful commands that can be placed under SPC l. For instance, switching to layout by name, or saving layout by name clearly don't require a list of layouts (since you get a list in helm). I think n and P p are fine too; people often will only have 2-3 layouts and will know their ordering, close current layout is fine (people can see what they're closing), see buffers in layout, jump to last layout (TAB), etc etc.

Final thought: there's a similar issue with workspaces, where currently there's just a transient state. I'd like to also have commands for workspaces from which key. If I just pull things into SPC l though, there will be conflicts: will SPC l d kill the current layout, or the current workspace? What about SPC l <num>? I see two solutions:

  • give workspaces their own prefix key, W. Then SPC W . will enter the transient state, SPC W d will delete current workspace, etc.
  • Keep workspace commands under SPC l, but prefix workspace commands with Ctrl to avoid collisions, and be consistent. So SPC l d and SPC l <N> will delete and switch to layouts, respectively, and SPC l C-d and SPC l C-<N> will delete and switch to workspaces, respectively.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!

@github-actions github-actions bot added the stale marked as a stale issue/pr (usually by a bot) label Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- Mailling list - Discussion Spacemacs Layouts stale marked as a stale issue/pr (usually by a bot)
Projects
None yet
Development

No branches or pull requests

6 participants