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

Implement C(-S)-tab buffer switch in Helm layer #14328

Conversation

dalanicolai
Copy link
Contributor

Equivalent to PR #14287 for ivy-layer

Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

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

Cool, thanks for adding this to helm too :)

@smile13241324 smile13241324 merged commit dc7b04d into syl20bnr:develop Feb 3, 2021
@dalanicolai
Copy link
Contributor Author

dalanicolai commented Feb 5, 2021

No problem. Personally I prefer to bind this functionality to C-j and C-k as in evil those bindings do not really add useful functionality (C-j in normal mode is somewhat equivalent to j in vim or o in evil, C-k is somewhat equivalent to dd in evil and enter digraph in vim). In Helm you can then set helm-follow-mode-persistent and toggle follow-mode.

For Ivy a solution to use C-j/k for buffer switching has been posted here.

   (defun trigger-ivy-switch-buffer ()
    (interactive)
    (minibuffer-with-setup-hook
        (lambda () (run-at-time nil nil #'ivy-toggle-calling))
      (ivy-switch-buffer)))

  (defun spacemacs/evilified-buffer-switch ()
    (interactive)
    (define-key evil-evilified-state-map (kbd "C-j") 'trigger-ivy-switch-buffer)
    (define-key evil-evilified-state-map (kbd "C-k") 'trigger-ivy-switch-buffer))

  (evil-global-set-key 'motion (kbd "C-j") 'trigger-ivy-switch-buffer)
  (evil-global-set-key 'motion (kbd "C-k") 'trigger-ivy-switch-buffer)
  (add-hook 'evil-evilified-state-entry-hook 'spacemacs/evilified-buffer-switch)

Maybe we could add these bindings to Spacemacs too, if you think that would be nice then let me know.

@lebensterben
Copy link
Contributor

lebensterben commented Feb 5, 2021

Bindings with TAB is more "expected" than j/k:

  • In many desktop environments, across Linux, MacOS, and Windows, <MODIFIER> TAB is the default keybinding to switch between active windows.
  • In Spacemacs, SPC TAB is the default keybinding to switch between the last two buffers.

A possible solution is to add a layer variable to helm and ivy:

  • When it's non-nil, SPC TAB is used to switch to next buffer and SPC S TAB is used to switch to last buffer.
  • When it's nil, it keeps the current default behaviour, that SPC TAB switches between last two buffers. And the new functionality in this commit is bound to C TAB and C S TAB.

The motivation for setting such variable to non-nil is that:

  • C TAB, and especially C S TAB, are really not comfortable to press on PC keyboard.
  • The original functionality of switching between last two buffers can be superseded by the new ones.

@dalanicolai
Copy link
Contributor Author

dalanicolai commented Feb 5, 2021

@lebensterben Well it is not about expectation (although that is why I implemented the C-tab). It is about offering and suggesting to users the most convenient workflow. I think there is no need to not improve things because of conventions/expectations (although it is important to consider such improvements carefully).

Well, otherwise users can find the suggestion here anyway.

EDIT
Also you suggest an alternative because C TAB and C S TAB would not be comfortable... well so that is why I suggest the C-j/k alternative which is very comfortable. It fits well with the additionally required C-l (or RET) to select the buffer. Also I think users do/should not really have keybinding expectations other than vim's when they start using Spacemacs.

dalanicolai added a commit to dalanicolai/pdf-continuous-scroll-mode.el that referenced this pull request Feb 11, 2021
These bindings overlap with my new style of recent buffer cycling.
See [here](syl20bnr/spacemacs#14328 (comment)).
@dalanicolai dalanicolai deleted the add_ctrl-tab_buffer_switch_to_helm_layer branch February 15, 2021 10:04
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.

4 participants