Qt Coding Style + clang-format + kate.git

[Christoph Cullmann]

Hi,

Dominik and me improved the clang-format file.

It is now based on what we have at my work place AbsInt.
(a slightly "improved" variant of the WebKit/Qt style)

We applied it now to the repository and it is stable.

You can apply it again by running ./reformat.sh at toplevel,
that will automatically create some reformat only commit.

Feel free to do so, if the formatting regresses again, shall
not create jitter.

Greetings
Christoph

On 2019-08-24 21:35, Christoph Cullmann wrote:
> On 2019-08-24 21:02, Dominik Haumann wrote:
>> Btw, we can discuss at Akademy again...
> Yes, we can take there a look, thought not that many people are there
> that are concerned :=)
> 
> For the issues:
> 
> I think most of them stem from the too short column limit,
> like I mentioned before, I think that should be raised to some
> reasonable amount.
> 
> That should avoid any strange re-flowing of comments, if not, we could
> turn that off
> completely.
> 
> The initializer stuff can be tuned to use , m_..., we do that at
> company, too.
> 
> For the const ll &x, yes, that is the Qt style and I would prefer it 
> that way.
> 
> Perhaps these concerns should be raised in
> 
> https://phabricator.kde.org/T11214
> 
> too, as I would prefer that we use some "common" style between the KDE
> sub-projects.
> 
> Greetings
> Christoph
> 
>> 
>> On Sat, Aug 24, 2019 at 8:53 PM Dominik Haumann <dhaumann at kde.org>
>> wrote:
>> 
>>> Hi,
>>> 
>>> I just applied clang-format to Kate's source code. And already
>>> looking at kateapp.cpp we find:
>>> 
>>> KateApp::KateApp(const QCommandLineParser &args)
>>> -    : m_args(args)
>>> -    , m_wrapper(appSelf = this)
>>> -    , m_docManager(this)
>>> -    , m_adaptor(this)
>>> -    , m_pluginManager(this)
>>> -    , m_sessionManager(this)
>>> +    : m_args(args),
>>> +      m_wrapper(appSelf = this),
>>> +      m_docManager(this),
>>> +      m_adaptor(this),
>>> +      m_pluginManager(this),
>>> +      m_sessionManager(this)
>>> 
>>> Is there an option for clang-format to prefer the old behavior? The
>>> leading comma allows us to have much cleaner diffs with better
>>> history.
>>> 
>>> Let's continue:
>>> 
>>> -    qputenv("KATE_PID",
>>> 
>> QStringLiteral("%1").arg(QCoreApplication::applicationPid()).toLatin1().constData());
>>> +    qputenv("KATE_PID",
>>> +
>>> 
>> QStringLiteral("%1").arg(QCoreApplication::applicationPid()).toLatin1().constData());
>>> 
>>> Ok, although given wide-screens I think the old variant is more
>>> readable. It gets more apparent here:
>>> 
>>> -    } else if (!m_args.isSet(QStringLiteral("stdin")) &&
>>> (m_args.positionalArguments().count() == 0)) { // only start session
>>> if no files specified
>>> +    } else if (!m_args.isSet(QStringLiteral("stdin"))
>>> +               && (m_args.positionalArguments().count()
>>> +                   == 0)) { // only start session if no files
>>> specified
>>> 
>>> I really cannot say that the new version looks better to me.
>>> 
>>> Continuing:
>>> 
>>> KMessageBox::sorry(activeKateMainWindow(),
>>> -                               i18n("The file '%1' could not be
>>> opened: it is not a normal file, it is a folder.",
>>> info.url.toString()));
>>> +                               i18n("The file '%1' could not be
>>> opened: it is not a normal file, "
>>> +                                    "it is a folder.",
>>> +                                    info.url.toString()));
>>> 
>>> Not convinced.
>>> 
>>> -    for (const auto& window : m_mainWindows) {
>>> +    for (const auto &window : m_mainWindows) {
>>> 
>>> Now, do we want the & left or right-aligned?
>>> 
>>> Let's have a look into the header file:
>>> 
>>> /**
>>> -     * Close the given \p document. If the document is modified,
>>> user will be asked if he wants that.
>>> -     * \param document the document to be closed
>>> -     * \return \e true on success, otherwise \e false
>>> +     * Close the given \p document. If the document is modified,
>>> user will be asked if he wants
>>> +     * that. \param document the document to be closed \return \e
>>> true on success, otherwise \e
>>> +     * false
>>> */
>>> 
>>> The reformatted version breaks the doxygen comment. Ok, maybe
>>> doxygen still gets it right, but the \return in the same line is
>>> imho totally wrong.
>>> 
>>> Unfortunately with current config: -2 (rejected) from my side. Maybe
>>> there is a better config, but breaking the comments this way is not
>>> worth it.
>>> 
>>> Best regards
>>> Dominik
>>> 
>>> On Sat, Aug 24, 2019 at 4:31 PM Christoph Cullmann
>>> <christoph at cullmann.io> wrote:
>>> 
>>>> On 2019-08-24 07:48, Dominik Haumann wrote:
>>>>> No, will do later today.
>>>> 
>>>> Thanks!
>>>> 
>>>> I did inspect my local result with the config again.
>>>> 
>>>> The only thing I might want to change is the
>>>> 
>>>> # Column width is limited to 100 in accordance with Qt Coding
>>>> Style.
>>>> # https://wiki.qt.io/Qt_Coding_Style
>>>> # Note that this may be changed at some point in the future.
>>>> ColumnLimit: 100
>>>> 
>>>> part.
>>>> 
>>>> For stuff with long variable + function names, you get ugly wraps.
>>>> 
>>>> In company we settled to some 240 limit.
>>>> No limit doesn't work well either, as otherwise a lot of stuff
>>>> will
>>>> just be collapsed to "LONG" lines.
>>>> 
>>>> Greetings
>>>> Christoph
>>>> 
>>>> --
>>>> Ignorance is bliss...
>>>> https://cullmann.io | https://kate-editor.org

-- 
Ignorance is bliss...
https://cullmann.io | https://kate-editor.org