<span class="gmail_quote">On 10/23/06, <b class="gmail_sendername">Brad Beveridge</b> <<a href="mailto:brad.beveridge@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">brad.beveridge@gmail.com
</a>> wrote:</span><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
I've just had a quick read of the patch, here are some comments:<br>delete-previous-char: if you look at the primitive DELETE-CHARS<br>function, there is an option to delete to the left or the right of the<br>cursor.<br>
<br> It looks like we do a lot of moving the cursor by one line, lets make<br>a new function to advance/retreat by a line and put it in cursor.lisp.<br> See this Emacs reference page for ideas for function names<br><a href="http://www.cse.huji.ac.il/support/emacs/elisp-help/elisp-manref/elisp_30.html#SEC478" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">
http://www.cse.huji.ac.il/support/emacs/elisp-help/elisp-manref/elisp_30.html#SEC478</a><br><br>I don't like reusing other commands inside commands, ie in<br>change-region. What we should do is create a DELETE-REGION function,
<br>and have command-d-motion and change-region call that. Same for<br>subst-char, I'd rather use the primitive (DELETE-CHARS), we may want<br>to change/remove COMMAND-X later.<br><br>Really good, probably I'm going to have to become more disiplined in
<br>how I code now though :) I can easily see myself writing exactly what<br>you did :)<br><br>If you want to send another patch (don't bother unrecording, just add<br>to it) feel free, otherwise I'll make the changes when I roll it in.
<br><br>Cheers<br>Brad<br><br>On 23/10/06, Brad Beveridge <<a href="mailto:brad.beveridge@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">brad.beveridge@gmail.com</a>> wrote:<br>> On 23/10/06,
<a href="mailto:laynor@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">laynor@gmail.com</a> < <a href="mailto:laynor@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">
laynor@gmail.com</a>> wrote:<br>> > Mon Oct 23 17:59:36 CEST 2006 <a href="mailto:laynor@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">laynor@gmail.com</a><br>> > * normal mode commands, cc, c<motion>, s, S, -, +, J, X
<br>> > Added some normal mode commands:<br>> > "cc": change the active line<br>> > "c<motion>": change the region defined by <motion><br>> > "s": subst char, deletes a character like "x" and then goes to insert mode
<br>> > "S": subst line, same as "cc"<br>> > "-": goto previous line beginning, moves the cursor to previous line at the 1st column<br>> > "+": goto next line beginning, moves the cursor to the next line at the 1st column
<br>> > "X": deletes previous char (like backspace in insert mode, but it works in normal mode)<br>> > "J": Joins the active line and the next line, It deserves to be fixed, because the undo is
<br>> > broken (it popups the debugger when the command has no effect (when applied on the last line))<br>> ><br>> > ---- Strange Behaviors (with respect to vim) ----<br>> > "c<motion>" doesnt behave the same as vim, just try it in gvim and in vial, it's faster than
<br>> > trying to explain it here. In particular, "cj" doesnt open a new line after deleting,<br>> > and cw eats the backspace that separates words.<br>> > ---- Broken behavior ----
<br>> > join-lines-command ("J") breaks the undo. To reproduce this behavior, just go to the last line of a file<br>> > and press J, and then undo.<br>><br>> Nice! I'll look at it tonight after work and roll it in. I think
<br>> I'll start a couple of new files:<br>> vim-differences.txt - a list of differences to Vim<br>> known-bugs.txt - known bugs in Vial<br>><br>> Thanks a lot for the patch!<br>><br>> Cheers<br>> Brad
<br>><br>_______________________________________________<br>vial-devel mailing list<br><a href="mailto:vial-devel@common-lisp.net" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">vial-devel@common-lisp.net
</a><br><a href="http://common-lisp.net/cgi-bin/mailman/listinfo/vial-devel" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://common-lisp.net/cgi-bin/mailman/listinfo/vial-devel</a><br></blockquote>
I think that reusing commands inside other commands is ugly too, I'm correcting some of those ugly things now.<br>I did it to avoid code repetition though.<br>I also noticed that there are some patterns in the commands code that we could try to avoid,
i.e:<br>(defun command-function (args) body) (define-pattern ...#' command-function ...)<br>This one could be better expressed with a macro that outputs those two forms, maybe something like<br><br>(define-command command-function (args)
<br> (body) <br> (<arguments for define-pattern, without the #'command-function))<br> <br>also we could then specialize this macro, to avoid the pattern<br>(defun command-function2 (args)<br> (active-register-needs-updating)
<br> body)<br><br>reducing our lines of code even more, and getting rid of some patterns.<br>I also thought of decoupling a little the commands from the actual bindings, defining a bunch of variables that contain the actual bindings, like *join-lines-command-binding*, and bound them using a macro like
<br>(define-bindings<br> (*join-lines-command-binding* "J")<br> (*change-single-line-from-active-binding* "cc")<br> .....)<br>This would make changing actual bindings clearer I think. Also I think that surely there's a better way to do something like this than "defining a bunch of variables", but I'm a noob and now i thought of it as a simple method =)
<br> <br>We also have a bunch of commands that go into insert-mode, thus creating the pattern <br>(defun foo (args) <br> body<br> (enter-insert-mode))<br>so.... what about erasing this pattern too?<br><br>Let me know what do you think!
<br>__<br>Alessandro<br><br>