search

codilime / veles

Binary data analysis and visualization tool
https://veles.io
Apache License 2.0
1.14k stars 117 forks source link

Collapse chunks on double clicks. #411

Closed mwarzynski closed 6 years ago

mwarzynski commented 6 years ago

This change is Reviewable

chivay commented 6 years ago

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

src/ui/disasm/row.cc, line 76 at r1 (raw file):

void Row::mouseDoubleClickEvent(QMouseEvent* event) {
  this->window_->chunkCollapseToggle(this->id_);
}

Not sure about this method of calling chunkCollapseToggle. What about adding a signal to Row and connecting it with some slot in Widget?

Comments from Reviewable

chivay commented 6 years ago

Reviewed 1 of 5 files at r1. Review status: 0 of 5 files reviewed at latest revision, 2 unresolved discussions.

src/ui/disasm/row.cc, line 76 at r1 (raw file):

Previously, chivay (Hubert Jasudowicz) wrote…
Not sure about this method of calling `chunkCollapseToggle`. What about adding a signal to Row and connecting it with some slot in Widget?

OK

src/ui/disasm/widget.cc, line 98 at r2 (raw file):


    QObject::connect(r, SIGNAL(chunkCollapse(const ChunkID&)), this,
                     SLOT(chunkCollapse(const ChunkID&)));

Please, don't use old connect syntax https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_QObject_member

Comments from Reviewable

chivay commented 6 years ago

Review status: 0 of 5 files reviewed at latest revision, 3 unresolved discussions.

include/ui/disasm/row.h, line 57 at r1 (raw file):

  QLabel* comment_;

  Window* window_;

Remove unused field.

Comments from Reviewable

chivay commented 6 years ago

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

include/ui/disasm/widget.h, line 62 at r1 (raw file):


  Arrows* arrows_;

Trailing whitespace

Comments from Reviewable

chivay commented 6 years ago

Reviewed 1 of 5 files at r1, 1 of 4 files at r2, 3 of 3 files at r3. Review status: all files reviewed at latest revision, 4 unresolved discussions.

include/ui/disasm/row.h, line 57 at r1 (raw file):

Previously, chivay (Hubert Jasudowicz) wrote…
Remove unused field.

OK

src/ui/disasm/widget.cc, line 98 at r2 (raw file):

Previously, chivay (Hubert Jasudowicz) wrote…
Please, don't use old `connect` syntax https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_QObject_member

OK

Comments from Reviewable