Review Request 117098: Add #include-suggestion to kdev-clang

[Jørgen Kvalsvik]

> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/missinginclude.h, line 66
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257428#file257428line66>
> >
> >     Why is that not just returning void?
> >     And use const& for the parameters

It does not return void but rather a self reference so that you can chain-set properties.
problem.setSymbol( "symbol ").setFile( file );

If this is not desirable then I'll change it to void.

--

I'm not using const as the parameters are actually copied to be set anyways, to give the compiler (and user!) a hint.


> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/missinginclude.cpp, line 48
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257429#file257429line48>
> >
> >     Call that 'isBlacklistedIncludeDir', like before?

Maybe. I'm still considering some behaviourial changes to this.


> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/missinginclude.cpp, line 118
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257429#file257429line118>
> >
> >     Can't you move these two lines to the for-condition?
> >     
> >     => "for(auto x = a.begin(), y = b.begin(); x != a.end() && y != b.end(); ++x, ++y ) {"

I originally had them there. But then the for condition got quite messy, so I moved them to the loop-and-a-half idiom. There is no issue for me moving them back.


> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/missinginclude.cpp, line 216
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257429#file257429line216>
> >
> >     That line is pretty much obvious, you don't need to state that.

Is it really? I mean, I think so because I discovered the error.

But ok, will remove.


> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/parsesession.cpp, line 277
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257430#file257430line277>
> >
> >     Can't you just re-use the original message? This looks likes unnecessary code + something that would need to be adapted quite often
> >     
> >     Also see 'prettyDiagnosticSpelling'

I could. Some of them, however, are more compiler-y and less suggestive for my taste. The other issue is that currently it will be identical to the clang problem created for the same diagnostic, which doesn't have a solution.

Once the suggestions are fine I'll write a patch that refcators problem creation and basically removes clang suggestion.

Shall I still reuse for this one?


> On March 27, 2014, 8:53 a.m., Kevin Funk wrote:
> > duchain/parsesession.cpp, line 296
> > <https://git.reviewboard.kde.org/r/117098/diff/1/?file=257430#file257430line296>
> >
> >     Check if symbol is non-empty

If it is empty, then the issue shouldn't have been created to begin with. I'll see if I can move this check to before the problem object is even constructed.


- Jørgen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117098/#review54266
-----------------------------------------------------------


On March 26, 2014, 7:48 p.m., Jørgen Kvalsvik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117098/
> -----------------------------------------------------------
> 
> (Updated March 26, 2014, 7:48 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Add #include-suggestion features to kdev-clang
> 
> 
> Diffs
> -----
> 
>   duchain/CMakeLists.txt d21011a 
>   duchain/missinginclude.h PRE-CREATION 
>   duchain/missinginclude.cpp PRE-CREATION 
>   duchain/parsesession.cpp eee9fb1 
> 
> Diff: https://git.reviewboard.kde.org/r/117098/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jørgen Kvalsvik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140327/1dbfd036/attachment-0001.html>