On 28/06/16 02:39, Jakub Kucharski wrote:
> Hello, everyone!
>
> Poppler has a very diverse codebase if it comes to coding style. In
> some places it gets really weird, e.g. the qt frontends, where we have
> code like this ("▸---" marks a tab):
>
> bool Document::unlock(const QByteArray &ownerPassword,
> ▸---▸---▸--- const QByteArray &userPassword)
> {
> ▸---if (m_doc->locked) {
> ▸--- /* racier then it needs to be */
> ▸--- DocumentData *doc2;
> ▸--- if (!m_doc->fileContents.isEmpty())
> ▸--- {
> ▸---▸---doc2 = new DocumentData(m_doc->fileContents,
> ▸---▸---▸---▸---▸---new GooString(ownerPassword.data()),
> ▸---▸---▸---▸---▸---new GooString(userPassword.data()));
> ▸--- }
> ▸--- else
> ▸--- {
> ▸---▸---doc2 = new DocumentData(m_doc->m_filePath,
> ▸---▸---▸---▸---▸---new GooString(ownerPassword.data()),
> ▸---▸---▸---▸---▸---new GooString(userPassword.data()));
> ▸--- }
> ▸--- if (!doc2->doc->isOk()) {
> ▸---▸---delete doc2;
> ▸--- } else {
> ▸---▸---delete m_doc;
> ▸---▸---m_doc = doc2;
> ▸---▸---m_doc->locked = false;
> ▸---▸---m_doc->fillMembers();
> ▸--- }
> ▸---}
> ▸---return m_doc->locked;
> }
>
> With code like this I just end up copy-pasting things hoping that I
> manage to guess the right way to format the code. So I've thought we
> could make things easier by using a source code beautifier and
> providing config files for it. We could either unify the coding style
> or we could have different config files for core, goo, qt etc. (I'd be
> in favor of unification.)
>
> Two source code beautifiers that I know of are Uncrustify[1] and
> ClangFormat[2]. For the second one there is a web page[3] which makes
> its configuration a piece of cake and there is a python script making
> it easy to integrate it with vim or any other highly configurable
> editor. However Uncrustify seems to be more configurable. Flatpak devs
> have a useful shell script making it easier to invoke Uncrustify[4].
>
> What do you think?
I would prefer we didn't do a mass reformat of the code as it makes it
harder to find the author or commit responsible for a line of code. If
we do regular code reformatting, git blame will always resolve to the
last reformat.
It would be better to include the config required to make editors follow
the poppler style so the code is correctly formatted at the time it is
written. For example with emacs it is possible to insert a comment like:
/* -*- Mode: c++; c-basic-offset: 2; indent-tabs-mode: nil; -*- */
to specify the indentation style for a file. Or even better, use
.dir-locals.el to specify the style for a source tree.
I don't care if qt/glib/cpp want to do their own style. The rest of the
code should be consistent with the poppler style.
>
> Jakub
>
> [1] http://uncrustify.sourceforge.net/
> [2] http://clang.llvm.org/docs/ClangFormat.html
> [3] http://clangformat.com/
> [4] https://github.com/flatpak/flatpak/blob/master/uncrustify.sh
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/poppler
>