Changing product to kate.

Yes, it's broken, but the C++/boost indenter is so super strange that I'm not sure if it's worth fixing. Just use the normal C++ indenter instead.

What "normal" C++ indenter do you mean? I only have "C style" and
"C++/boost style". The former doesn't indent class methods properly and
the latter is broken.
Also for some reason all new files use the C-style indenter, even when
set to C++ mode until I close and reopen the file (with "Override Kate
indentation mode" turned on).

Yes, C style. What do you mean by "doesn't indent class methods properly"?

The C style indenter is the normal indenter for C/C++. The C++/boost style indenter is some really specific tool with lots of assumptions on your code style that generally (imo) do not apply.

Well, yeah, what I said. I want to have

class Foo
{
public:
    void foo()
    {
        |
    }
};

but with the C-style indenter I end up with

class Foo
{
public:
    void foo()
{
    |
}
};

even when I initially was at the correct indentation level, i.e. there:

class Foo
{
public:
    void foo()
    {|
};

As soon as I hit enter, the braces are automatically unindented.

I think this should be fixed in the C-style indenter.

Either there or in the C++/boost indenter. All other indenters seem to work fine with the automatic brace placement, but either don't indent my cursor inside the braces, i.e.,

class Foo
{
public:
    void foo()
    {
    |}
};

 or do some very funky stuff such as

class Foo
{
public:
    void foo()
    {
        |}
};

Actually, what I said is not quite correct. Not the enter key unindents the braces, but the completion of the closing brace. As soon as I type "{", the line gets unindented:

class Foo
{
public:
    void foo()
    |
};

entering "{" leads to

class Foo
{
public:
    void foo()
 {|}
};

and then enter behaves correctly, but the line is already unindented:

class Foo
{
public:
    void foo()
 {
    |
}
};

If the braces are already there and I just hit enter, they stay in place:

class Foo
{
public:
    void foo()
     {|}
};

now enter results in the desired

class Foo
{
public:
    void foo()
     {
         |
    }
};

I think I know why this is: typing the closing brace triggers a deindent -- which makes sense, but is wrong in this case. Also this needs to be fixed in the c style indenter.

We should add a unit test for this with // kate: auto-brackets on;

PS: this mode line needs to be added first again, since it seems this was removed in KDE 4.10, when the auto-brackets were removed...

PS: I just added the auto-brackets modeline again, will be in KF 5.28.

Janek Bevendorff,

I've tried your buggy snippet w/ kate 4, and can't reproduce... unfortunately I have no idea whats changed in KF5/Plasma5 version of kate...

I still use C++/boost indenter w/ Kate 4 and can't migrate while there is no Python support %(

I'll fix as soon as it would be possible to me to move to kate5.

I downloaded a KDE Neon ISO to test this in KWrite and make sure it's
reproducible on other systems. I can indeed reproduce both issues:
C++/boost inserting {; instead of {} and C unindenting the line.

BTW C indenter also indents incorrectly in if statements when you want
the brace to be in its own line.

When I have this code

class X
{
private:
    void foo()
    {
        if (true) |
    }
};

and press enter, I get

class X
{
private:
    void foo()
    {
        if (true)
            |
    }
};

which is okay. But when I type an opening brace, I get

class X
{
private:
    void foo()
    {
        if (true)
    {|}
    }
};

This can't even be corrected by pressing Ctrl+Z. The unindentation for
the method foo() can be corrected this way, but when I press Ctrl+Z
here, I end up with

class X
{
private:
    void foo()
    {
        if (true)
            {|}
    }
};

When I have the opening braces on the same line as the method or if
condition header, indentation works as expected.

I played a little with the ktexteditor code. The culprit for the C-style
indenter is this part (line 774):

    } else if (firstPos == column - 1 && c == '}') {
        var indentation = findLeftBrace(line, firstPos);
        if (indentation == -1)
            indentation = -2;
        return indentation;
    }

When I manually enter "{}", then firstPos is 4 and column is 6. However,
when using automatic bracket completion, my cursor is at 5, so the
condition is true and findLeftBrace() is called which retrieves a cursor
to the preceding brace like that:

var cursor = document.anchor(line, column, '{');

But since the cursor is before the insert position, this always results
in a (1, 0) cursor and therefore always unindents the line. My fix would
be to extend the initial check with one more condition:

} else if (firstPos == column - 1 && c == '}' &&
           document.charAt(line, firstPos) == '}') {

This would simply check if the inserted character is actually where the
cursor is. If not, it was probably inserted automatically.

The C++/boost indenter seems to be a lot more complex and I haven't
looked into that issue yet.

Thanks a lot for looking into that! Could you maybe be so kind and put a patch up on phabricator.kde.org? Ideally, you could add a test for the new behaviour :)

I can't get Kate (and therefore the editor) to respect the auto-brackets on modeline. Any tips?

Okay, I fixed it in C++/boost style as well. However, that indenter is overall a little broken. The only effect auto brackets had on it was that it inserted semicolons after opening braces. I fixed the semicolons and some over-indentation after condition or loop headers, but not the rest.  It's not generating bullshit code anymore, but getting it to not ignore automatically inserted closing braces seems to be beyond reasonable effort.

However, I still have the problem, that I can't produce proper unit tests, because the auto bracket modeline seems to be ignored. Otherwise I'd already submit the patches to Phabricator.

@Alex: This issue probably also exists on KDE 4, so you can look into this already now. Besides that, you could also just use Sven's AppImage http://files.svenbrauch.de/kate-linux/ which gives you a recent Kate out of the box on every system.

@Janek: Can you please attach the patch, or of that's not possible, you modified cppstyle.js and/or cstyle.js indenter?

Created attachment 101608 [details]
Indenter Patch

Yes, sure, I can append it here. The patch I uploaded includes the failing unit tests (because the auto bracket modeline) is ignored and is therefore not ready for merging yet.

Since I couldn't leave it alone, I also experimented a little more with the C++ indenter. It had some functionality to consume characters to allow some sort of overwrite mode for brackets. That's pretty useful, but it completely breaks auto brackets. With a resolution for bug #370716 this should be a feature of the editor component anyway. So I removed that. It causes one unit test which tests comma consumption to fail, but not in a way that I would say the new behavior is buggy. With this consumption feature removed and a few more fixes to semicolon placement, auto brackets seem to work properly even with the C++ indenter. What do you think?

Patch looks sensible to me and even has tests, if there are no objections I would simply apply it.

Well, it has tests, but they fail (actually, the C++ ones don't, but
only because I haven't adjusted them yet to my latest changes in the
indenter which allow for auto brackets).
Additionally, one already existing test for the C++ indenter fails now
as I said above, but not in a way that would consider buggy.
The reason why my tests fail is still that I can't get the test
environment to enable auto brackets.

Feel free to just adjust that test to the new behaviour, if you consider the new behaviour sensible.

To enable autobraces, try view->config()->setAutoBrackets(true).

Thanks for the tip. I added a new Q_INVOKABLE to expose setAutoBrackets() to test scripts. The only possible pitfall I see here is that you have to explicitly disable auto brackets again at the end of your test, because the config is taken over to successive tests. I hope this won't backfire should "auto brackets on" become a default setting at some point. But in that case most tests would fail anyway, so not a big deal I guess.

I submitted a patch with all the changes to Phabricator: https://phabricator.kde.org/D3104

Git commit acd1b4581468684f927f014c6a5e14c7da643449 by Dominik Haumann.
Committed on 20/10/2016 at 17:59.
Pushed by dhaumann into branch 'master'.

Fix CStyle and C++/boost indenters when automatic brackets enabled

Thanks to Janek Bevendorff for the patch!

FIXED-IN: KDE Frameworks 5.28
Differential Revision: D3105

A  +7    -0    autotests/input/indent/cppstyle/autobrackets1/expected
A  +6    -0    autotests/input/indent/cppstyle/autobrackets1/input.js
A  +4    -0    autotests/input/indent/cppstyle/autobrackets1/origin
A  +10   -0    autotests/input/indent/cppstyle/autobrackets2/expected
A  +9    -0    autotests/input/indent/cppstyle/autobrackets2/input.js
A  +6    -0    autotests/input/indent/cppstyle/autobrackets2/origin
A  +9    -0    autotests/input/indent/cppstyle/autobrackets3/expected
A  +8    -0    autotests/input/indent/cppstyle/autobrackets3/input.js
A  +7    -0    autotests/input/indent/cppstyle/autobrackets3/origin
M  +1    -1    autotests/input/indent/cppstyle/comma4/expected
A  +7    -0    autotests/input/indent/cstyle/openpar11/expected
A  +5    -0    autotests/input/indent/cstyle/openpar11/input.js
A  +6    -0    autotests/input/indent/cstyle/openpar11/origin
A  +7    -0    autotests/input/indent/cstyle/openpar12/expected
A  +6    -0    autotests/input/indent/cstyle/openpar12/input.js
A  +4    -0    autotests/input/indent/cstyle/openpar12/origin
M  +8    -1    autotests/src/testutils.cpp
M  +6    -0    autotests/src/testutils.h
M  +27   -10   src/script/data/indentation/cppstyle.js
M  +8    -2    src/script/data/indentation/cstyle.js

http://commits.kde.org/ktexteditor/acd1b4581468684f927f014c6a5e14c7da643449

@Janek: Really good patch, even with unit tests - keep it up! :)

Thanks. I'm a long-term KDE user and contributor to this bug tracker,
but haven't really submitted any patches so far. :-)