Review Request 121456: GDB plugin enhancements and some command queue optimizations

[Milian Wolff]

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



debuggers/gdb/disassemblewidget.h
<https://git.reviewboard.kde.org/r/121456/#comment50285>

    while at it: remove this newline, and also the whitespace of the line below please



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50286>

    I actually think that this, and the text above and below, should be tooltips, not whatsthis - no? Hovering the button should show you this explanation and not require to use the esoteric whats-this shortcut.
    
    also, please say "disassembly flavor", not style here - to stay consistent.



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50287>

    whitespace before SIGNAL, and newline before widget



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50288>

    style:
    
        if (...) {
            return;
        }



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50289>

    whitespace after if



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50290>

    remove spaces before )



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50291>

    merge conditionals:
    
        if (active_ && r.reason == "done") {
            ...
        }



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50292>

    style like mentioned above



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50293>

    again, remove spaces before )



debuggers/gdb/disassemblewidget.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50294>

    style: add braces and whitespace after if/else



debuggers/gdb/gdbcommand.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50295>

    while at it, please break this onto multiple lines.
    
        GDBCommand::...()
            : type_(type)
            , ...(...)
            , ...(...)
            ...
        {}



debuggers/gdb/gdbcommand.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50296>

    this stuff is only for debugging purposes, or? should it really always run?



debuggers/gdb/gdbcommandqueue.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50298>

    style:
    
        if (...) {



debuggers/gdb/gdbcommandqueue.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50297>

    if at all possible, I'd prefer stl-like iterators:
    
        auto it = m_commandList.begin();
        while (it != m_commandList.end()) {
            auto currentCmd = *it;
            if (...) {
                delete currentCmd;
                it = m_commandList.erase(it);
            } else {
                ++it;
            }
        }
    
    but I now see that below the Java style was already used. anyhow, not that important



debuggers/gdb/memviewdlg.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50299>

    whitespaces after comma



debuggers/gdb/memviewdlg.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50300>

    join line



debuggers/gdb/memviewdlg.cpp
<https://git.reviewboard.kde.org/r/121456/#comment50301>

    join line


- Milian Wolff


On Dec. 15, 2014, 7:29 a.m., Andreas Roth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121456/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2014, 7:29 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> Repo is available at https://github.com/aroth-arsoft/kdevelop/tree/4.7-gdb-enhancements
> - switch the assembler flavor from AT&T to Intel and back in the dissassembler widget
> - remove StackList commands from the queue when a new Exec command arrives. This reduces the wait time when stepping through the code quickly.
> - remove duplicated command in the queue. Mostly to eliminate duplicate variable updates.
> - show the pending command in the command queue. This is purely for debugging and to check which commands are currently pending.
> - slight improvements to memory view to use sizeof() when no size argument is given
> 
> 
> Diffs
> -----
> 
>   debuggers/gdb/debugsession.cpp edc0d34473e99230db510457fd609a178bc8fa34 
>   debuggers/gdb/disassemblewidget.h ccffc2f0325aac8c8486ff413ed64219548461c3 
>   debuggers/gdb/disassemblewidget.cpp 91194ed7e69ff87ebdfbb5383c1b38939760d098 
>   debuggers/gdb/gdb.cpp 46420876149604a318caec7111f252dcc12b39b4 
>   debuggers/gdb/gdbcommand.h e1ebc6988fb508a187d2775735c13b9123fcda04 
>   debuggers/gdb/gdbcommand.cpp 67abbd87f1f0d10a84a45b3ee9f104c77ffe925b 
>   debuggers/gdb/gdbcommandqueue.h 7d82f45a244d9fffb9b3a2ba5a82eaaa06f5c8d0 
>   debuggers/gdb/gdbcommandqueue.cpp 801bc41e79f62a226de04cd1a91500c86a9cf1a3 
>   debuggers/gdb/memviewdlg.cpp cf9d0b49ea0b617b405a5065354af9c9e59afa8d 
> 
> Diff: https://git.reviewboard.kde.org/r/121456/diff/
> 
> 
> Testing
> -------
> 
> Only preliminary testing done; haven't checked the unit-tests yets
> 
> 
> Thanks,
> 
> Andreas Roth
> 
>

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