Fixes to GrepOutputView toolbar
ClosedPublic

Authored by croick on Jul 7 2017, 7:42 AM.

Details

Summary

Fixes to GrepOutputView toolbar

  • fix crash when aborting grep jobs at startup by clicking on "Clear Search History" (introduced in commit 42eb6bb62c28)
  • correctly disable result navigation buttons (fixes crash when clicking on "Collapse All" button for an empty result)
Test Plan
  • quit KDevelop with a long GrepJob in history
  • restart KDevelop and abort job by clicking on "Clear Search History" -> no crash
  • start a long GrepJob and stop it by clicking on "Clear Search History"
  • navigation buttons are now disabled

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
croick created this revision.Jul 7 2017, 7:42 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJul 7 2017, 7:42 AM
kfunk requested changes to this revision.Jul 7 2017, 7:53 AM
kfunk added a subscriber: kfunk.
kfunk added inline comments.
plugins/grepview/grepdialog.cpp
426

This doesn't really change behavior,does it?

plugins/grepview/grepdialog.h
56–57

nextHistory(false) is never called(?)

plugins/grepview/grepoutputview.cpp
77

To be honest I don't think these icons are an improvement. Now they're pretty much independent (i.e. you don't clearly see they're connected, cf. collapse/expand).

I'd rather keep the old ones.

354

Turn that into a updateButtonStates() and do something along

const bool enabled = model()->hasResults();
m_prev->setEnabled(enabled);
...

Then call it from GrepOutputView::expandElements and other places?

This revision now requires changes to proceed.Jul 7 2017, 7:53 AM
croick updated this revision to Diff 16322.Jul 7 2017, 7:02 PM
croick edited edge metadata.
  • use general function for enabling/disabling navigation buttons
  • undo icon changes
croick marked an inline comment as done.Jul 7 2017, 7:11 PM
croick added inline comments.
plugins/grepview/grepdialog.cpp
426

QTimer emits with a QPrivateSignal which conflicts with the bool argument

plugins/grepview/grepdialog.h
56–57

it is called as a slot by grepJobFinished, which now returns whether the job was successfully finished

plugins/grepview/grepoutputview.cpp
354

self checking does not work here with the asynchronous job, so enabling/disabling is done explicitly

croick edited the summary of this revision. (Show Details)Jul 7 2017, 7:12 PM
kfunk accepted this revision.Jul 8 2017, 11:06 PM

Will push

plugins/grepview/grepoutputview.cpp
407 ↗(On Diff #16322)

model()->rowCount() > 0 to avoid implicit int -> bool conversion

This revision is now accepted and ready to land.Jul 8 2017, 11:06 PM
This revision was automatically updated to reflect the committed changes.