Qt Coding Style + clang-format + kate.git

[Dominik Haumann]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190824/4c56eb71/attachment-0001.html>