Difference between revisions of "Coding style/Reindenting"
(howto) |
(→Cleanup coordination) |
||
(35 intermediate revisions by 3 users not shown) | |||
Line 8: | Line 8: | ||
** <code>svn diff</code> |
** <code>svn diff</code> |
||
** <code>svn diff -x -w</code> (ignores whitespace changes) |
** <code>svn diff -x -w</code> (ignores whitespace changes) |
||
+ | ** <code>svn diff --diff-cmd=diff -x -uE</code> (ignores untabification changes; requires gnu diff) |
||
** <code>svn diff | cat -et </code> (to show tabs and end-of-line) |
** <code>svn diff | cat -et </code> (to show tabs and end-of-line) |
||
* Look for [[#Common failure modes]] |
* Look for [[#Common failure modes]] |
||
+ | * If stuff gets mangled, reset the affected files to the repository version (<code>svn revert</code>), and try adding exclusions. |
||
+ | * Document exclusions (temporary or permanent) in the tables below at [[#Cleanup coordination]]. |
||
+ | * Document additions to <code>$(INDENTDIRS)</code> in the tables as well. |
||
+ | * Commit changes to the trunk (if you have commit access). |
||
== Infrastructure == |
== Infrastructure == |
||
Line 76: | Line 81: | ||
== Cleanup coordination == |
== Cleanup coordination == |
||
− | Edit the tables to volunteer to evaluate a directory or exclusion. Please see [http://meta.wikimedia.org/wiki/Help:Table] for help with editing MediaWiki tables. If a table row |
+ | Edit the tables to volunteer to evaluate a directory or exclusion. Use <nowiki>~~~</nowiki> (three tildes) to insert your username in the "who?" cell of a row. Please see [http://meta.wikimedia.org/wiki/Help:Table] for help with editing MediaWiki tables. If a table row for a directory name seems too broad, feel free to create new rows specifying subdirectories so that we can divide the work with a finer granularity. |
− | Abbreviations |
+ | === Abbreviations/definitions === |
;P/C: Proposed/Committed |
;P/C: Proposed/Committed |
||
;E/F: Evaluate/Fix |
;E/F: Evaluate/Fix |
||
+ | ;Proposed: means that a directory or file is proposed for addition to one of the Makefile variables controlling reformatting. Generally this is meant for use by people without commit access who are volunteering to help with the reindenting effort. |
||
+ | ;Committed: means that a given directory or file is listed in the Makefile variable in the version of src/Makefile.in that is already committed to the trunk. |
||
+ | ;Evaluate: means that the listed person is going to inspect the file to determine if it should be manually fixed or moved to a permanent exclusion. |
||
+ | ;Fix: means that the listed person is going to manually fix the file to remedy any reindenting issues. |
||
+ | |||
+ | === Directories === |
||
{| class="wikitable" |
{| class="wikitable" |
||
Line 93: | Line 104: | ||
! comments |
! comments |
||
|- |
|- |
||
− | | appl || || || || || |
+ | | appl || C || N || Y|| [[User:Ghudson|Ghudson]] || |
|- |
|- |
||
| ccapi || || || || || |
| ccapi || || || || || |
||
Line 99: | Line 110: | ||
| clients || C || N || N || || |
| clients || C || N || N || || |
||
|- |
|- |
||
− | | include || |
+ | | include || C || Y || Y || [[User:Ghudson|Ghudson]] || |
|- |
|- |
||
| kadmin || C || Y || N || || |
| kadmin || C || Y || N || || |
||
|- |
|- |
||
− | | kdc || C || N || |
+ | | kdc || C || N || Y || [[User:tsitkova|Zhanna]] || |
|- |
|- |
||
| kim || || || || || |
| kim || || || || || |
||
|- |
|- |
||
− | | lib/apputuils || || || || || |
+ | | lib/apputuils || C || Y || Y || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | lib/crypto || || || || || |
+ | | lib/crypto || C || Y || Y || [[User:Ghudson|Ghudson]] || |
|- |
|- |
||
− | | lib/gssapi || || || || || |
+ | | lib/gssapi || C || Y || N || [[User:TomYu|TomYu]] || |
|- |
|- |
||
| lib/kadm5 || C || Y || N || || |
| lib/kadm5 || C || Y || N || || |
||
Line 117: | Line 128: | ||
| lib/kdb5 || C || N || N || || |
| lib/kdb5 || C || N || N || || |
||
|- |
|- |
||
− | | lib/krb5 || C || Y || |
+ | | lib/krb5 || C || Y || Y || [[User:Ghudson|Ghudson]] || |
|- |
|- |
||
| lib/rpc || || || || || |
| lib/rpc || || || || || |
||
|- |
|- |
||
− | | plugins || || || || || |
+ | | plugins || C || Y || Y || [[User:Ghudson|Ghudson]] || |
|- |
|- |
||
| prototype || C || N || Y || [[User:TomYu|TomYu]] || |
| prototype || C || N || Y || [[User:TomYu|TomYu]] || |
||
Line 127: | Line 138: | ||
| slave || C || Y || N || [[User:TomYu|TomYu]] || |
| slave || C || Y || N || [[User:TomYu|TomYu]] || |
||
|- |
|- |
||
− | | tests || || || || || |
+ | | tests || C || Y || N || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | util || || || || || |
+ | | util || C || Y || N || [[User:TomYu|TomYu]] || |
|- |
|- |
||
| windows || || || || || |
| windows || || || || || |
||
Line 136: | Line 147: | ||
=== Temporary exclusions === |
=== Temporary exclusions === |
||
⚫ | |||
+ | Add files or directories here if you need to temporarily exclude them from automatic reindenting (by adjusting the Makefile variables). For example, the reindenting may badly degrade the formatting of a file; if you notice that, add the file to one of the exclusion lists and try again. (from a fresh, unmodified copy of the file) |
||
+ | |||
⚫ | |||
{| class="wikitable" |
{| class="wikitable" |
||
Line 147: | Line 160: | ||
! comments |
! comments |
||
|- |
|- |
||
− | | lib/ |
+ | | lib/gssapi/generic/gssapiP_generic.h || C || F || open parens || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | lib/ |
+ | | lib/gssapi/generic/gssapi_ext.h || C || F || open parens || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | lib/krb5/ |
+ | | lib/gssapi/krb5/gssapiP_krb5.h || C || F || open parens || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | lib/ |
+ | | lib/gssapi/mechglue || C || E || Sun || [[User:TomYu|TomYu]] || |
|- |
|- |
||
− | | lib/ |
+ | | lib/gssapi/spnego || C || E || Sun || [[User:TomYu|TomYu]] || |
|- |
|- |
||
⚫ | |||
+ | | plugins/preauth/pkinit_accessor.h || C || E || Prototypes || [[User:Ghudson|Ghudson]] || Prototypes with long identifiers and comments for each argument |
||
+ | |- |
||
+ | | plugins/preauth/pkinit/pkinit_crypto.h || C || E || Prototypes || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/preauth/pkinit/pkinit.h || C || E || Prototypes || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/preauth/pkinit/pkinit_crypto_openssl.h || C || E || Prototypes || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
⚫ | |||
+ | |- |
||
+ | | tests/asn.1/ktest_equal.h || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | tests/asn.1/utility.h || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | tests/gss-threads/gss-misc.c || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | tests/gss-threads/gss-misc.h || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | tests/hammer/kdc5_hammer.c || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/et/com_err.h || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/profile/prof_int.h || C || F || open parens || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/profile/profile.hin || C || F || open parens || [[User:TomYu|TomYu]] || |
||
|} |
|} |
||
=== Exclusions to keep === |
=== Exclusions to keep === |
||
+ | |||
+ | BSD-marked exclusions are for files of BSD or similar origin, and that should have the formatting preserved in a BSD style for consistency. "Other exclusions" should be uncommon, but can include source code of external origin or automatically generated source code. |
||
{| class="wikitable" |
{| class="wikitable" |
||
Line 169: | Line 208: | ||
! who? |
! who? |
||
! comments |
! comments |
||
+ | |- |
||
+ | | include/gssrpc || C || Sun || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | include/iprop.h || C || rpcgen || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | include/k5-platform.h || C || Macros || [[User:Ghudson|Ghudson]] || |
||
|- |
|- |
||
| kadmin/cli/strftime.c || C || BSD || [[User:TomYu|TomYu]] || |
| kadmin/cli/strftime.c || C || BSD || [[User:TomYu|TomYu]] || |
||
Line 175: | Line 220: | ||
|- |
|- |
||
| kadmin/server/kadm_rpc_svc.c || C || rpcgen || [[User:TomYu|TomYu]] || |
| kadmin/server/kadm_rpc_svc.c || C || rpcgen || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | lib/apputils/daemon.c || C || BSD || [[User:TomYu|TomYu]] || |
||
|- |
|- |
||
| lib/kadm5/admin_xdr.h || C || rpcgen || [[User:TomYu|TomYu]] || |
| lib/kadm5/admin_xdr.h || C || rpcgen || [[User:TomYu|TomYu]] || |
||
Line 191: | Line 238: | ||
|- |
|- |
||
| slave/kpropd_rpc.c || C || rpcgen || [[User:TomYu|TomYu]] || |
| slave/kpropd_rpc.c || C || rpcgen || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/support/mkstemp.c || C || BSD || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/support/strlcpy.c || C || BSD || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/windows/getopt.c || C || BSD || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/windows/getopt.h || C || BSD || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/windows/getopt_long.c || C || BSD || [[User:TomYu|TomYu]] || |
||
|} |
|} |
||
Line 200: | Line 257: | ||
! who? |
! who? |
||
! comments |
! comments |
||
+ | |- |
||
+ | | lib/apputils/dummy.c || C || trivial || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | lib/crypto/builtin/aes || C || Gladman || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | lib/crypto/builtin/camellia || C || Camellia || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | lib/crypto/builtin/sha2 || C || Heimdal || [[User:TomYu|TomYu]] || |
||
|- |
|- |
||
| lib/krb5/krb/deltat.c || C || Bison || [[User:TomYu|TomYu]] || |
| lib/krb5/krb/deltat.c || C || Bison || [[User:TomYu|TomYu]] || |
||
|- |
|- |
||
| lib/krb5/unicode || C || Unicode || [[User:TomYu|TomYu]] || |
| lib/krb5/unicode || C || Unicode || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | plugins/kdb/db2/libdb2 || C || Sleepycat || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/db2/pol_xdr.c || C || rpcgen || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/hdb/hdb.h || C || Heimdal || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/hdb/hdb_asn1.h || C || Heimdal || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/hdb/hdb_err.h || C || Heimdal || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/hdb/windc_plugin.h || C || Heimdal || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/ldap/libkdb_ldap/princ_xdr.c || C || rpcgen || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | plugins/kdb/ldap/libkdb_ldap/princ_xdr.h || C || rpcgen || [[User:Ghudson|Ghudson]] || |
||
+ | |- |
||
+ | | util/k5ev || C || libev || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/profile/profile_tcl.c || C || SWIG || [[User:TomYu|TomYu]] || |
||
+ | |- |
||
+ | | util/support/fnmatch.c || C || BSD || [[User:TomYu|TomYu]] || |
||
|} |
|} |
Latest revision as of 18:24, 14 October 2011
Contents
How to help with the "great reindent"
Sign up below at #Cleanup coordination.
- Temporarily modify the #Makefile variables to try out stuff
- Run the
make mark-cstyle
andmake reindent
targets - Inspect the changes
-
svn diff
-
svn diff -x -w
(ignores whitespace changes) -
svn diff --diff-cmd=diff -x -uE
(ignores untabification changes; requires gnu diff) -
svn diff | cat -et
(to show tabs and end-of-line)
-
- Look for #Common failure modes
- If stuff gets mangled, reset the affected files to the repository version (
svn revert
), and try adding exclusions. - Document exclusions (temporary or permanent) in the tables below at #Cleanup coordination.
- Document additions to
$(INDENTDIRS)
in the tables as well. - Commit changes to the trunk (if you have commit access).
Infrastructure
- emacs file-local variable settings
- scripts in src/util
- make mark-cstyle
- make reindent
Prerequisites
The reindenting scripts require at least GNU Emacs version 22. Earlier versions of Emacs had versions of CC-Mode that produce variant indentations. The number of differences is small, though. Different versions of Emacs have different incarnations of whitespace.el
that behave differently and have different interfaces. The reindenting scripts accommodate for these differences, with a very few exceptions. (Old-style whitespace.el
cleans up mixtures of spaces and tabs anywhere on a line, while the new-style one only does so inside indentation whitespace.)
GNU find (or a reasonably compatible find program that supports the -path
and -print0
options) is also required, as is an xargs program that supports the -0
option.
Python is also needed, though any modern version (2.3 or later) should work.
If the versions of these programs that you are using do not have their usual names, you can override the program names when running make, e.g.
make reindent FIND=gfind XARGS=gxargs EMACS=/usr/local/bin/emacs22
Emacs variables
We use various combinations Emacs file-local variable settings to mark what sort of C formatting style a given source file conforms with, as well as for controlling the operation of the reindenting scripts.
This is the standard file-local variable line (at top of file) for marking files expected to fully conform to our "krb5" formatting style:
/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
For BSD-derived code, we use:
/* -*- mode: c; c-file-style: "bsd"; indent-tabs-mode: t -*- */
Makefile targets
Running make mark-cstyle
will mark certain designated files as the "krb5" style, and certain others as "bsd" style. At the moment, it uses an explicitly specified group of directories as starting points, but in the future it should start at the top of the source tree and rely on exclusions to tailor its operation.
Running make reindent
executes the actual reindenting script, which examines the file-local variable settings in each file. The steps are:
- Untabify (expand tabs to spaces) if the file-local variable settings specify
- Reindent if the file is marked as conforming to the "krb5" C style
- Perform whitespace cleanup
- Delete whitespace at the ends of lines
- Remove blank lines at beginning and end of file
- Fix up mixtures of tabs and spaces (only if
indent-tabs-mode
is explicitly specified)
Makefile variables
$(INDENTDIRS)
sets the list of directories that the make mark-cstyle
target will operate on.
$(BSDFILES)
contains a list of files that the make mark-cstyle
target will mark as BSD-style.
$(OTHEREXCLUDES)
contains a list of other files or directories to exclude from being marked as krb5-style.
Common failure modes
The most consistently occurring problems when reindenting appear to be:
- Function declarations or similar constructs that have an open-parenthesis as the first non-whitespace character of a line
- Non-conforming block comments, i.e., ones without an asterisk on the left-hand side of each line
Cleanup coordination
Edit the tables to volunteer to evaluate a directory or exclusion. Use ~~~ (three tildes) to insert your username in the "who?" cell of a row. Please see [1] for help with editing MediaWiki tables. If a table row for a directory name seems too broad, feel free to create new rows specifying subdirectories so that we can divide the work with a finer granularity.
Abbreviations/definitions
- P/C
- Proposed/Committed
- E/F
- Evaluate/Fix
- Proposed
- means that a directory or file is proposed for addition to one of the Makefile variables controlling reformatting. Generally this is meant for use by people without commit access who are volunteering to help with the reindenting effort.
- Committed
- means that a given directory or file is listed in the Makefile variable in the version of src/Makefile.in that is already committed to the trunk.
- Evaluate
- means that the listed person is going to inspect the file to determine if it should be manually fixed or moved to a permanent exclusion.
- Fix
- means that the listed person is going to manually fix the file to remedy any reindenting issues.
Directories
directory name | P/C | exclusions? | reviewed? | who? | comments |
---|---|---|---|---|---|
appl | C | N | Y | Ghudson | |
ccapi | |||||
clients | C | N | N | ||
include | C | Y | Y | Ghudson | |
kadmin | C | Y | N | ||
kdc | C | N | Y | Zhanna | |
kim | |||||
lib/apputuils | C | Y | Y | TomYu | |
lib/crypto | C | Y | Y | Ghudson | |
lib/gssapi | C | Y | N | TomYu | |
lib/kadm5 | C | Y | N | ||
lib/kdb5 | C | N | N | ||
lib/krb5 | C | Y | Y | Ghudson | |
lib/rpc | |||||
plugins | C | Y | Y | Ghudson | |
prototype | C | N | Y | TomYu | |
slave | C | Y | N | TomYu | |
tests | C | Y | N | TomYu | |
util | C | Y | N | TomYu | |
windows |
Temporary exclusions
Add files or directories here if you need to temporarily exclude them from automatic reindenting (by adjusting the Makefile variables). For example, the reindenting may badly degrade the formatting of a file; if you notice that, add the file to one of the exclusion lists and try again. (from a fresh, unmodified copy of the file)
Exclusions here will move to one of the "to keep" tables below once evaluated. If they are fixed, remove them instead. (from the table, and from the Makefile, if the exclusion was already in a committed version of src/Makefile.in)
directory or file name | P/C | E/F | why? | who? | comments |
---|---|---|---|---|---|
lib/gssapi/generic/gssapiP_generic.h | C | F | open parens | TomYu | |
lib/gssapi/generic/gssapi_ext.h | C | F | open parens | TomYu | |
lib/gssapi/krb5/gssapiP_krb5.h | C | F | open parens | TomYu | |
lib/gssapi/mechglue | C | E | Sun | TomYu | |
lib/gssapi/spnego | C | E | Sun | TomYu | |
plugins/preauth/pkinit_accessor.h | C | E | Prototypes | Ghudson | Prototypes with long identifiers and comments for each argument |
plugins/preauth/pkinit/pkinit_crypto.h | C | E | Prototypes | Ghudson | |
plugins/preauth/pkinit/pkinit.h | C | E | Prototypes | Ghudson | |
plugins/preauth/pkinit/pkinit_crypto_openssl.h | C | E | Prototypes | Ghudson | |
tests/asn.1/ktest.h | C | F | open parens | TomYu | |
tests/asn.1/ktest_equal.h | C | F | open parens | TomYu | |
tests/asn.1/utility.h | C | F | open parens | TomYu | |
tests/gss-threads/gss-misc.c | C | F | open parens | TomYu | |
tests/gss-threads/gss-misc.h | C | F | open parens | TomYu | |
tests/hammer/kdc5_hammer.c | C | F | open parens | TomYu | |
util/et/com_err.h | C | F | open parens | TomYu | |
util/profile/prof_int.h | C | F | open parens | TomYu | |
util/profile/profile.hin | C | F | open parens | TomYu |
Exclusions to keep
BSD-marked exclusions are for files of BSD or similar origin, and that should have the formatting preserved in a BSD style for consistency. "Other exclusions" should be uncommon, but can include source code of external origin or automatically generated source code.
directory or file name | P/C | why? | who? | comments |
---|---|---|---|---|
include/gssrpc | C | Sun | Ghudson | |
include/iprop.h | C | rpcgen | Ghudson | |
include/k5-platform.h | C | Macros | Ghudson | |
kadmin/cli/strftime.c | C | BSD | TomYu | |
kadmin/server/ipropd_svc.c | C | rpcgen | TomYu | |
kadmin/server/kadm_rpc_svc.c | C | rpcgen | TomYu | |
lib/apputils/daemon.c | C | BSD | TomYu | |
lib/kadm5/admin_xdr.h | C | rpcgen | TomYu | |
lib/kadm5/clnt/client_rpc.c | C | rpcgen | TomYu | |
lib/kadm5/kadm_rpc.h | C | rpcgen | TomYu | |
lib/kadm5/kadm_rpc_xdr.c | C | rpcgen | TomYu | |
lib/kadm5/srv/adb_xdr.c | C | rpcgen | TomYu | |
lib/krb5/krb/strftime.c | C | BSD | TomYu | |
lib/krb5/krb/strptime.c | C | BSD | TomYu | |
slave/kpropd_rpc.c | C | rpcgen | TomYu | |
util/support/mkstemp.c | C | BSD | TomYu | |
util/support/strlcpy.c | C | BSD | TomYu | |
util/windows/getopt.c | C | BSD | TomYu | |
util/windows/getopt.h | C | BSD | TomYu | |
util/windows/getopt_long.c | C | BSD | TomYu |
directory or file name | P/C | why? | who? | comments |
---|---|---|---|---|
lib/apputils/dummy.c | C | trivial | TomYu | |
lib/crypto/builtin/aes | C | Gladman | Ghudson | |
lib/crypto/builtin/camellia | C | Camellia | TomYu | |
lib/crypto/builtin/sha2 | C | Heimdal | TomYu | |
lib/krb5/krb/deltat.c | C | Bison | TomYu | |
lib/krb5/unicode | C | Unicode | TomYu | |
plugins/kdb/db2/libdb2 | C | Sleepycat | Ghudson | |
plugins/kdb/db2/pol_xdr.c | C | rpcgen | Ghudson | |
plugins/kdb/hdb/hdb.h | C | Heimdal | Ghudson | |
plugins/kdb/hdb/hdb_asn1.h | C | Heimdal | Ghudson | |
plugins/kdb/hdb/hdb_err.h | C | Heimdal | Ghudson | |
plugins/kdb/hdb/windc_plugin.h | C | Heimdal | Ghudson | |
plugins/kdb/ldap/libkdb_ldap/princ_xdr.c | C | rpcgen | Ghudson | |
plugins/kdb/ldap/libkdb_ldap/princ_xdr.h | C | rpcgen | Ghudson | |
util/k5ev | C | libev | TomYu | |
util/profile/profile_tcl.c | C | SWIG | TomYu | |
util/support/fnmatch.c | C | BSD | TomYu |