On Wed, 2016-06-29 at 21:30 +0930, Adrian Johnson wrote:
> 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.These solutions are editor-specific. We could reformat just the new commits[1]. J. [1] http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reform atting > > 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 > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/poppler > > > _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
