[PATCHv3] Vimrc config with GNU formatting

Hi all,

This is the third version of the patch. A list of changes since last 
version:
* move config to contrib so that it's _not_ enabled by default (current 
score is 2/1 in favor of no Vim config by default)
* update Makefile.in to make .local.vimrc if developer asks for it
* disable autoformatting for flex files
* fix filtering of non-GNU sources (libsanitizer)
* added some small fixes in cinoptions based on feedback from community

As noted by Richard, the config does not do a good job of formatting 
unbound {} blocks e.g.
void
foo ()
{
   int x;
     {
       // I'm an example of bad bad formatting
     }
}
but it seems to be the best we can get with Vim's cindent
(and I don't think anyone seriously considers writing a custom indentexpr).

Ok to commit?

-Y

On Tue, Sep 16, 2014 at 08:38:58PM +0400, Yury Gribov wrote:
> Hi all,
> 
> This is the third version of the patch. A list of changes since last
> version:
> * move config to contrib so that it's _not_ enabled by default (current
> score is 2/1 in favor of no Vim config by default)

fwiw, I think enabling it by default especially when that really means
enable it if you've enabled the localrc plugin makes sense.  I don't see
how you can enable the localrc plugin and then complain when people use
it for its designed purpose.  However something in contrib/ is probably
easier for new people to find than something on the wiki or something so
better than doing nothing :)

Thanks!

Trev

> * update Makefile.in to make .local.vimrc if developer asks for it
> * disable autoformatting for flex files
> * fix filtering of non-GNU sources (libsanitizer)
> * added some small fixes in cinoptions based on feedback from community
> 
> As noted by Richard, the config does not do a good job of formatting unbound
> {} blocks e.g.
> void
> foo ()
> {
>   int x;
>     {
>       // I'm an example of bad bad formatting
>     }
> }
> but it seems to be the best we can get with Vim's cindent
> (and I don't think anyone seriously considers writing a custom indentexpr).
> 
> Ok to commit?
> 
> -Y

> commit 67219512dac9a5cc14eea8f157222a226044dd72
> Author: Yury Gribov <y.gribov@samsung.com>
> Date:   Thu Sep 4 16:55:44 2014 +0400
> 
>     2014-09-16  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
>     	    Yury Gribov  <y.gribov@samsung.com>
>     
>     	Vim config with GNU formatting.
>     
>     contrib/
>     	* vimrc: New file.
>     
>     /
>     	* .gitignore: Added .local.vimrc.
>     	* Makefile.tpl (.local.vimrc): New target.
>     	* Makefile.in: Regenerate.
> 
> diff --git a/.gitignore b/.gitignore
> index e9b56be..252b8b0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -32,6 +32,8 @@ POTFILES
>  TAGS
>  TAGS.sub
>  
> +.local.vimrc
> +
>  .gdbinit
>  .gdb_history
>  
> diff --git a/Makefile.in b/Makefile.in
> index d6105b3..e573069 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -2384,6 +2384,11 @@ mail-report-with-warnings.log: warning.log
>  	chmod +x $@
>  	echo If you really want to send e-mail, run ./$@ now
>  
> +# Local Vim config
> +
> +vimrc:
> +	(cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc)
> +
>  # Installation targets.
>  
>  .PHONY: install uninstall
> diff --git a/Makefile.tpl b/Makefile.tpl
> index f7c7e38..d050694 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -867,6 +867,11 @@ mail-report-with-warnings.log: warning.log
>  	chmod +x $@
>  	echo If you really want to send e-mail, run ./$@ now
>  
> +# Local Vim config
> +
> +vimrc:
> +	(cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc)
> +
>  # Installation targets.
>  
>  .PHONY: install uninstall
> diff --git a/contrib/vimrc b/contrib/vimrc
> new file mode 100644
> index 0000000..7287bd1
> --- /dev/null
> +++ b/contrib/vimrc
> @@ -0,0 +1,43 @@
> +" Code formatting settings for Vim.
> +"
> +" To enable this for GCC files by default, install thinca's localrc plugin
> +" and do
> +"   $ make .local.vimrc
> +" Or if you dislike plugins, add autocmd in your .vimrc:
> +"   :au BufNewFile,BufReadPost path/to/gcc/* :so path/to/gcc/.local.vimrc
> +" Or just source file manually every time if you are masochist:
> +"   :so .local.vimrc
> +" 
> +" Copyright (C) 2014 Free Software Foundation, Inc.
> +"
> +" This program is free software; you can redistribute it and/or modify
> +" it under the terms of the GNU General Public License as published by
> +" the Free Software Foundation; either version 3 of the License, or
> +" (at your option) any later version.
> +"
> +" This program is distributed in the hope that it will be useful,
> +" but WITHOUT ANY WARRANTY; without even the implied warranty of
> +" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +" GNU General Public License for more details.
> +"
> +" You should have received a copy of the GNU General Public License
> +" along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +function! SetStyle()
> +  let l:fname = expand("%:p")
> +  if stridx(l:fname, 'libsanitizer') != -1
> +    return
> +  endif
> +  let l:ext = fnamemodify(l:fname, ":e")
> +  let l:c_exts = ['c', 'h', 'cpp', 'cc', 'C', 'H', 'def', 'java']
> +  if index(l:c_exts, l:ext) != -1
> +    setlocal cindent
> +    setlocal shiftwidth=2
> +    setlocal softtabstop=2
> +    setlocal cinoptions=>2s,n-s,{s,^-s,:s,=s,g0,f0,hs,p2s,t0,+s,(0,u0,w1,m0
> +    setlocal textwidth=79
> +    setlocal formatoptions-=ro formatoptions+=cql
> +  endif
> +endfunction
> +
> +call SetStyle()

On Tue, Sep 16, 2014 at 05:58:58PM -0400, Trevor Saunders wrote:
> fwiw, I think enabling it by default especially when that really means
> enable it if you've enabled the localrc plugin makes sense.

Enabling it by default means enabling it for all users.  That is a really
really bad plan; many of the options this script sets are user preferences.

You can make Vim automatically adapt settings, but you cannot make the Vim
user adapt to that.

Of course Vim won't use this script by default anyway.  It would be nice
if there was some "modeline for this whole subtree" mechanism, but there
is not.

> I don't see
> how you can enable the localrc plugin and then complain when people use
> it for its designed purpose.

Sure.  If you have the localrc thing installed, anyone who can write files
you can read can make your vim do *anything* (and I mean *anything*).  It
is a security disaster, that is, there is no security at all.  It runs any
script in the path from the file you open up to the root.  There is no
confirmation asked, no whitelist, no blacklist, no nothing.  And no
sandboxing either, of course.

Did I mention /tmp?  And writing to files?  Or just opening a shell.  The
possibilities are endless!

We should not encourage people to install this.  Running it is reckless;
telling other people to run it is irresponsible.

> However something in contrib/ is probably
> easier for new people to find than something on the wiki or something so
> better than doing nothing :)

Yup, just a bunch of recommended settings somewhere easy to find in contrib/
should be quite helpful to many people.


Segher

On 09/17/2014 05:01 PM, Segher Boessenkool wrote:
> You can make Vim automatically adapt settings, but you cannot make the Vim
> user adapt to that.

How about making Vim user adapt to GNU coding style though?
Not that I want to start flame again.

> Sure.  If you have the localrc thing installed, anyone who can write files
> you can read can make your vim do *anything* (and I mean *anything*).

I thought that modern versions of localrc run .local.vimrc scripts in a 
sandbox?

-Y

>> Sure.  If you have the localrc thing installed, anyone who can write
>> files
>> you can read can make your vim do *anything* (and I mean *anything*).
>
> I thought that modern versions of localrc run .local.vimrc scripts in a
> sandbox?

Ok, looks like Markus Braun's 
http://www.vim.org/scripts/script.php?script_id=441 does this.
Thinca's plugin currently indeed allows arbitrary code execution.

-Y