754136 - Add emacs/vim modelines to files in NSS

Status: RESOLVED WONTFIX

(NSS :: Libraries, enhancement, P5)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Add modelines and remove CVS keywords from most C source files and shell scripts

This patch rewrites the top comment block of the affected files approximately as follows:

   +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 4 -*- */
   +/* vim: set ts=8 noet sw=4 tw=80: */
   -/*
   - * filename.ext - Description of the file
   - * 
   - * This Source Code Form is subject to the terms of the Mozilla Public
   +/* This Source Code Form is subject to the terms of the Mozilla Public
     * License, v. 2.0. If a copy of the MPL was not distributed with this
     * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
   -/* $Id$ */

   +/* Description of the file */
    -#ifdef DEBUG
    -static const char CVS_ID[] = "@(#) $RCSfile$ $Revision$ $Date$";
    -#endif /* DEBUG */
 
That is, it:

1. Adds emacs and VIM modelines
2. Removes CVS keywords
3. Moves top-level comments below the license block, so they aren't between the modelines and the license text.

Motivation:

1. Mozilla (and Chromium) coding guidelines are "2 space indent, no tabs", but NSs uses a variety of coding styles, usually "4 space indent with tabs, tab stops every 8 characters". The modelines are helpful to Mozilla (and probably Chromium) developers like me who need to go back and forth between the two coding styles. This is especially important to me personally now, because I use Visual Studio and it now supports modelines through an extension that a Mozilla developer wrote.

It is very tedious to review patches, especially from people who don't frequently write NSS patches, because we have to fix whitespace, then that can cause merge conflicts when patches are updated, etc. In general, the whitespace issues are an unnecessary distraction.

2. The CVS keyword expansion makes managing local patches to NSS tedious. It also makes comparing different copies of the source code (e.g. the copy in mozilla-central vs. a local copy checked out from CVS) difficult. Although it is possible to use the "-kk" CVS option to avoid keyword expansion, we don't do that in mozilla-central and that makes the imports into mozilla-central noisy and difficult to compare with other copies of the tree. Also, in general, the keyword expansion doesn't interact well when trying to use another version control system (like hg) locally on top of the CVS repository.

3. It is hard to read the top-level comments when they occur between the modelines and the license.

What files were changed:

* Modelines were added to *.c, *.h, or *.sh that hae MPL 2.0 license block, except for lib/sqlite and lib/zlib.
* CVS keywords were removed from all comments except lib/ckfw.
* All *_CVS_ID string constants were removed except lib/ckfw.
* I made sure to preserve places where the CVS keywords seem to have semantic meaning, e.g. lib/ckfw and ssltap.c.

What modelines were used:

* The default is 8 spaces per tab, using tabs, 4 space indents; see the example above.

* I used some regex foo to find files that were exceptions to that. Some examples of exceptions include:

   * lib/freebl/mpi/ecl
   * shell scripts,
   * lib/base
   * lib/libpkix and cmd/libpkix

* What does the emacs modeline mean, exactly:

   Mode: C;             The file is a C source file.
   tab-width: 8         tabs stops are every 8 spaces
   indent-tabs-mode: t; Replace sequences of tab-width spaces with tabs
   c-basic-offset:      Block indention is every 4 spaces

* What does the VIM modeline mean, exactly:

   set ts=8             tab stops are every 8 spaces
   noet                 do not replace tabs with spaces
   sw=4                 Block indention is every 4 spaces
   tw=80                Hint that the max acceptable line length
                        is 80 chars

I made sure to use "Mode: sh" for the shell scripts.

Note that the Mozilla Coding Guidelines encourage the use of modelines:

https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide#Mode_line

Comment 1

[Robert Relyea]

Comment on attachment 622990 [details] [diff] [review]
Add modelines and remove CVS keywords from most C source files and shell scripts

r- but only for the following file:

everything under nss/lib/crmf, nss/lib/softokn/fipstest.c

Consider everything else r+ from me. Your arguments are sound, and the changes are reasonable.

Comments for the exceptions:

nss/lib/softoken/fipstest.c lost it's 'title comment' (FIPS Power-Up Self tests).

Two other files that lost the title comment are reasonable because those title comments were cut and paste artifacts which did not belong to the in the files to begin with (ssl3ecc.c and sslinit.c) 

nss/lib/crmf - please remove the old EMAC tabs line from the majority of these files. They are now redundant.

Thanks for the work going over so many files, and the careful work of avoiding proper uses.

bob

Comment 2

[Wan-Teh Chang]

(In reply to Brian Smith (:bsmith) from comment #0)
>
> * What does the emacs modeline mean, exactly:
> 
>    Mode: C;             The file is a C source file.
>    tab-width: 8         tabs stops are every 8 spaces
>    indent-tabs-mode: t; Replace sequences of tab-width spaces with tabs
>    c-basic-offset:      Block indention is every 4 spaces

Please do not use indent-tabs-mode: t.  Bob is the only NSS developer
who likes to use tabs.  (Nelson was the other one.)  I often ask
people to use tabs if the code he modifies uses tabs.  I am willing
to stop asking that.

> * What does the VIM modeline mean, exactly:
> 
>    set ts=8             tab stops are every 8 spaces
>    noet                 do not replace tabs with spaces
>    sw=4                 Block indention is every 4 spaces
>    tw=80                Hint that the max acceptable line length
>                         is 80 chars

Your description of 'noet' seems to contradict your description
of indent-tabs-mode: t.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Wan-Teh Chang from comment #2)> 
> Please do not use indent-tabs-mode: t.

First, I tried to limit indent-tabs-mode: t to files that already contain tabs (predominantly). I am OK to switch everything to indent-tabs-mode: nil. Bob, does that seem reasonable to you?

> > * What does the VIM modeline mean, exactly:
> > 
> >    set ts=8             tab stops are every 8 spaces
> >    noet                 do not replace tabs with spaces
> 
> Your description of 'noet' seems to contradict your description
> of indent-tabs-mode: t.

It means "do not expand tabs to spaces" (note: not "spaces to tabs"), which is what "indent-tabs-mode: t" means.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Removing CVS keywords was done in bug 863871.