I used the reindent.py tool to fix indentation in the translate directory (and pddl subdirectory). A nifty little tool I must say. Did the job really well as far as I can tell by 
looking at the meld diffs. Translation of gripper:prob01.pddl still works, I hope nothing got broken.

I added the script to the misc directory.

Closing this ticket.

Looks like there already is a perfect tool for this job:
http://pypi.python.org/pypi/Reindent/0.1.0

It's public domain, so we could include it in our repository (e.g. in the new
misc directory).

Modified uncrustify-all a bit, so that you can specify which files it should
work on. By default, it works on the current directory and its subdirs. See the
explanations at the beginning of the file for details.

I think this leaves only the Python sources to be cleaned up, so I'm reassigning
this to Jendrik.

OK, preprocessor uncrustified in r4985.

I also added a script to "misc" that runs uncrustify on all C++ sources. On a
"clean" source, this should not have any effect.

WARNING: this modifies the sources in place, so only use it if you trust it
and/or after doing backups.

WARNING FOR THE WARNING: If you do backups, store them outside
downward/preprocess or downward/search, or else they will be modified, too. (The
script uses find to find all *.cc and *.h files in the downward/preprocess and
downward/search trees.)

Still to do:

 * Unify indentation of translator (=> Jendrik).
 * Uncrustify the preprocessor (=> Malte).

There are some small uglinesses in the uncrustified code that we might want to
discuss. Also, it's worth doing a revmeld on r4981 to learn more about our
preferred style.

OK, uncrustification on the search code is now idempotent. Run as follows:

# uncrustify --replace --no-backup -c ../../misc/downward-uncrustify.cfg
*.{h,cc} */*.{h,cc}

(first update to make sure you have the misc directory with the correct config).
If this doesn't work, check your uncrustify version. I'm using 0.56.

Uncrustified the search code in r4981. That was a *giant* changeset because
there were all sorts of things wrong with the code (not just indentation).

Unfortunately, uncrustify appears not to be fully idempotent, which is bad if we
want to run it automatically: if I run it again on the already uncrustified
source, it will insert additional blank lines in a few places (10 altogether
across all source files).

Fortunately, this only happens in contexts where the formatting is messed up
anyway because we violate our rule "in a block which has a sub-block which uses
braces, always use braces." That is, uncrustify gets confused by this:

    if (foo)
        while (bar) {
            ...
        }

and inserts an extra blank line, but we shouldn't use such code anyway; we
should write

    if (foo) {
        while (bar) {
            ...
        }
    }

instead. (Omitting braces around the body of statement X is *only* permissible
if the body doesn't itself contain braced statements.)

I'll be fixing this manually.

Jendrik will look into the Python source.

We should also use four spaces to indent in the Python code uniformly.
There are still a few old source files that use two spaces, I think.

I've merged the content of the CODING-CONVENTIONS file onto the wiki page
and removed the file from the repository.

If there's any stuff you have questions about, let me know. The easiest way is
to add the question directly to the wiki (Erez, you will need to generate the
wiki account "ErezKarpas" first, and someone will need to activate that account).

On the other hand, if there's something you *don't* have questions about, don't
add it to the wiki, i.e., I'd prefer only to described those coding conventions
that are not clear to us anyway, so that the amount of stuff isn't larger than
necessary. (That is, grow on demand.)

Saying nothing means agreement, so I am for option A.

Gabi, your opinion on the A or B question in msg366? If B, how strong is the
preference? I was asking because the code is currently a mix of A (Java style)
and B (K&R style) and we have to decide somehow. I prefer A, but would be fine
with switching to B if there's a very strong preference.


Both of your comments are related to the fact that the position of line breaks
within expressions etc. is left alone for the most apart; only the indentation
is changed. The bad thing about that is that there'll be some manual work
involved. The good thing is that it means we have some liberty in breaking
lines, which should help with the mid-term goal of making uncrustify idempotent
on the codebase (when it's OK) so that it can be used for automated checking.


With regard to Erez's comment, we'll have to deal with this manually. The
problem with removing this line break automatically (which uncrustify can do) is
that in some cases it can lead to very long lines, and there we want a line
break. I've opened a wiki page for the coding conventions:
    http://alfons.informatik.uni-freiburg.de/downward/CodingConventions
which explains how and when to break such lines.

With regard to Gabi's comment, uncrustify will indent long expressions like this:

    my_function_call(foo, bar,
                     baz, bla,
                     xxx);

if the argument list has been started on the first line, but as

   my_function_call(
       foo, bar, baz,
       bla, xxx);

if there's only an open parenthesis on the first line.

Looks good. The only thing is that it sometimes leads
to very long lines, e.g.:

option_parser.add_heuristic_list_option("preferred",
                                        &preferred_list, "use preferred
operators of these heuristics");

Overall it looks good.
I think it missed this:

void
GeneralEagerBestFirstSearch::set_pref_operator_heuristics(

I've been playing around with the "uncrustify" prettifier a bit and have started
moving towards an uncrustify configuration file for our code base. The current
version is in trunk/downward/search/uncrustify.cfg for convenience; should later
move somewhere else. This is still work in progress.

I have tentatively applied uncrustify to one source file
(general_eager_best_first_search.cc). For the most part, I like what I see so
far. The versions before and after applying uncrustify are attached to this
issue. They are also in the repository (r4517 before, r4518 after).

I'd like you to do a
# meld before-uncrustify.cc after-uncrustify.cc
and have a look at what was changed. Make sure to disable meld's options for
ignoring whitespace changes (set everything in Edit/Preferences/Text Filters to
"not active"), or you'll only see a very small fraction of the changes.

If the motivation for any of the changes is unclear, let me know and I'll
explain them in our coding conventions file. (I'd rather not document
everything, but only the stuff that isn't obvious.)

There are a few more formatting issues in the file that need manual
intervention; e.g. this one (probably will not display here properly because of
the long line):

    cout << "Conducting best first search" <<
    (reopen_closed_nodes ? " with" : " without") << " reopening closed nodes" <<
endl;

but let's look at the automatic stuff first. The medium-term goal is to
uncrustify the whole codebase.

A

Hi guys, I'm currently playing around with some automatic indentation/code
formatting tools. I'd like to hear your opinions on some things.

Background knowledge: We'll definitely use this brace style for
if/while/for/class etc.:

   if (foo) {
       bar;
       baz;
   }

Given that, which style for function/method definitions do you prefer:

(A):

void Foo::bar(int baz) {
   // ...
}

(B):

void Foo::bar(int baz)
{
   // ...
}

?

Change indentation in C++ and Python code to four spaces, not using tab
characters. (This should already be the prevalent convention, but isn't used
everywhere.)

Also change spacing in C++ blocks from "if/while/for(x)" to "if/while/for (x)",
i.e. add a space before the condition.

Should do this after the big merge, since it'll mess up all the diffs.