On Feb 11, 12:06 am, Yarin <[email protected]> wrote: > Hello, > I've recently made a useful addition/extension for version 7.3: > It's basically elastic tabs, but it aligns the dust-covered vertical > tab character instead (because aligning the standard tab would break > lots of stuff). The diff is here: > > http://naqua.net/p/vim/vim73_clustervtabs.diff >
As to the patch itself, I have not looked in detail yet, but I do have a few suggestions from a very brief glance: 1.) I believe Vim code uses C-style /*...*/ comments only, not C++- style // comments. Some C compilers do not support the latter. 2.) You use "iff" instead of "if" in many places. Perhaps you meant "if and only if" but that's probably not really needed for understanding and it looks like a typo. 3.) There's a commented-out #define NO_CLUSTER_VTABS in feature.h which should be removed. 4.) There's a few commented-out debug-like printf statements, and some other commented code which should be removed, like "// || back_up == -2)" in screen.c 5.) I don't personally like seeing attribution/author comments in the code (and neither do many others, http://www.producingoss.com/en/managing-volunteers.html#territoriality ), although a quick grep of the Vim source shows that it is done in a handful of places; especially for complicated features like encryption or specific GUIs. Personally, I'd shy away from a prominent in-code attribution like the one in the patch. If you really want it, a very short statement in the header comment would probably suffice. If this ever gets included, Bram normally does mention developers of specific features or patches in either the release notes or the patch description. -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php
