#2 Common code-formatting

Thimo Neubauer (thimo@macht.org) commented on the issue:

http://astyle.sourceforge.net/ claims to be a C++ formatter, we tested an older version (probably 1.15.3) which seemed ok but didn't totally convince us.

We're searching for a system which a) runs on a large variety of systems b) knows how to format C++ (this is where indent fails) c) can be parametrized to our liking

Sounds like you're looking for vim. It's available everywhere, handles everything and has more options than you could wish for. For C-specific indent-options, e.g., look up cinoptions-values via

vim +':he cinoptions-values | :only | :normal zt'

To reindent a file (in this case dune-common/common/poolallocator.hh), something along the lines of

vim +'set ts=2 sw=2 et cino= | :%s/ +$//ge | normal gg=G ZZ' dune-common/common/poolallocator.hh >/dev/null

can be done (cleans up and writes).

flyspray ate my backslash.

vim +'set ts=2 sw=2 et cino= | :%s/ +$//ge | normal gg=G ZZ' dune-common/common/poolallocator.hh >/dev/null

Maybe enforcing a single formatting style is more useful than adding a tool and a mechanism to run it.

Avoiding trailing whitespace is easy.

Add

let c_space_errors = 1

to your ~/.vimrc or

'(show-trailing-whitespace t)

to your ~/.emacs

Since there are no code formating rules no one seems to care about this. A lot of files contain a mixture of tabs and space with 2/3/4 spaces per indent and even tab+spaces in one line which makes them really ugly. Perhaps a first step would be to agree on some basic rules - if this is possible without starting a holy war on K&R/Linux/GNU/...-style. Then one could add mode lines for vim and emacs. Id' suggest something like the result of

astyle --style=kr -s4 -pUC astyle --style=ansi -s4 -pUC

If we like to give astyle a try again rather then vim since vim indents templates in a strange way (not used in dune 'til now).

the general coding style is of little importance. all of them are readable, it's just a matter of taste.

what matters is tabs vs. spaces. working w/ a mixed indentation is a mess and changing indentation adds a lot of noise to patches (yes, diff -w ignores whitespace but the websvn diff viewer does not provide that functionality afaik)

mode lines can take care of indentation in emacsen and vim, once something has been agreed upon. that implies adding two lines to every file. doing that can be automated, but maybe there's a better way. also, maybe someone is using another editor? then that'd either become three mode lines or impossible to handle, depending on the feature set of that editor. anyone using another editor should probably speak up now.

another, imho better approach would be customizing one's editor, either by applying project-specific changes or (and this should work in any scriptable editor -- i suppose e.g. textmate, too? -- even though e.g. vimscript is awful to work w/) by adding an appropriate script. checking whether one's cwd is below a certain directory and, if so, making buffer-local changes is trivial in emacs.

emacs 23 makes setting per-project settings very easy: http://www.gnu.org/software/emacs/manual/html_node/emacs/Directory-Variables.html

I believe there are two import issues regarding indentation we should agree upon:

The use of tabs for indentation,
how many character positions one indentation level should usually indent. Once these two issues a decided, we can write mode-lines for emacs and vim into all files, although I doubt that can be done automatically. (You have to look at each file and find the tab-width currently to put it in the mode-line or to replace the tabs by spaces. Some files may not even use a consistent tab-width.)

I propose to ban tabs for indentation, and to enforce that via mode-lines for emacs and vim. For files which currently do indentation using tabs, I propose record the tab-width in the mode-lines so that the editors will display them correctly, but not to replace the tabs with spaces, as long as the tab-width within that file is consistent.

I propose two character positions as the usual indentation width, but to allow exceptions where that improves the readability.

My reason for banning tabs is that there is no good reason to use them (disk space is cheap), and that there seems to be very little consensus as to how far apart to place the tab stops. It is very likely that two persons with different tab widths editing the same file will produce utter chaos. On the other hand I don't want to produce unnecessarily large diffs to keep forward and backward porting easy. So if we can get away with recording the tab-width in the mode-line for emacs and vim, we should not replace every tab by spaces.

The reasons for the indent width of two are 1) that seem to be most common in dune, and 2) I have grown to like it. There are many cases such as function and template parameter lists where a different indentation level produces better readability, so this should be a guideline rather then a strict rule.

There are many other indentation issues such as the placement of opening braces, whether to enforce line-breaks before the statement in a single-clause if-statement etc., but I think they are a all of marginal importance.

Are there any users of editors besides Emacs and Vim out there? Is it possible the tell your editor whether to use tabs for indentation, the indent width and the tab width via some magical comment in the file or a special file somewhere in the directory?

I think Jö hit the nail on the head. The character is the source of this evil and there is no reason for using them (except for guys who don't know their editor). Everything else (including the indentation depth) is a minor issue in comparison. It is absolutely ok to help the users of emacs and vim by mode lines. But by avoiding the use of characters, our code will look the same in every editor -- and this should be our main objective.

Banning would be nice. I also think that the number of spaces is a minor issue, same holds for the place of opening braces. As suggested one could fix the tab-width with mode lines and furthermore replace them if one does a code change.

So I suggest the following rules:

Don't use tabs anymore !
Add modelines that ban tabs and prescribe an indentation width to every new file and every file you change.
For old files: Try to examine the 'mostly used width' for the added mode line.
If there is no 'mostly used width' use X.
If you change or add code: Follow the indentation rule prescribed by the modeline.
Don't indent line that shouldn't be indentend.
Indent all lines that should be indented.
You may violate 5.-7. for good reason but not for laziness.

Rule 4 would be the fall back for the very chaotic sources. Jö suggested X=2, I'd prefer X=4 but any choice Xin{2,4} would be fine for me. Perhaps we can also agree that lines with opening/closing braces are covered by rule 6 (opposed to the GNU style).

These rules are not to invasive and might help to improve code formating in the future. Perhaps we should add a template for the modelines to the coding style page where one might adapt the width for old files.

The tab character is nowhere near the source of evil.

The issue is consistency. What standard is eventually agreed on does not matter from a technical standpoint.

Having a different mode line for every file sounds a bit silly. If every file had consistent indentation, that might sound feasible. That's not the case, though -- files will have to be touched for consistent indentation anyway. So they might as well be made to conform to a single standard, obsoleting the mode lines completely.

@Martin: I hope you didn't mean to suggest I didn't know my editor because I do use tabs.

But once you introduce tabs in file containing spaces (or the other way around) without mode lines you run into the problem. Mode lines would just prevent this by enforcing spaces (since they are more common in dune).

OK, here is the mode-line for Emacs:

-- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 --

It must be in the first line of a file (there are exception for scripts but they don't apply here) and there may be stuff before or after it (so you can make it a comment with //).

"tab-width: 4" sets a tab-stop every 4 columns "indent-tabs-mode: nil" prevents emacs from using tabs for indentation in the future "c-basic-offset: 2" sets the usual indentation depth to 2

These setting can also be made in a specially formatted block at the end of the file, or for all files in a tree via a special file in the root directory.

Does anyone know the equivalent mode-line for vim (or other editors)?

I agree with the suggestions of Carsten. The thing with the tabs is that only advanced users know how to handle them, e.g. switch from tabs to spaces and so on. Since DUNE might also be used by rookies in UNIX stuff (let's remember that we are not writing papers on UNIX) I think it would be very useful to have a common (i.e. no tabs in source files) rule and we provide help to configure the editor of choice in that way.

@Elias: Tell me more about the advantages of tabs (except makefiles) please. I never really understood what they are (were) good for. And I always was too lazy to look it up.

@Carsten: I didn't suggest anything for X. If one file has different tab widths it should be cleaned up, either by replacing every tab with space, or only the ones that don't have the most commonly used width. But that decision can be left to whoever cleans that file. Note that I make a distinction between the indentation width and the tab width.

I don't think I understand what exactly rule 5 applies to. An indentation style is a rather complex thing, I don't think you can completely put it into a mode-line. (But I may be mistaken.)

I don't understand rules 6 and 7, because they imply a guideline (rather than common sense) what should and should not be indented, and I see no reference to that guideline. Different people have different assumptions as to what should be indented.

@robertk: As I said earlier, none of the available indentation styles provides any advantage over another. In other words, they're all equally useful, as long as they indent.

I don't see how tabs are related to *nix either. It's not like the tab vs. spaces issue didn't exist in the windows world. A certain level of familiarity w/ some OS can be expected of people that are to grok templates, no?

@Elias: The comment wasn't sarcastic. I really wanted to know why you are using them, because maybe I could learn something new. Of course only if you like to tell me and amybe in a private email.

First of all I'd like to state that I did not think of any DUNE developer. I thought of the newbee trying to reaad our sources (or myself when editing fvector.hh). As for the disadvantage of tabs: The standard for the tab-size is 8, which cannot be used for indentation directry. Therefore, we need so-called smart tabs. I actually don't care how many tabs a file contains, but the tab width should be what most editors display: 8. Since we will not be able to agree on this, I think we should simply not use tab characters -- then everybody sees the same, even me.

@robertk: I didn't mean to be sarcastic either. I could be using spaces as well, it doesn't make a difference.

@Elias: I don't think we will have a different mode-line for every file. Most probably we will have a standard mode-line for most files, and some files that require special mode lines.

I too would be interested in the advantages of tabs for indentation. I also think there is a misunderstanding: when I speak of an indentation style mean how many columns I indent stuff. Whether I use tabs or spaces to reach that column is another issue. I think the amount of indentation is mostly an issue of taste (I like two columns), so arguing about which indentation style is best is rather pointless. On the matter of using tabs for indentation: I think there are good arguments against it, and I didn't hear (or didn't understand) any arguments for it. Since you seem to think that tabs do make sense for indentation, maybe you can enlighten me.

@Elias: forget my last paragraph, you already answered it in your answer to robert.

@Jö: OK, some explanations:

Rule 5 means: Don't add code with an indentation style different to a present mode line. Rule 6 and 7 mean e.g.: If you enclose something in a block - add indent. If you remove a block or 'if' - remove old indent. Currently there are a lot of files with obviously erroneous indented lines. See e.g. the example below:

UGGridGeometry()
{
mode_ = element_mode;
      jacobianIsUpToDate_ = false;
      jacobianInverseIsUpToDate_ = false;

}

Arrr - flyspray stole my indent. See the example in lines 43-48 of uggridgeometry.hh.

The mode line for vim would be

vi: set et ts=4 sw=2 sts=2:

"et" gives you spaces if you press . "ts=4" sets the tab width to 4. "sw=2" sets the standard (auto)indent width to 2. "sts=2" sets the number of spaces added and removed with and , respectively, to 2.

The mode line has to be one of the first or last lines. There might be other text before or after these options, but then 'vi:' has to be preceded with a space.

I did a survey of about the use of tabs in dune-common. Out of 83 .cc/.hh files, 28 contained tabs in the indentation. Out of those,

1 file needs major cleanup because of total chaos as a consequence of the use of tabs

1 file needs minor cleanup because of inconsistent indentation within the same block (unrelated to tabs)

26 files are self-consistent enough so the can be dealt with by merely inserting a proper modeline

Tab widths used: Tab width mainly 8: 19 files Tab width mainly 4: 7 files Tab width mainly 2: 1 file

The indentation is less consistent. Some files use different indentation widths for different levels or even different blocks, but most of the time, given the right tab-width, the code is still quiet readable. The main indentation widths are: Indentation width 2: 22 files Indentation width 4: 4 files

Attachments

tab-survey.txt

@Carsten: Lines 43-48 of uggridgeometry.hh are one of the best examples why tabs are bad for indentation.

I still don't understand rule 5. Does it apply to the indentation width only, or does it cover over parts of an indentation style?

I understand rule 6 and 7 now as: Rule 6: use (additional) indentation inside blocks. Rule 7: indent each statement inside a block consistently with the other statements inside the same block.

I would be OK with those two.

The statistics for tab-in-indentation for all modules are

module-name files-total files-with-tabs files-with-tabs-and-modelines dune-common 83 29 0 dune-grid 305 56 1 dune-grid-howto 29 22 0 dune-istl 84 65 0 dune-disc 39 30 0 dune-localfunctions 94 64 36 dune-pdelab 91 64 56 dune-pdelab-howto 51 47 21

The command for finding the total files was: find ( -type d -name .svn -prune ) -o -type f ( -name *.hh -o -name .cc ) -print| wc -l The command for finding files with tabs in indentation was find ( -type d -name .svn -prune ) -o -type f ( -name .hh -o -name .cc ) -print0 | xargs -0 egrep -l -e '^ * [ ][^ ]'| wc -l The command for finding files with tabs in indentation and emacs mode-line was find ( -type d -name .svn -prune ) -o -type f ( -name .hh -o -name .cc ) -print0 | xargs -0 egrep -l -e '^ * [ ][^ ]' | xargs egrep -nH -e '--.--'|grep :1: | wc -l

You're right rule 5 is about the width. Rule 6 and 7 also imply add exactly one indentation to the statement in non-block if/for/while/do. Perhaps there are more things I still miss. That's why I formulated it more general.

The rules are not meant to describe the style but how to handle files if we don't want to reformat all of them to one single style at once. But I see that they mix both.

Regarding the style itself we might also fix a little more than just 'no tabs, fixed number of spaces indent per file'. So more suggestions:

Indent...

...the interior of blocks.
...single statements after if/else/for/while/do/try/catch.
...class content (public/...).
...class members within a public/private/... block.
...namespace content.
...case blocks.
...broken lines.
Don't indent lines opening/closing a block (brace lines)
Use one indentations level per indent only. (Perhaps with exceptions, especially regarding 7.)

If one looks at the code everything besides 3,5, and 6 seems to be consensus. 5 is also very common. Regarding 3 and 6 I'm not so sure.

I tried the vi modeline and it didn't work. Admittedly all I know about vi is how to quit it :)

I installed vim using the Debian packages vim-gnome and opened the file https://svn.dune-project.org/svn/dune-localfunctions/trunk/dune/finiteelements/common/basis.hh. The tabs were displayed with a width of 8 instead of 4 as set in the mode-line. I also tried to put the vi mode-line in the first line before or after the emacs mode-line, with the same result.

What am I doing wrong?

@Jö: You have to activate modelines by typing ':set modeline' or 'set ml' in normal mode (the one that is active if you start vi or after pressing ). If you want to activate this permanently put the line 'set modeline' in your .vimrc . You can also activate it only for files in certain directories using a combination of autocommands (see ':h au') and 'setlocal ml'. The later activates it only for the active buffer.

The posted version uses vi-compatible syntax. However, following the vim docs, (posix) vi does not know the 'et' and 'sts' options. But this should be no show stopper - or does anyone work with nvi, elvis or the busybox vi ?

@Elias: If you don't like vimscript you can use the perl/python/ruby interface.

Emacs supports not only file-local variables but also directory-local variables. Allowing emacs to indent e.g. dune-{common,grid,istl,localfunctions} would then require only four(!) separate files.

Each file would have to be placed in a parent directory of all c++ code, thus e.g. the top directory; it would have to go by the name .dir-local.el and its contents would look something like this:

((nil . ((indent-tabs-mode . nil))) (c++-mode . ((c-basic-offset . 4) (c-file-style . "gnu"))))

This means that tabs are not used for any files and that in particular, for c++ code, the gnu indentation style of width 4 is used.

Typo: .dir-local.el should have been .dir-locals.el

Setting indent-tabs-mode to nil globally doesn't play nice with make.

show-trailing-whitespace could be controlled through .dir-locals.el as well -- at first it would be set to nil (there is currently a lot of trailing whitespace so that highlighting it would be nothing but annoying) and then to t once that's been cleaned up to keep it that way.

The result would be

((c++-mode . ((c-basic-offset . 2) (c-file-style . "gnu") (indent-tabs-mode . nil) (show-trailing-whitespace . nil))))

Something similar appears to be possible with vim: http://vim.wikia.com/wiki/Highlight_unwanted_spaces

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue:

Dear all,

I would like to add a new flavor to the discussion. Lately I stumbled upon uncrustify 1, which is a command line code formatting tool for various languages. Given a text based config file it can more or less format every aspect of the code (this is, I believe, not an exaggeration! It took us several hours to work through all settings). Such a config file can be crated in a graphical way using the universal indent gui 2. What I like about these two tools is the following:

they are available on all platforms, open source and free
the configuration lies in a text file, which can be easily distributed
the configuration covers every possible aspect (once agreed upon everyone can infer the settings for their editors)
uncrustify can be integrated into most editors (if only by defining an external tool and assigning a shortcut) and version control systems

Given such a config file, users can adapt their editors to comply with the config file (or use uncrustify directly). One could even implement uncrustify as a post-commit hook to check if commited files comply with he coding standard (if wished). I do not care that much about the actual configuration but I think it would just be great to have.

If you want to play around with uncrustify I recommend getting your hands on a recent version of both uncrustify and the universal indent gui. I also append a sample config file to play with (not a suggestion for an actual configuration!). Be sure to run uncrustify with '-l CPP', since it would otherwise interpret .hh files as C code: 'uncrustify -l CPP -c sample.cfg --replace *.hh'

I do thus ask for opinions and, if there was general agreement, would be willing to create a first configuration file base on past discussions and try to moderate an agreement upon a configuration.

Attachments

sample.cfg

I would like to have such a thing but only for new files. There are strong arguments to leave the existing files as they are. On the other hand I am annoyed by inconsistent indention - I would abolish tabs.

As a first step I'd like to see automatically reformated sources for dune-geometry because they have virtually no history.

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue:

I will start to collect information about the integration of uncrustify into various editors and version control systems. First: emacs

https://github.com/glima/Emacs-uncrustify

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue:

If we want to keep the history we can use the following tool to create new repositories and apply uncrustify to all patches in the history. We would thus have repositories with the full history (including dates, authors and stuff) and the new coding style:

http://progetti.arstecnica.it/tailor/ticket/8

mentioned in issue #576 (closed)

mentioned in issue #579 (closed)

mentioned in issue #666 (closed)

mentioned in issue #768 (closed)

mentioned in issue #776 (closed)

mentioned in issue #805 (closed)

mentioned in issue #866

mentioned in issue #900

mentioned in issue #946 (closed)

mentioned in issue #1062 (closed)

mentioned in issue #1165 (closed)

mentioned in issue #1173 (closed)

mentioned in issue #1217 (closed)

mentioned in issue #1268 (closed)

mentioned in issue #1537 (closed)

Added discussion suggestion and removed bug labels

Is still an open issue?

Will we finally close our venerable first task?

We will.

Status changed to closed

Property	Value
Reported by	Thimo Neubauer (thimo@macht.org)
Reported at	Aug 24, 2005 20:21
Type	Bug Report
Version	Git (pre2.4) [autotools]
Operating System	Unspecified / All

#2 Common code-formatting

Metadata

Description

Designs

Child items ...

Activity

Thimo Neubauer (thimo@macht.org) commented on the issue:

Attachments

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue:

Attachments

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue:

Felix Albrecht (felix.albrecht@uni-muenster.de) commented on the issue: