[poppler] Source code beautifiers

[Jakub Kucharski]

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
> > poppler at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/poppler
> > 
>