On Nov 27, 2013 10:19 PM, "hari.g" <[email protected]> wrote: > > On Wednesday, November 27, 2013 8:29:28 PM UTC+5:30, Ken Takata wrote: > > Hi, > > > > 2013/11/27 Wed 23:34:12 UTC+9 ZyX wrote: > > > On Nov 27, 2013 10:56 AM, "hari.g" <[email protected]> wrote: > > > > > > > > Hi, > > > > > > > > The attached patch adds a 'listchars' option 'lsp' which marks leading space at 'shiftwidth' with the defined characer so that you can follow the indent level in a bit too much nested code when you also have 'expandtab' option set and use spaces for indentation (e.g. Python code in many repos enforce a 'spaces only indenting' policy). > > > > > > This functionality would be good to have and I see it constantly seeked in vim-use and stackoverflow. But there are some issues though: > > > > > > 0. No tests. There is screenchar()/screenattr() pair for the tests. > > I've added a test for this change (Haven't been able to run the testsuite though).
Not enough comprehensive. It is good to test the following: - two positive values of &shiftwidth - two values of &tabstop with &shiftwidth set to zero - character displayed in *indentation* areas that should not be affected by your option - character displayed with set nolist - character displayed without listchars containing lsp - highlighting of the characters (I have pointed to screenattr() for this very reason) You *must* test for a regression (sw=0) in any case though. When you design test your goal is not verifying your code work under one certain circumstances. It is verifying that it works in all circumstances you can imagine to make any difference. I would also search for all mentions of testN in src/testdir: there is not a single Makefile (I used to submit patches that had problem of not doing all necessary changes, but that means Bram will have to do them for you which may slow him down unless he uses a script). > > > 1. I fail to comprehend what "lsp" stands for. Guess it is abbreviation: Leading Space P... Pwhat? > > 'lsp' was just a take on 'nbsp' (No-Break SPace), I also feel that it isn't much meaningful (since the substition happens only for some space characters). Any suggestions? nbsp is rather common abbreviation (e.g. in HTML), EOL also is. Other items in &lcs are not abbreviations. Your patch is displaying levels of indentation, right? Then I guess indent:, or indentlevel:, or indentsp(ace): will work. > > > 2. Your patch strips out meaningful trailing spaces from documentation. > > > 3. It also adds something to Make_mvc.mak which is not related (if you have to use it to compile it should be a separate patch). > > Reverted. > > > > 4. No interline tabs in nw_seen declaration. > > > 5. Tabs are not used in line > > > + nw_seen = nw_seen || !vim_iswhite(c); > > > > Fixed. > > > > > I found two more issues: > > > > 6. `:set sw=0` causes a crash. > > 7. C99 style comment should not be used. > > > Fixed. > > A modified patch is attached. > > Thanks for the input. > > -- > hari.g > > -- > -- > 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 > > --- > You received this message because you are subscribed to the Google Groups "vim_dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. -- -- 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 --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
