From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24417 invoked by alias); 18 Sep 2014 08:39:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 24403 invoked by uid 89); 18 Sep 2014 08:39:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mailout4.w1.samsung.com Received: from mailout4.w1.samsung.com (HELO mailout4.w1.samsung.com) (210.118.77.14) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (DES-CBC3-SHA encrypted) ESMTPS; Thu, 18 Sep 2014 08:39:52 +0000 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NC3005379J0SP20@mailout4.w1.samsung.com> for gcc-patches@gcc.gnu.org; Thu, 18 Sep 2014 09:42:36 +0100 (BST) Received: from eusync4.samsung.com ( [203.254.199.214]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id EF.F5.15956.35A9A145; Thu, 18 Sep 2014 09:39:47 +0100 (BST) Received: from [106.109.9.145] by eusync4.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0NC300ITC9EB3UA0@eusync4.samsung.com>; Thu, 18 Sep 2014 09:39:47 +0100 (BST) Message-id: <541A9A68.7050000@samsung.com> Date: Thu, 18 Sep 2014 08:39:00 -0000 From: Yury Gribov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-version: 1.0 To: Segher Boessenkool Cc: GCC Patches , Laurynas Biveinis , Jeff Law , Richard Biener , Bernhard Reutner-Fischer , Trevor Saunders , Mike Stump Subject: Re: [PATCHv4] Vimrc config with GNU formatting References: <540863C1.4000909@samsung.com> <54100735.5040700@samsung.com> <541867A2.6020405@samsung.com> <5419C01C.2040404@samsung.com> <20140918035233.GB24532@gate.crashing.org> In-reply-to: <20140918035233.GB24532@gate.crashing.org> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg01433.txt.bz2 On 09/18/2014 07:52 AM, Segher Boessenkool wrote:>> +# Local Vim config >> + >> +vimrc: >> + (cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc; $(LN_S) contrib/vimrc .lvimrc) >> + > > This is another target than what the doc (in the script itself) mentions. Right, I've forgot to fix it before sending the patch. Too much experimenting in the evening... > It is not marked as phony. Noted. > It should not _be_ phony; the two files should > be separate targets. I've done that initially but that may look weird for the user. When typing 'make .local.vimrc' in GCC build directory one would expect .local.vimrc to be created at the root of build directory, not srcdir. > Why make links instead of copies? A user will likely > want to edit his config. I see your point. On the other hand fixing a bug in contrib/vimrc will not be propagated to .local.vimrc which looks like a major disadvantage (to me at least). > The way you use ";" is wrong (it continues if there > is an error). Agreed. Current Makefiles do use ";" in backticks and that drew me away. > You don't need the "cd" anyway, come to that. Noted. > It's pretty silly to have a makefile target that only copies a file (that > is never used by the makefile itself); just tell in the doc where to copy > the file. I personally prefer a Makefile target to simplify things. But let's wait for other people opinions on this. >> --- /dev/null >> +++ b/contrib/vimrc >> @@ -0,0 +1,45 @@ >> +" Code formatting settings for Vim. >> +" >> +" To enable this for GCC files by default, install thinca's vim-localrc >> +" plugin and do >> +" $ make .local.vimrc > > No, we should *not* advertise an "enough rope" solution without mentioning > it *will* kill you. How about adding a disclaimer? E.g. "beware that Vim plugins are a GAPING SECURITY HOLE so use the at YOUR OWN RISK". (And note that Braun's plugin does use sandboxes). > Or not mention it at all. Esp. since your next option > has all the same functionality and more. It lacks very important functionality: user has to specify path to concrete GCC source tree when adding the autocmd. I have a dozen of trees on my box and I regularly rename, move or copy them. With plugins one doesn't have to bother fixing paths in ~/.vimrc which is important for productivity. >> +" Or if you dislike plugins, add autocmd in your ~/.vimrc: >> +" :au BufNewFile,BufReadPost path/to/gcc/* :so path/to/gcc/contrib/vimrc > > There are many more reasons than just "dislike of plugins" to prefer > something like this. For one thing, many Vim users will have many > similar statements in their config _already_. So "if you don't want to use plugins"? >> +" Or just source file manually every time if you are masochist: >> +" :so path/to/gcc/contrib/vimrc > > How is that masochist? Typing that cino by hand though, now that would > qualify ;-) Note that user has to type source command for every newly opened file. This indeed looks inconvenient (to me again). > Just keep things neutral please. Trying to salt the boring docs a bit to attract reader's attention ;) >> + 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 > > If you write this as absolute numbers instead of as shift widths, you do not > need to force sw and sts settings down people's throat. It might also be > easier to read? Well I doubt that, but it will be slightly shorter at least. IMHO matching shiftwidth with GNU indent may be useful. E.g. Vim won't reindent when you start editing an empty line and user will have to insert indents manually. Also replacing offsets with numbers hides the fact that they are based on GNU shiftwidth. >> + setlocal textwidth=79 > > The coding conventions say maximum line length is 80. From https://www.gnu.org/prep/standards/html_node/Formatting.html : "Please keep the length of source lines to 79 characters or less, for maximum readability in the widest range of environments." > 'tw' is a user preference as well The config just follows the GNU coding standard. Now we rarely do violate textwidth in our codes, that's why I do formatoptions+=l below. >> + setlocal formatoptions-=ro formatoptions+=cql > > Yet another user preference. Also mostly the default, except "l" -- which > won't do anything if tw=0 as it should be. And you do not enable "t" (also > on by default), so you do not want to wrap text anyway? Confused now. Me as well, the original config author did it that way. IMHO +t makes sense here. -Y