Handling if's with parenthesis

[Giorgos Keramidas]

On Fri, 5 Dec 2008 15:24:51 +0200, "E.L.K." <some.any.key at gmail.com> wrote:
> Hello all!
>
> My question is:
> For example, I have a file like this:
>
> int main(){
>   if (true){
>     somefunc();
>  }
> }
>
> It is checked in to mercurial repository.  Then, two people clone this
> repo and make next changes:
>
> 1. Remove parenthesis over if's block, so result looks like:

> int main(){
>   if (true)
>     somefunc();
> }
>
> 2. Adds second function call to if's block, so result looks like:
>
> int main(){
>   if (true){
>     somefunc();
>     somefunc2();
>  }
> }
>
> Then first makes push, all ok.  Then second makes pull, merge (merge
> done automatic) and push.  Now we have next contents of this file:
>
> int main(){
>   if (true)
>     somefunc();
>     somefunc2();
> }
>
> Which is clearly not what was intended.

This is a merge problem of the developer who merged the two heads.  The
person who merged the two heads did not really do a sensible merge of
the two code blocks.

If I re-create the example merge here, with a history like this:

: keramida at kobe:/tmp/hgtest$ hg manifest
: hello.c
: keramida at kobe:/tmp/hgtest$ hg glog
: @  2[tip]:0   772af6164141   2008-12-05 16:53 +0200   keramida
: |    add extra function call
: |
: | o  1   894862a0a70b   2008-12-05 16:53 +0200   keramida
: |/     remove unnecessary braces
: |
: o  0   2465815a51b3   2008-12-05 16:53 +0200   keramida
:      add hello.c
:
: keramida at kobe:/tmp/hgtest$

Then ediff-mode of GNU Emacs, running as a 3-way merge tool, shows three
buffer windows that clearly mark (with colors) the following regions as
parts of the file that include changes:

   +-------------------------+     +-------------------------+
   |int main() {             |     |int main() {             |
=> |    if (true)            |  => |    if (true) {          |
   |        somefunc();      |     |        somefunc();      |
=> |}                        |  => |        somefunc2();     |
   |                         |  => |    }                    |
   |                         |     |}                        |
   +-------------------------+     +-------------------------+

The conflict resolution area may be a bit confusing in this case,
because it shows:

   +---------------------------------------------------------+
   |int main() {                                             |
=> |    if (true)                                            |
   |        somefunc();                                      |
=> |<<<<<<< variant A                                        |
=> |>>>>>>> variant B                                        |
=> |        somefunc2();                                     |
=> |    }                                                    |
=> |####### Ancestor                                         |
=> |    }                                                    |
=> |======= end                                              |
   |}                                                        |
   +---------------------------------------------------------+

The only indication that there may be something `odd' about the ``if
(true)'' part is that is is highlighted by a different colour, but there
is *still* a choice of which variant to pick for every little hunk of
changes.

A good merge tool can do that for you.  It can point out that something
needs more attention.  But it can't really do anything `sensible' on a
_semantic_ level.  That's the job of the human who does the merge and
checks the merge results.

What you just noticed as a bad human choice during a merge is a bit easy
to hit.  This is why it is _always_ a good idea to look at the merge
diff *after* the merge and check that everything looks sane :)