syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.64k stars 4.89k forks source link

Feature request: simpler api for customizing transient states #5405

Closed sooheon closed 1 month ago

sooheon commented 8 years ago

For example, paste transient state is defined:

(spacemacs|define-transient-state paste
        :title "Pasting Transient State"
        :doc "\n[%s(length kill-ring-yank-pointer)/%s(length kill-ring)] \
[_C-j_/_C-k_] cycles through yanked text, [_p_/_P_] pastes the same text above or \
below. Anything else exits."
        :bindings
        ("C-j" evil-paste-pop)
        ("C-k" evil-paste-pop-next)
        ("p" evil-paste-after)
        ("P" evil-paste-before)
        ("0" spacemacs//transient-state-0))

Is there a way for me to have in my config:

(spacemacs|customize-transient-state paste 
  :bindings ("C-j" nil) 
            ("C-k" nil) 
            ("M-n" evil-paste-pop) 
            ("M-p" evil-paste-pop-next))

I suppose I could duplicate the code and redefine it at the end of config load, but I'm hoping for a better approach.

nixmaniack commented 8 years ago

I think you're looking for this if it's just related to customizing bindings. https://github.com/syl20bnr/spacemacs/blob/895455d44cc935a4f1099886ac154522a5a18b26/layers/+completion/spacemacs-ivy/packages.el#L322-L328

sooheon commented 8 years ago

@nixmaniack great, that looks like exactly what I was looking for. Do you know if there's anything to also fix the docstring, then?

nixmaniack commented 8 years ago

As per my understanding, I don't think there's a way at this moment to customize docstring. You might want to create a feature request for it.

sooheon commented 8 years ago

Ideally, if the customization macro could be in the form I suggested above, one could arbitrarily adjust any part of the transient state transparently, rather than having to create separate functions to touch each piece. Can @justbur comment on the feasibility of this?

TheBB commented 8 years ago

You change the docstring by tweaking the variable (in this case) spacemacs/paste-transient-state/hint.

It's a form that is evaled.

justbur commented 8 years ago

@TheBB is right. Be aware that if you want the coloring you either need to do it manually or use hydra--format

syl20bnr commented 8 years ago

There is a macro now to apply hydra formatting to a string, I'm on mobile and cannot paste the name here for now.

sooheon commented 8 years ago

So to switch C-k/C-j bindings to C-n/C-p, I need:

(defun sooheon/post-init-evil ()
  (setq spacemacs-paste-transient-state-remove-bindings
        '("C-j" "C-k"))
  (setq spacemacs-paste-transient-state-add-bindings
        '(("C-n" evil-paste-pop)
          ("C-p" evil-paste-pop-next)))
  (setq spacemacs/paste-transient-state/hint
        '(concat
          #("Pasting Transient State\n"
            0 23 (face
                  spacemacs-transient-state-title-face))
          (concat
           (format
            "[%s/%s] [%s/%s] cycles through yanked text, [%s/%s] pastes the same text above or below. Anything else exits."
            (length kill-ring-yank-pointer)
            (length kill-ring)
            #("C-n"
              0 3 (face hydra-face-red))
            #("C-p"
              0 3 (face hydra-face-red))
            #("p" 0 1 (face hydra-face-red))
            #("P" 0 1 (face hydra-face-red)))
           "")
          nil
          nil)))

I can confirm that this does work, but isn't it a bit much? Or am I missing a more elegant way to do it?

justbur commented 8 years ago

You're adjusting the docstring and most of the bindings, so why not just redefine the transient state?

sooheon commented 8 years ago

That seems like the cleaner way. It's not really clear how to do that, though (as in where to add the redefinition). So far, I've tried it in dotspacemacs-user-config, inside a post-config-evil function in private layer, and inside pre-init-evil, dotspacemacs|use-package-add-hook evil :post-config as well. None have worked so far. Where should it go to take effect?

justbur commented 8 years ago

try this which should work anywhere in user-config (no hooks)

(spacemacs|define-transient-state my-paste
        :title "Pasting Transient State"
        :doc "\n[%s(length kill-ring-yank-pointer)/%s(length kill-ring)] \
[_M-n_/_M-p_] cycles through yanked text, [_p_/_P_] pastes the same text above or \
below. Anything else exits."
        :bindings
        ("M-n" evil-paste-pop)
        ("M-p" evil-paste-pop-next)
        ("p" evil-paste-after)
        ("P" evil-paste-before)
        ("0" spacemacs//transient-state-0))
(define-key evil-normal-state-map "p" 'spacemacs/my-paste-transient-state/evil-paste-after)
(define-key evil-normal-state-map "P" 'spacemacs/my-paste-transient-state/evil-paste-before)
justbur commented 8 years ago

The issue is that the transient-states are all loaded at once at the end of the initialization, so that your definition is probably getting overwritten. This was to allow people to change the key bindings ironically.

sooheon commented 8 years ago

@justbur thanks, my mistake was defining the same paste transient state, which I thought would be fine if "I am the one who overrides".

While my question is essentially answered, I do wonder what kind of impact all of these patches on patches on patches of elisp has for startup time...

sooheon commented 8 years ago

With the unspoken consensus that for now these two methods answer the issue, I'll close.

I do think that as we deal with editing styles molding to user preferences, having transient states be so rigid is a problem in the long term. It'd be amazing if transient state bindings could be as easily editable as evil-define-key for a given state.

sooheon commented 8 years ago

On second thought, no harm in keeping this around as context, and an open ended call for those with the elisp-foo to mull over this api. If we think of transient states as really first class states or "modes", it makes sense that customizing them should be as accessible as something like evil-define-key.

braham-snyder commented 7 years ago

here's an open PR taking a step in the right direction, from @madand – it adds a function that automatically sets or appends to spacemacs-%s-transient-state-add-bindings variables, depending on whether the variable already exists: https://github.com/syl20bnr/spacemacs/pull/8155

EDIT: I think this may be another good example of a transient state customization that would benefit greatly from something like spacemacs|customize-transient-state (as suggested in the OP): I'd simply like to remove the :on-exit property of the time-machine transient state (so that I can exit the hydra without quitting git-timemachine and its buffer).

github-actions[bot] commented 4 years ago

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!

madand commented 4 years ago

This issue is still valid and transient state machinery would certainly benefit from improved macrology around it.

Regarding @braham-snyder's issue with not being able to override :on-exit hook of time-machine transient state . I believe I figured out a simple way to solve it. We just need to convert every lisp form, that is used with :on-entry and :on-exit hook, into a defun. This way users will be able to redefine or advice them at will, no changes to define-transient-state required.

I will make PR with the mentioned change later this week.

--

Another issue I can think of is abuse of :dynamic-hint for implementing switching between (static) minimal and full hints. Note the same boilerplate code again and again:

https://github.com/syl20bnr/spacemacs/blob/09a9273e225c84ed9c7b6c5c4f5b50ab721ee57c/layers/%2Blang/html/funcs.el#L78-L91 https://github.com/syl20bnr/spacemacs/blob/09a9273e225c84ed9c7b6c5c4f5b50ab721ee57c/layers/%2Bframeworks/vue/funcs.el#L126-L139 https://github.com/syl20bnr/spacemacs/blob/09a9273e225c84ed9c7b6c5c4f5b50ab721ee57c/layers/%2Bdistributions/spacemacs-bootstrap/funcs.el#L202-L214

github-actions[bot] commented 3 years ago

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[bot] commented 4 months ago

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!