Review Request: Remember URLs in select core file dialog

Andreas Pakulat apaku at gmx.de
Mon Feb 6 08:43:59 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103877/#review10367
-----------------------------------------------------------


I think the startDir should be remembered across sessions and maybe even the core-file, there might be people who use the kernel-settings to ensure that core-file names are always the same. That would also avoid the necessity for the class-statics.


debuggers/gdb/selectcore.ui
<http://git.reviewboard.kde.org/r/103877/#comment8532>

    Is there a particular reason you removed the minimum size here?



debuggers/gdb/selectcoredialog.h
<http://git.reviewboard.kde.org/r/103877/#comment8534>

    I don't like this, the usage of the static's gives no hint why they're put. It almost looks like an error to a bystander, who may assume you wanted member variables.
    
    So at least a big fat comment is needed here I think. Maybe even better would be if the caller would store the url's and feed them into the dialog since the caller probably lives a bit longer than the dialog.



debuggers/gdb/selectcoredialog.cpp
<http://git.reviewboard.kde.org/r/103877/#comment8535>

    Did you try what happens when happens when startDir and url are not equal and I use the browse-button?



debuggers/gdb/selectcoredialog.cpp
<http://git.reviewboard.kde.org/r/103877/#comment8536>

    I think it would be better to override the accept function instead of the getters to store the url. That allows the getters to stay const and they're not doing some hidden magic


- Andreas Pakulat


On Feb. 5, 2012, 11:08 p.m., Gerhard Stengel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103877/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2012, 11:08 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> This patch addresses the problem that the URLs of the core file and the executable weren't remembered. Now, it remembers the URLs during a session.
> The start URL is set to the project base dir if the KUrlRequester is empty. It furthermore sets a minimum width for both KUrlReuesters, they were too narrow.
> 
> 
> Diffs
> -----
> 
>   debuggers/gdb/selectcore.ui c98a641 
>   debuggers/gdb/selectcoredialog.h 74cd329 
>   debuggers/gdb/selectcoredialog.cpp 7298682 
> 
> Diff: http://git.reviewboard.kde.org/r/103877/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gerhard Stengel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120206/3faa1c35/attachment.html>


More information about the KDevelop-devel mailing list