On Fri, Feb 12, 2010 at 07:28:02PM +0000, Vegard Øye wrote:
(I also don't see the point of wrapping `let' around `dolist', am I missing something?)
`dolist' needs a temporary variable to bind each list element to.
Well, sure, but `dolist' does the binding for you: (let ((cool 8)) (dolist (cool '(1 2 3 4)) (prin1 cool)) (prin1 cool)) 123488 Even Emacs Lisp does not suck *that* bad.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;; VISUAL MODE BINDINGS ;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (define-key viper-vi-basic-map "v" 'vimpulse-visual-mode) (define-key viper-vi-basic-map "V" 'vimpulse-visual-mode-linewise)
This should be deleted, as you redefine "v" and "V" later anyway, and the following has nothing to do with Visual mode.
My point is that the above lines do not belong *anywhere*, they're just redundant; starting at l. 2994 of vimpulse-big.el, you have: ;;;;;;;;;;;;;;;;;;;; ;;; Key bindings ;;; ;;;;;;;;;;;;;;;;;;;; (define-key viper-vi-basic-map "v" 'vimpulse-visual-toggle-normal) (define-key viper-vi-basic-map "V" 'vimpulse-visual-toggle-line) ... so you're defining the keys again.
Yes. Visual bindings don't belong in vimpulse-misc-keybindings.el at all. The file has turned into a "miscellaneous" section of sorts. I think it would be cleaner to have a vimpulse-keys.el file which merges vimpulse-modal.el with the keybinding stuff in vimpulse-misc-keybindings.el, while the rest could go in vimpulse-commands.el or some such.
Maybe, yeah; that would likely prevent situations like the above, at least.
I also think the current file names are too long. The libraries bundled with either Emacs seldom use more than one hyphen in their file names. Viper, for instance, consists of viper.el, viper-cmd.el, viper-ex.el, viper-init.el, viper-keym.el, viper-macs.el, viper-mous.el and viper-util.el.
Here's a renaming proposal:
vimpulse-dependencies.el => vimpulse-depend.el vimpulse-misc-keybindings.el => vimpulse-keys.el vimpulse-paren-matching.el => vimpulse-paren.el vimpulse-text-object-system.el => vimpulse-text.el vimpulse-viper-function-redefinitions.el => vimpulse-viper.el vimpulse-visual-mode.el => vimpulse-visual.el
What do you think?
I don't think that is really necessary -- for one thing, the current file names describe their contents better than the shorter alternatives you propose, and besides that, it's all just kinda behind-the-scenes -- most users don't even know about the single files, they're using the concatenated vimpulse.el. Which makes more sense to me anyway -- I don't think even the splitting itself is necessary. 4000 lines might be a lot, but it's not too much IMO, also not in comparison with some files in Emacs core (like simple.el with 6655). I personally would just stick to a single file (and do so, as you see from the line counts I give, sorry for that ;)) -- it's simply more convenient. It's a PITA for me to look at ten files instead of one, esp. when some of them really only contain a few lines. Regards, Štěpán