Review Request 115600: Added buttons to jump to next/prev outputmark in an outputwidget

Sven Brauch svenbrauch at googlemail.com
Sun Feb 9 18:10:38 UTC 2014



> On Feb. 9, 2014, 5:12 p.m., Sven Brauch wrote:
> > The feature certainly makes sense.
> > 
> > Doesn't the outputview already have actions which do this? Because when you, like this, create new actions, you won't have e.g. the same shortcut associated with them. It would be better to add the existing actions (which are also in the global action collection and thus e.g. in the configure shortcuts dialog) to the toolbar.
> > 
> > Also, your patch introduces whitespace errors, please avoid that ;)
> > 
> > Thanks,
> > Sven
> 
> Todd Nowacki wrote:
>     I'm fixing the added whitespace. Do you want me to add the needless whitespace back that was there before the diff?
>     
>     And I can't seem to access actionCollection() in outputwidget.cpp, any advice?
>     If not, I can pass the actions to the widget when they are created in StandardOutputView.

data->plugin->actionCollection() should do hopefully, try that.

Cleaning whitespace errors is fine but please do not mix formatting changes with code changes (except in the lines you edit anyways). If you want to fix formatting, create a separate commit. The reason is that when the code changes you did shall later be reverted, cherry-picked into a different branch etc. the whitespace changes will always come with that. In the case of a revert, the formatting changes are lost too; in the change of a cherry-pick or merge they might cause conflicts etc.


- Sven


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


On Feb. 9, 2014, 5:07 p.m., Todd Nowacki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115600/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2014, 5:07 p.m.)
> 
> 
> Review request for KDevelop and Alexander Dymo.
> 
> 
> Bugs: 330206
>     http://bugs.kde.org/show_bug.cgi?id=330206
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This is in response to https://bugs.kde.org/show_bug.cgi?id=330206
> 
> I added buttons to all output widgets instead of the one for the build tool view; we can easily restrict this to a certain type of view if we want to. 
> 
> The only problem with the buttons is that they are enabled even if there are no outputmarks to jump to. At the time of creating this diff, I did not see an easy way to find the number of outputmark.
> 
> 
> Diffs
> -----
> 
>   plugins/standardoutputview/outputwidget.h 98cd94988b9a278b6509702c18956e580226ffc9 
>   plugins/standardoutputview/outputwidget.cpp 7f248c4924ba4c2129f1f1fa7b28d7fd6ab6a7ed 
> 
> Diff: https://git.reviewboard.kde.org/r/115600/diff/
> 
> 
> Testing
> -------
> 
> The buttons work for the build window.
> 
> 
> Thanks,
> 
> Todd Nowacki
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140209/48936bde/attachment.html>


More information about the KDevelop-devel mailing list