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