Fix for makebuilder not opening properly files referenced by errors/warnings

[Andreas Pakulat]

On 16.08.08 22:52:50, dizzy wrote:
> Because kdev4 does not (and as I understand will not) have support for 
> automake based projects

Thats due to nobody stepping up to do the work. If somebody wants to
work on it we'd be happy to include it, but its a strange buildsystem
and few people fully understand it :)

> I am using the custom makefile support (and will be 
> using it for some time now). One of the many problems I have noticed is that 
> when building if make changes directories and errors/warnings are printed 
> inside such a directory, when trying to click on such a message to open the 
> relevant file it won't open (and errors in the console showing it tries to 
> open a relative path). The problem is it tries to open that relative path but 
> from the project root dir which is not the directory where make was at the 
> moment of the error generation. The attached patch fixes the problem.

Hmm, makes me wonder wether cmake prints absolute paths for errors or
why did I never have a problem with that - anybody?

> The patch should also fix some other issues with the old code where a KUrl for 
> openDocument() was created from the path QString although this type of 
> construction requires the given QString to be encoded (and it wasn't from what 
> I could gather). Current code uses the documented KUrl::fromPath() 
> construction method that is said to be used with unencoded path strings.

Before I now tear apart the patch itself, let me first thank you for
working on the code. It really needs some love.

> ===================================================================
> --- buildtools/builders/makebuilder/outputfilters.h	(revision 848068)
> +++ buildtools/builders/makebuilder/outputfilters.h	(working copy)
> @@ -13,6 +13,7 @@
>  
>  class IOutputViewItem;
>  class MakeBuilder;
> +class MakeOutputModel;
>  class QString;
>  // template <typename T1> class QList;
>  #include <QList>
> @@ -23,23 +24,26 @@
>  class ErrorFilter
>  {
>  public:
> -    ErrorFilter( );
> -    virtual ~ErrorFilter();
> +    explicit ErrorFilter( MakeOutputModel const& model );
> +    ~ErrorFilter();
>  
> -    virtual QStandardItem* processAndCreate( const QString& line );
> +    QStandardItem* processAndCreate( const QString& line );
>  
>  private:
> +    MakeOutputModel const& m_model;

That should be a pointer not a const&.

> Index: buildtools/builders/makebuilder/makeitem.h
> ===================================================================
> --- buildtools/builders/makebuilder/makeitem.h	(revision 848068)
> +++ buildtools/builders/makebuilder/makeitem.h	(working copy)
> @@ -38,6 +38,7 @@
>      QString file;
>      int lineNo;
>      QString errorText;
> +    QString currDir; // store current directory if relative path is stored in "file"

Why not store the absolute path in file instead of a relative one, isn't
it used just for opening the file?

> +            url = KUrl::fromPath( warn->currDir );
> +            url.cd( warn->file );

Interesting that this actually works. For readability reasons I'd
suggest to change it into addPath() nonetheless - unless that breaks
something.

> +        // FIXME: is this portable?
> +        if (!file.isEmpty() && file[0] != '/')

No its not portable, the right thing to do is use
QFileInfo::isRelative()

The rest seems to look fine.

Andreas

-- 
A few hours grace before the madness begins again.