Did anybody else use surround.vim? I've missed it ever since I switched to Emacs. Finally, last night, I got up the energy to hack something together: http://gist.github.com/487846 Things that work: * Changes surrounding enclosures * add surrounding enclosures * delete sounding enclosures Not working yet: * Surround region with a tag * non-visual surrounding of regions (IE: ysiw") * repeat surround modifications I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach. Feedback welcome, but mostly, I hope this is enjoyed by somebody else. Tim
Hi Tim, I'm interested in it and bookmarked your code. Don't have time at the moment to look at it. I recommend that you consider creating a real project. I think it will be picked up by someone else. If not today then tomorrow. Marc Weber
On Fri, Jul 23, 2010 at 1:22 PM, Marc Weber <[email protected]> wrote:
Hi Tim,
I'm interested in it and bookmarked your code. Don't have time at the moment to look at it.
I recommend that you consider creating a real project. I think it will be picked up by someone else. If not today then tomorrow.
Marc Weber
I will probably do that after it sits on my hard drive for a while. :-) Tim
Tim Harper <[email protected]> writes:
Did anybody else use surround.vim?
I've missed it ever since I switched to Emacs. Finally, last night, I got up the energy to hack something together:
Hey, this is nice, thanks! I've been somewhat missing surround.vim, too, but haven't got up the energy. ;-) It's late here and the energy is *cough* still lacking, but while reading your code, I've fixed some (of what seemed to me to be) typos and style issues. The result of my reading is attached. A few comments on that matter: - docstrings: Section D.6 of the Elisp manual (Appendix) is the canonical reference here. - Using asterisks for *variables* is also discouraged, but I left it as is, because it is actually a nice and sensible convention ;-P (disregarding the fact that Elisp is not sensible and all variables are dynamic) and this is just a hack so far anyway. - You used the `map' function provided by the cl package, but didn't `require' it; but you actually don't need cl at all -- you can (should) use `mapcar' instead, and simpler versions of both `push' and `pop' are defined in subr.el. - I wonder why you were setting the KILLFLAG arg to `delete-char'. Is saving random characters on the kill ring really your intention? - In `vimpulse-surround-pairs' docstring you say: "This only has effect on creating new surround pairs, not deleting them." I have no idea what you mean here, could you try to explain it some more?
Things that work:
* Changes surrounding enclosures * add surrounding enclosures * delete sounding enclosures
Not working yet:
* Surround region with a tag * non-visual surrounding of regions (IE: ysiw") * repeat surround modifications
I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach.
Feedback welcome, but mostly, I hope this is enjoyed by somebody else.
It surely is, thanks again and good night, Štěpán
Hey Štěpán, I integrated your changes and made an "official" project: http://github.com/timcharper/vimpulse-surround.el Thanks for your corrections, many good ones in there! Tim On Fri, Jul 23, 2010 at 4:28 PM, Štěpán Němec <[email protected]> wrote:
Tim Harper <[email protected]> writes:
Did anybody else use surround.vim?
I've missed it ever since I switched to Emacs. Finally, last night, I got up the energy to hack something together:
Hey, this is nice, thanks! I've been somewhat missing surround.vim, too, but haven't got up the energy. ;-)
It's late here and the energy is *cough* still lacking, but while reading your code, I've fixed some (of what seemed to me to be) typos and style issues. The result of my reading is attached.
A few comments on that matter: - docstrings: Section D.6 of the Elisp manual (Appendix) is the canonical reference here. - Using asterisks for *variables* is also discouraged, but I left it as is, because it is actually a nice and sensible convention ;-P (disregarding the fact that Elisp is not sensible and all variables are dynamic) and this is just a hack so far anyway. - You used the `map' function provided by the cl package, but didn't `require' it; but you actually don't need cl at all -- you can (should) use `mapcar' instead, and simpler versions of both `push' and `pop' are defined in subr.el. - I wonder why you were setting the KILLFLAG arg to `delete-char'. Is saving random characters on the kill ring really your intention? - In `vimpulse-surround-pairs' docstring you say: "This only has effect on creating new surround pairs, not deleting them." I have no idea what you mean here, could you try to explain it some more?
Things that work:
* Changes surrounding enclosures * add surrounding enclosures * delete sounding enclosures
Not working yet:
* Surround region with a tag * non-visual surrounding of regions (IE: ysiw") * repeat surround modifications
I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach.
Feedback welcome, but mostly, I hope this is enjoyed by somebody else.
It surely is, thanks again and good night,
Štěpán
_______________________________________________ implementations-list mailing list [email protected] https://lists.ourproject.org/cgi-bin/mailman/listinfo/implementations-list
Tim Harper <[email protected]> writes:
Hey Štěpán,
I integrated your changes and made an "official" project:
Cool.
Thanks for your corrections, many good ones in there!
Oh yeah, I thought they were all good! Turns out they weren't... a fix attached, sorry. Štěpán
On 2010-07-23 20:46, Tim Harper wrote:
I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach.
There isn't a perfect way, not even in Vim itself. surround.vim does this: nmap ds <Plug>Dsurround Which corresponds to making a "careful" binding in Vimpulse -- that is, one of these lines: (vimpulse-map "ds" 'vimpulse-surround-delete) (vimpulse-make-careful-binding viper-vi-basic-map "ds" 'vimpulse-surround-delete) Which will execute `vimpulse-surround-delete' when the user hits "ds" and `vimpulse-delete' when the user hits "d" + any other key than "s". The fatal flaw with this approach, both in Emacs and Vim, is that the keystrokes are read /before/ Operator-Pending mode is entered, so the half-height "operator cursor" is permanently lost. Second approach: write a "null motion" (one that returns an empty range) and bind it to "s" in Operator-Pending mode: (defun vimpulse-surround-delete (&optional arg) (interactive) ;; Delete surrounding characters ... (viper-move-marker-locally 'viper-com-point (point))) (define-key vimpulse-operator-basic-map "s" 'vimpulse-surround-delete) This will preserve the half-height cursor. The call to `viper-move-marker-locally' ensures that the starting point is equivalent to the ending point -- e.g., (vimpulse-delete 45 45) -- hence no actual deletion takes place. Third approach: we could make the function bodies of `vimpulse-delete' and `vimpulse-delete' contingent on a "cancellation variable". A pseudo-motion like above could set this to t during its execution to inhibit the deletion code of `vimpulse-delete', the change code of `vimpulse-change', and so on. (Conceivably, `vimpulse-range' itself could cancel further code execution with `keyboard-quit', but this would probably be noisy.) Fourth approach: stay with the delete-or-surround DWIM functions, but bind them in a separate, toggleable keymap rather than overwriting the basic bindings. Like this: (define-minor-mode surround-mode) (vimpulse-define-key 'surround-mode 'vi-state "d" 'vimpulse-delete-surround-or-delete) (vimpulse-define-key 'surround-mode 'vi-state "c" 'vimpulse-change-surround-or-change) Then you can enable surround-mode in certain buffers as appropriate, or turn it into a global minor mode. -- Vegard
Vegard Øye <[email protected]> writes:
On 2010-07-23 20:46, Tim Harper wrote:
I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach.
Fourth approach: stay with the delete-or-surround DWIM functions, but bind them in a separate, toggleable keymap rather than overwriting the basic bindings. Like this:
(define-minor-mode surround-mode)
(vimpulse-define-key 'surround-mode 'vi-state "d" vimpulse-delete-surround-or-delete) (vimpulse-define-key 'surround-mode 'vi-state "c" vimpulse-change-surround-or-change)
Then you can enable surround-mode in certain buffers as appropriate, or turn it into a global minor mode.
(With the usual disclaimer about my very limited knowledge of Vimpulse and Viper,) this strikes me as the cleanest one -- it just looks so Right. :-D Defining this kind of minor modes (possibly global -- I assume that would work, too?) is perhaps The Way for writing Vimpulse plug-ins. (And to put some more ashes on my head, this seems to be precisely the kind of thing for which you implemented these binding commands recently and which I failed to appreciate at the time.) Thanks for the insight! Štěpán
On 2010-07-25 00:29, ??t??p??n N??mec wrote:
Vegard ??ye <[email protected]> writes:
(define-minor-mode surround-mode)
(vimpulse-define-key 'surround-mode 'vi-state "d" vimpulse-delete-surround-or-delete) (vimpulse-define-key 'surround-mode 'vi-state "c" vimpulse-change-surround-or-change)
(With the usual disclaimer about my very limited knowledge of Vimpulse and Viper,) this strikes me as the cleanest one -- it just looks so Right. :-D Defining this kind of minor modes (possibly global -- I assume that would work, too?) is perhaps The Way for writing Vimpulse plug-ins.
Yes, it should work with globalized minor modes as well -- the code just checks the mode variable and, if t,[1] enables the appropriate state bindings associated with it. It checks the `major-mode' variable too, so one could also modify/define major modes this way. In essence, state bindings become just another "property" that an Emacs mode can have. [1] Strictly speaking: unless nil.
Thanks for the insight!
You're welcome :) -- Vegard
On Sat, Jul 24, 2010 at 3:23 PM, Vegard Øye <[email protected]> wrote:
On 2010-07-23 20:46, Tim Harper wrote:
I'm not extremely happy with my approach on how I'm looking into the delete/change commands. Basically, I'm hijacking them in doing an advise (as in defadvice) approach.
There isn't a perfect way, not even in Vim itself. surround.vim does this:
nmap ds <Plug>Dsurround
Which corresponds to making a "careful" binding in Vimpulse -- that is, one of these lines:
(vimpulse-map "ds" 'vimpulse-surround-delete) (vimpulse-make-careful-binding viper-vi-basic-map "ds" 'vimpulse-surround-delete)
Which will execute `vimpulse-surround-delete' when the user hits "ds" and `vimpulse-delete' when the user hits "d" + any other key than "s". The fatal flaw with this approach, both in Emacs and Vim, is that the keystrokes are read /before/ Operator-Pending mode is entered, so the half-height "operator cursor" is permanently lost.
Second approach: write a "null motion" (one that returns an empty range) and bind it to "s" in Operator-Pending mode:
(defun vimpulse-surround-delete (&optional arg) (interactive) ;; Delete surrounding characters ... (viper-move-marker-locally 'viper-com-point (point)))
(define-key vimpulse-operator-basic-map "s" 'vimpulse-surround-delete)
This will preserve the half-height cursor. The call to `viper-move-marker-locally' ensures that the starting point is equivalent to the ending point -- e.g., (vimpulse-delete 45 45) -- hence no actual deletion takes place.
Third approach: we could make the function bodies of `vimpulse-delete' and `vimpulse-delete' contingent on a "cancellation variable". A pseudo-motion like above could set this to t during its execution to inhibit the deletion code of `vimpulse-delete', the change code of `vimpulse-change', and so on. (Conceivably, `vimpulse-range' itself could cancel further code execution with `keyboard-quit', but this would probably be noisy.)
Fourth approach: stay with the delete-or-surround DWIM functions, but bind them in a separate, toggleable keymap rather than overwriting the basic bindings. Like this:
(define-minor-mode surround-mode)
(vimpulse-define-key 'surround-mode 'vi-state "d" 'vimpulse-delete-surround-or-delete) (vimpulse-define-key 'surround-mode 'vi-state "c" 'vimpulse-change-surround-or-change)
Then you can enable surround-mode in certain buffers as appropriate, or turn it into a global minor mode.
Approach 4 does seem pretty good (better than the current behavior of hijacking another keymap). What about option 5: advise vimpulse-range to set the *vimpulse-surrounding* variable to nil before calling it advise vimpulse-delete with an around filter, if *vimpulse-surrounding* is non-nil, then call vimpulse-surround-delete, otherwise invoke as normal with ad-do-it Benefits: * If for some sick reason one decides to remap vimpulse-delete, they can do so with less complexity of having to remap vimpulse-delete-surround-or-delete (maybe dvorak users would want to?) * Multiple plugins can chain in this way. Both the current solution and approach 4 have the limitation that if another plugin wants to do something similar, it will conflict with vimpulse-surround. Drawbacks: * We are depending on side-effect value from a previous function call. This adds complexity and can make things difficult to follow.
Tim Harper <[email protected]> writes:
On Sat, Jul 24, 2010 at 3:23 PM, Vegard Øye <[email protected]> wrote:
[...]
Fourth approach: stay with the delete-or-surround DWIM functions, but bind them in a separate, toggleable keymap rather than overwriting the basic bindings. Like this:
(define-minor-mode surround-mode)
(vimpulse-define-key 'surround-mode 'vi-state "d" vimpulse-delete-surround-or-delete) (vimpulse-define-key 'surround-mode 'vi-state "c" vimpulse-change-surround-or-change)
Then you can enable surround-mode in certain buffers as appropriate, or turn it into a global minor mode.
Approach 4 does seem pretty good (better than the current behavior of hijacking another keymap). What about option 5:
advise vimpulse-range to set the *vimpulse-surrounding* variable to nil before calling it advise vimpulse-delete with an around filter, if *vimpulse-surrounding* is non-nil, then call vimpulse-surround-delete, otherwise invoke as normal with ad-do-it
Sounds good. It would also still be possible to define a minor mode and enable/disable the advice cleanly (albeit not buffer-locally, but that's not a problem I guess). [...]
Drawbacks:
* We are depending on side-effect value from a previous function call. This adds complexity and can make things difficult to follow.
I'm not sure I understand -- how is that worse than your current *vimpulse-surrounding* ugliness where you always initialise it to nil and set it during the `vimpulse-range' call? Or are you speaking about precisely that? Štěpán
participants (4)
-
Marc Weber -
Tim Harper -
Vegard Øye -
Štěpán Němec