diff mbox series

[v2,2/6] kernel-shark-qt: Cosmetic modifications in KsQuickContextMenu

Message ID 20190109130945.28519-3-ykaradzhov@vmware.com (mailing list archive)
State Superseded
Headers show
Series Modifications toward KS 1.0 | expand

Commit Message

Yordan Karadzhov Jan. 9, 2019, 1:09 p.m. UTC 
The following minor modifications in KsQuickContextMenu are introduced:

1. Removes the "Apply to list/graph" check-boxes.
2. Swaps the position of the Show / Hide actions
3. Adds "clear all filters" action.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsQuickContextMenu.cpp | 63 +++++++---------------
 kernel-shark-qt/src/KsQuickContextMenu.hpp |  4 +-
 2 files changed, 23 insertions(+), 44 deletions(-)

Comments

Steven Rostedt Jan. 9, 2019, 4:33 p.m. UTC | #1 
On Wed,  9 Jan 2019 15:09:41 +0200
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> The following minor modifications in KsQuickContextMenu are introduced:
> 
> 1. Removes the "Apply to list/graph" check-boxes.
> 2. Swaps the position of the Show / Hide actions
> 3. Adds "clear all filters" action.
> 
This should actually be three separate patches. Even if they are minor,
each patch should only do one thing, with the exception of formatting
clean ups. Formatting clean ups can be done when the code that's being
cleaned up is being changed.

But the above are three distinct changes, and should be three distinct
patches. I know it's a little tedious to do it this way, but it's the
cleaner approach.

Can you resend with them separate?

-- Steve


> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark-qt/src/KsQuickContextMenu.cpp | 63 +++++++---------------
>  kernel-shark-qt/src/KsQuickContextMenu.hpp |  4 +-
>  2 files changed, 23 insertions(+), 44 deletions(-)
> 
>
Yordan Karadzhov Jan. 9, 2019, 4:38 p.m. UTC | #2 
On 9.01.19 г. 18:33 ч., Steven Rostedt wrote:
> On Wed,  9 Jan 2019 15:09:41 +0200
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> The following minor modifications in KsQuickContextMenu are introduced:
>>
>> 1. Removes the "Apply to list/graph" check-boxes.
>> 2. Swaps the position of the Show / Hide actions
>> 3. Adds "clear all filters" action.
>>
> This should actually be three separate patches. Even if they are minor,
> each patch should only do one thing, with the exception of formatting
> clean ups. Formatting clean ups can be done when the code that's being
> cleaned up is being changed.
> 
> But the above are three distinct changes, and should be three distinct
> patches. I know it's a little tedious to do it this way, but it's the
> cleaner approach.
> 
> Can you resend with them separate?
> 

Yes, I can do this.
Is there anything else that has to change in this series?

Thanks!
Yordan

> -- Steve
> 
> 
>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark-qt/src/KsQuickContextMenu.cpp | 63 +++++++---------------
>>   kernel-shark-qt/src/KsQuickContextMenu.hpp |  4 +-
>>   2 files changed, 23 insertions(+), 44 deletions(-)
>>
>>
Steven Rostedt Jan. 9, 2019, 4:47 p.m. UTC | #3 
On Wed, 9 Jan 2019 16:38:09 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> > Can you resend with them separate?
> >   
> 
> Yes, I can do this.
> Is there anything else that has to change in this series?

So far no, I'm still looking at it.

You can send the three patches (split from this one) as a separate
series, and not wait for this series to be finished for review.

Thanks!

(and also I didn't have to play with these patches before applying
them ;-)

-- Steve
diff mbox series

Patch

diff --git a/kernel-shark-qt/src/KsQuickContextMenu.cpp b/kernel-shark-qt/src/KsQuickContextMenu.cpp
index 6c9c9ef..728ecbd 100644
--- a/kernel-shark-qt/src/KsQuickContextMenu.cpp
+++ b/kernel-shark-qt/src/KsQuickContextMenu.cpp
@@ -62,7 +62,7 @@  KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
   _addTaskPlotAction(this),
   _removeCPUPlotAction(this),
   _removeTaskPlotAction(this),
-  _deselectAction(this)
+  _clearAllFilters(this)
 {
 	typedef void (KsQuickContextMenu::*mfp)();
 	QString taskName, parentName, descr;
@@ -87,37 +87,14 @@  KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
 
 	parentName = parent->metaObject()->className();
 
-	addSection("Pointer menu");
+	addSection("Pointer filter menu");
 
-	if (parentName == "KsTraceViewer") {
-		_graphSyncCBox =
-			KsUtils::addCheckBoxToMenu(this, "Apply filters to Graph");
-
-		connect(_graphSyncCBox,	&QCheckBox::stateChanged,
-					&KsUtils::graphFilterSync);
-
-		/*
-		 * By defauls the filters will be append to the List (Table)
-		 * only.
-		 */
-		KsUtils::listFilterSync(true);
-		KsUtils::graphFilterSync(false);
-		_graphSyncCBox->setChecked(false);
-	}
-
-	if (parentName == "KsTraceGraph" &&
-	    (graphs = dynamic_cast<KsTraceGraph *>(parent))) {
-		_listSyncCBox =
-			KsUtils::addCheckBoxToMenu(this, "Apply filters to List");
-
-		connect(_listSyncCBox,	&QCheckBox::stateChanged,
-					&KsUtils::listFilterSync);
-
-		/* By defauls the filters will be append to the Graph only. */
-		KsUtils::graphFilterSync(true);
-		KsUtils::listFilterSync(false);
-		_listSyncCBox->setChecked(false);
-	}
+	descr = "Show task [";
+	descr += taskName;
+	descr += "-";
+	descr += QString("%1").arg(pid);
+	descr += "] only";
+	lamAddAction(&_showTaskAction, &KsQuickContextMenu::_showTask);
 
 	descr = "Hide task [";
 	descr += taskName;
@@ -126,30 +103,30 @@  KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
 	descr += "]";
 	lamAddAction(&_hideTaskAction, &KsQuickContextMenu::_hideTask);
 
-	descr = "Show task [";
-	descr += taskName;
-	descr += "-";
-	descr += QString("%1").arg(pid);
+	descr = "Show event [";
+	descr += kshark_get_event_name_easy(_data->rows()[_row]);
 	descr += "] only";
-	lamAddAction(&_showTaskAction, &KsQuickContextMenu::_showTask);
+	lamAddAction(&_showEventAction, &KsQuickContextMenu::_showEvent);
 
 	descr = "Hide event [";
 	descr += kshark_get_event_name_easy(_data->rows()[_row]);
 	descr += "]";
 	lamAddAction(&_hideEventAction, &KsQuickContextMenu::_hideEvent);
 
-	descr = "Show event [";
-	descr += kshark_get_event_name_easy(_data->rows()[_row]);
-	descr += "] only";
-	lamAddAction(&_showEventAction, &KsQuickContextMenu::_showEvent);
+	if (parentName == "KsTraceViewer") {
+		descr = QString("Show CPU [%1] only").arg(cpu);
+		lamAddAction(&_showCPUAction, &KsQuickContextMenu::_showCPU);
+	}
 
 	descr = QString("Hide CPU [%1]").arg(_data->rows()[_row]->cpu);
 	lamAddAction(&_hideCPUAction, &KsQuickContextMenu::_hideCPU);
 
-	if (parentName == "KsTraceViewer") {
-		descr = QString("Show CPU [%1] only").arg(cpu);
-		lamAddAction(&_showCPUAction, &KsQuickContextMenu::_showCPU);
+	descr = "Clear all filters";
+	lamAddAction(&_clearAllFilters, &KsQuickContextMenu::_clearFilters);
 
+	addSection("Pointer plot menu");
+
+	if (parentName == "KsTraceViewer") {
 		descr = "Add [";
 		descr += taskName;
 		descr += "-";
diff --git a/kernel-shark-qt/src/KsQuickContextMenu.hpp b/kernel-shark-qt/src/KsQuickContextMenu.hpp
index f5a2a78..df8a65b 100644
--- a/kernel-shark-qt/src/KsQuickContextMenu.hpp
+++ b/kernel-shark-qt/src/KsQuickContextMenu.hpp
@@ -85,6 +85,8 @@  private:
 
 	QVector<int> _getFilterVector(tracecmd_filter_id *filter, int newId);
 
+	void _clearFilters() {_data->clearAllFilters();}
+
 	KsDataStore	*_data;
 
 	size_t		_row;
@@ -105,7 +107,7 @@  private:
 
 	QAction _removeTaskPlotAction;
 
-	QAction _deselectAction;
+	QAction _clearAllFilters;
 };
 
 /**