[PATCH v2 0/5] Fix split view search and debug info navigation

[PATCH v2 0/5] Fix split view search and debug info navigation

[Mauro Carvalho Chehab]

This series fixes some issues with search while on split view and fix the
broken hyperlink navigation.

The first patches restore the pre-Qt5 conversion behavior. The last
one implements navigation on a different way. I opted to keep this one
in separate, as this one is a new feature.

Mauro Carvalho Chehab (5):
  kconfig: qconf: cleanup includes
  kconfig: qconf: ensure that only one item will be highlighted
  kconfig: qconf: make search fully work again on split mode
  kconfig: qconf: make debug links work again
  kconfig: qconf: navigate menus on hyperlinks

 scripts/kconfig/qconf.cc | 120 +++++++++++++++++++++++++++++----------
 scripts/kconfig/qconf.h  |  15 ++---
 2 files changed, 98 insertions(+), 37 deletions(-)

-- 
2.26.2

[PATCH v2 1/5] kconfig: qconf: cleanup includes

[Mauro Carvalho Chehab]

The usage of c-like include is deprecated on modern Qt
versions. Use the c++ style includes.

While here, remove uneeded and redundant ones, sorting
them on alphabetic order.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 scripts/kconfig/qconf.cc | 27 +++++++++------------------
 scripts/kconfig/qconf.h  | 14 +++++++-------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index c0ac8f7b5f1a..0e06afa08908 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -4,27 +4,18 @@
  * Copyright (C) 2015 Boris Barbulovski <bbarbulovski@gmail.com>
  */
 
-#include <qglobal.h>
-
-#include <QMainWindow>
-#include <QList>
-#include <qtextbrowser.h>
 #include <QAction>
+#include <QApplication>
+#include <QCloseEvent>
+#include <QDesktopWidget>
 #include <QFileDialog>
+#include <QLabel>
+#include <QLayout>
+#include <QList>
 #include <QMenu>
-
-#include <qapplication.h>
-#include <qdesktopwidget.h>
-#include <qtoolbar.h>
-#include <qlayout.h>
-#include <qsplitter.h>
-#include <qlineedit.h>
-#include <qlabel.h>
-#include <qpushbutton.h>
-#include <qmenubar.h>
-#include <qmessagebox.h>
-#include <qregexp.h>
-#include <qevent.h>
+#include <QMenuBar>
+#include <QMessageBox>
+#include <QToolBar>
 
 #include <stdlib.h>
 
diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
index c879d79ce817..d913a02967ae 100644
--- a/scripts/kconfig/qconf.h
+++ b/scripts/kconfig/qconf.h
@@ -3,17 +3,17 @@
  * Copyright (C) 2002 Roman Zippel <zippel@linux-m68k.org>
  */
 
-#include <QTextBrowser>
-#include <QTreeWidget>
-#include <QMainWindow>
+#include <QCheckBox>
+#include <QDialog>
 #include <QHeaderView>
-#include <qsettings.h>
+#include <QLineEdit>
+#include <QMainWindow>
 #include <QPushButton>
 #include <QSettings>
-#include <QLineEdit>
 #include <QSplitter>
-#include <QCheckBox>
-#include <QDialog>
+#include <QTextBrowser>
+#include <QTreeWidget>
+
 #include "expr.h"
 
 class ConfigView;
-- 
2.26.2

[PATCH v2 2/5] kconfig: qconf: ensure that only one item will be highlighted

[Mauro Carvalho Chehab]

The logic which is used by search to show a new link lacks
support for un-selecting a previously-selected item.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 scripts/kconfig/qconf.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 0e06afa08908..67dac2406a98 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1662,6 +1662,13 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
 	}
 
 	if (list) {
+		/* Clear an already-selected item */
+		if (!list->selectedItems().isEmpty()) {
+			item = (ConfigItem*)list->selectedItems().first();
+			if (item)
+				item->setSelected(false);
+		}
+
 		item = list->findConfigItem(menu);
 		if (item) {
 			item->setSelected(true);
-- 
2.26.2

[PATCH v2 3/5] kconfig: qconf: make search fully work again on split mode

[Mauro Carvalho Chehab]

When the search dialog box finds symbols/menus that match
the search criteria, it presents all results at the window.

Clicking on a search result should make qconf to navigate
to the selected item. This works on singleMode and on
fullMode, but on splitMode, the navigation is broken.

This was partially caused by an incomplete Qt5 conversion
and by the followup patches that restored the original
behavior.

When qconf is on split mode, it has to update both the
config and the menu views. Right now, such logic is broken,
as it is not seeking using the right structures.

On qconf, the screen is split into 3 parts:

	+------------+-------+
	|            |       |
	|   Config   | Menu  |
	|            |       |
	+------------+-------+
	|                    |
	|     ConfigInfo     |
	|                    |
	+--------------------+

On singleMode and on fullMode, the menuView is hidden, and search
updates only the configList (which controls the ConfigView).

On SplitMode, the search logic should detect if the variable is a
leaf or not. If it is a leaf, it should be presented at the menuView,
and both configList and menuList should be updated. Otherwise, just
the configList should be updated.

Link: https://lore.kernel.org/lkml/a98b0f0ebe0c23615a76f1d23f25fd0c84835e6b.camel@redhat.com/
Reported-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 scripts/kconfig/qconf.cc | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 67dac2406a98..85782da3e464 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1636,22 +1636,33 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
 			return;
 		list->setRootMenu(parent);
 		break;
-	case symbolMode:
+	case menuMode:
 		if (menu->flags & MENU_ROOT) {
-			configList->setRootMenu(menu);
+			menuList->setRootMenu(menu);
 			configList->clearSelection();
-			list = menuList;
-		} else {
 			list = configList;
+		} else {
 			parent = menu_get_parent_menu(menu->parent);
 			if (!parent)
 				return;
-			item = menuList->findConfigItem(parent);
+
+			/* Clear an already-selected item */
+			if (!configList->selectedItems().isEmpty()) {
+				item = (ConfigItem*)configList->selectedItems().first();
+				if (item)
+					item->setSelected(false);
+			}
+
+			/* Select the config view */
+			item = configList->findConfigItem(parent);
 			if (item) {
 				item->setSelected(true);
-				menuList->scrollToItem(item);
+				configList->scrollToItem(item);
 			}
-			list->setRootMenu(parent);
+
+			menuList->setRootMenu(parent);
+			menuList->clearSelection();
+			list = menuList;
 		}
 		break;
 	case fullMode:
-- 
2.26.2

[PATCH v2 4/5] kconfig: qconf: make debug links work again

[Mauro Carvalho Chehab]

The Qt5 conversion broke support for debug info links.

Restore the behaviour added by changeset
ab45d190fd4a ("kconfig: create links in info window").

The original approach were to pass a pointer for a data struct
via an <a href>. That doesn't sound a good idea, as, if something
gets wrong, the app could crash. So, instead, pass the name of
the symbol, and validate such symbol at the hyperlink handling
logic.

Link: https://lore.kernel.org/lkml/20200628125421.12458086@coco.lan/
Reported-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 scripts/kconfig/qconf.cc | 75 +++++++++++++++++++++++++++++++++++++---
 scripts/kconfig/qconf.h  |  1 +
 2 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 85782da3e464..49f0688fceb8 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -7,6 +7,7 @@
 #include <QAction>
 #include <QApplication>
 #include <QCloseEvent>
+#include <QDebug>
 #include <QDesktopWidget>
 #include <QFileDialog>
 #include <QLabel>
@@ -1012,7 +1013,7 @@ ConfigInfoView::ConfigInfoView(QWidget* parent, const char *name)
 	: Parent(parent), sym(0), _menu(0)
 {
 	setObjectName(name);
-
+	setOpenLinks(false);
 
 	if (!objectName().isEmpty()) {
 		configSettings->beginGroup(objectName());
@@ -1085,7 +1086,7 @@ void ConfigInfoView::menuInfo(void)
 			if (sym->name) {
 				head += " (";
 				if (showDebug())
-					head += QString().sprintf("<a href=\"s%p\">", sym);
+					head += QString().sprintf("<a href=\"s%s\">", sym->name);
 				head += print_filter(sym->name);
 				if (showDebug())
 					head += "</a>";
@@ -1094,7 +1095,7 @@ void ConfigInfoView::menuInfo(void)
 		} else if (sym->name) {
 			head += "<big><b>";
 			if (showDebug())
-				head += QString().sprintf("<a href=\"s%p\">", sym);
+				head += QString().sprintf("<a href=\"s%s\">", sym->name);
 			head += print_filter(sym->name);
 			if (showDebug())
 				head += "</a>";
@@ -1145,7 +1146,7 @@ QString ConfigInfoView::debug_info(struct symbol *sym)
 		switch (prop->type) {
 		case P_PROMPT:
 		case P_MENU:
-			debug += QString().sprintf("prompt: <a href=\"m%p\">", prop->menu);
+			debug += QString().sprintf("prompt: <a href=\"m%s\">", sym->name);
 			debug += print_filter(prop->text);
 			debug += "</a><br>";
 			break;
@@ -1217,13 +1218,74 @@ void ConfigInfoView::expr_print_help(void *data, struct symbol *sym, const char
 	QString str2 = print_filter(str);
 
 	if (sym && sym->name && !(sym->flags & SYMBOL_CONST)) {
-		*text += QString().sprintf("<a href=\"s%p\">", sym);
+		*text += QString().sprintf("<a href=\"s%s\">", sym->name);
 		*text += str2;
 		*text += "</a>";
 	} else
 		*text += str2;
 }
 
+void ConfigInfoView::clicked(const QUrl &url)
+{
+	QByteArray str = url.toEncoded();
+	const std::size_t count = str.size();
+	char *data = new char[count + 1];
+	struct symbol **result;
+	struct menu *m = NULL;
+	char type;
+
+	if (count < 1) {
+		qInfo() << "Clicked link is empty";
+		delete data;
+		return;
+	}
+
+	memcpy(data, str.constData(), count);
+	data[count] = '\0';
+	type = data[0];
+
+	/* Seek for exact match */
+	data[0] = '^';
+	strcat(data, "$");
+	result = sym_re_search(data);
+	if (!result) {
+		qInfo() << "Clicked symbol is invalid:" << data;
+		delete data;
+		return;
+	}
+
+	sym = *result;
+	if (type == 's') {
+		symbolInfo();
+		emit showDebugChanged(true);
+		free(result);
+		delete data;
+		return;
+	}
+
+	/* URL is a menu */
+	for (struct property *prop = sym->prop; prop; prop = prop->next) {
+		    if (prop->type != P_PROMPT && prop->type != P_MENU)
+			    continue;
+		    m = prop->menu;
+		    break;
+	}
+
+	if (!m) {
+		qInfo() << "Clicked menu is invalid:" << data;
+		free(result);
+		delete data;
+		return;
+	}
+
+	_menu = m;
+	menuInfo();
+
+	emit showDebugChanged(true);
+	free(result);
+	delete data;
+}
+
 QMenu* ConfigInfoView::createStandardContextMenu(const QPoint & pos)
 {
 	QMenu* popup = Parent::createStandardContextMenu(pos);
@@ -1497,6 +1559,9 @@ ConfigMainWindow::ConfigMainWindow(void)
 	helpMenu->addAction(showIntroAction);
 	helpMenu->addAction(showAboutAction);
 
+	connect (helpText, SIGNAL (anchorClicked (const QUrl &)),
+		 helpText, SLOT (clicked (const QUrl &)) );
+
 	connect(configList, SIGNAL(menuChanged(struct menu *)),
 		helpText, SLOT(setInfo(struct menu *)));
 	connect(configList, SIGNAL(menuSelected(struct menu *)),
diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
index d913a02967ae..a193137f2314 100644
--- a/scripts/kconfig/qconf.h
+++ b/scripts/kconfig/qconf.h
@@ -250,6 +250,7 @@ public slots:
 	void setInfo(struct menu *menu);
 	void saveSettings(void);
 	void setShowDebug(bool);
+	void clicked (const QUrl &url);
 
 signals:
 	void showDebugChanged(bool);
-- 
2.26.2

[PATCH v2 5/5] kconfig: qconf: navigate menus on hyperlinks

[Mauro Carvalho Chehab]

Instead of just changing the helper window to show a
dependency, also navigate to it at the config and menu
widgets.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 scripts/kconfig/qconf.cc | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 49f0688fceb8..d8870b15a178 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1232,7 +1232,6 @@ void ConfigInfoView::clicked(const QUrl &url)
 	char *data = new char[count + 1];
 	struct symbol **result;
 	struct menu *m = NULL;
-	char type;
 
 	if (count < 1) {
 		qInfo() << "Clicked link is empty";
@@ -1242,7 +1241,6 @@ void ConfigInfoView::clicked(const QUrl &url)
 
 	memcpy(data, str.constData(), count);
 	data[count] = '\0';
-	type = data[0];
 
 	/* Seek for exact match */
 	data[0] = '^';
@@ -1255,15 +1253,8 @@ void ConfigInfoView::clicked(const QUrl &url)
 	}
 
 	sym = *result;
-	if (type == 's') {
-		symbolInfo();
-		emit showDebugChanged(true);
-		free(result);
-		delete data;
-		return;
-	}
 
-	/* URL is a menu */
+	/* Seek for the menu which holds the symbol */
 	for (struct property *prop = sym->prop; prop; prop = prop->next) {
 		    if (prop->type != P_PROMPT && prop->type != P_MENU)
 			    continue;
@@ -1271,17 +1262,11 @@ void ConfigInfoView::clicked(const QUrl &url)
 		    break;
 	}
 
-	if (!m) {
+	if (!m)
 		qInfo() << "Clicked menu is invalid:" << data;
-		free(result);
-		delete data;
-		return;
-	}
+	else
+		emit menuSelected(m);
 
-	_menu = m;
-	menuInfo();
-
-	emit showDebugChanged(true);
 	free(result);
 	delete data;
 }
@@ -1750,6 +1735,7 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
 			item->setSelected(true);
 			list->scrollToItem(item);
 			list->setFocus();
+			helpText->setInfo(menu);
 		}
 	}
 }
-- 
2.26.2

[PATCH] kconfig: qconf: re-implement setSelected()

[Mauro Carvalho Chehab]

The default implementation for setSelected() at QTreeWidgetItem
allows multiple items to be selected.

Well, this should never be possible for the configItem lists.

So, implement a function that will automatically clean any
previous selection. This simplifies the logic somewhat, while
making the selection logic to be applied atomically, avoiding
future issues on that.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

This is a followup patch after the series I sent previously.

I noticed that there are still some situations where it might be
possible to have multiple lines selected at the config lists.

This patch addresses it on a different way, ensuring that it shouldn't
be possible anymore to have more than one lines selected.

 scripts/kconfig/qconf.cc | 22 ++++------------------
 scripts/kconfig/qconf.h  | 14 ++++++++++----
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index d8870b15a178..0ba373a3cdd4 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -537,7 +537,7 @@ void ConfigList::setRootMenu(struct menu *menu)
 	rootEntry = menu;
 	updateListAll();
 	if (currentItem()) {
-		currentItem()->setSelected(hasFocus());
+		setSelected(currentItem(), hasFocus());
 		scrollToItem(currentItem());
 	}
 }
@@ -865,7 +865,7 @@ void ConfigList::focusInEvent(QFocusEvent *e)
 
 	ConfigItem* item = (ConfigItem *)currentItem();
 	if (item) {
-		item->setSelected(true);
+		setSelected(item, true);
 		menu = item->menu;
 	}
 	emit gotFocus(menu);
@@ -1696,17 +1696,10 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
 			if (!parent)
 				return;
 
-			/* Clear an already-selected item */
-			if (!configList->selectedItems().isEmpty()) {
-				item = (ConfigItem*)configList->selectedItems().first();
-				if (item)
-					item->setSelected(false);
-			}
-
 			/* Select the config view */
 			item = configList->findConfigItem(parent);
 			if (item) {
-				item->setSelected(true);
+				configList->setSelected(item, true);
 				configList->scrollToItem(item);
 			}
 
@@ -1723,16 +1716,9 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
 	}
 
 	if (list) {
-		/* Clear an already-selected item */
-		if (!list->selectedItems().isEmpty()) {
-			item = (ConfigItem*)list->selectedItems().first();
-			if (item)
-				item->setSelected(false);
-		}
-
 		item = list->findConfigItem(menu);
 		if (item) {
-			item->setSelected(true);
+			list->setSelected(item, true);
 			list->scrollToItem(item);
 			list->setFocus();
 			helpText->setInfo(menu);
diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
index a193137f2314..fb9e9729266f 100644
--- a/scripts/kconfig/qconf.h
+++ b/scripts/kconfig/qconf.h
@@ -45,11 +45,17 @@ class ConfigList : public QTreeWidget {
 public:
 	ConfigList(ConfigView* p, const char *name = 0);
 	void reinit(void);
-	ConfigView* parent(void) const
-	{
-		return (ConfigView*)Parent::parent();
-	}
 	ConfigItem* findConfigItem(struct menu *);
+	ConfigView* parent(void) const
+	{
+		return (ConfigView*)Parent::parent();
+	}
+	void setSelected(QTreeWidgetItem *item, bool enable) {
+		for (int i = 0; i < selectedItems().size(); i++)
+			selectedItems().at(i)->setSelected(false);
+
+		item->setSelected(enable);
+	}
 
 protected:
 	void keyPressEvent(QKeyEvent *e);
-- 
2.26.2

Re: [PATCH v2 0/5] Fix split view search and debug info navigation

[Maxim Levitsky]

On Mon, 2020-06-29 at 11:35 +0200, Mauro Carvalho Chehab wrote:
> This series fixes some issues with search while on split view and fix the
> broken hyperlink navigation.
> 
> The first patches restore the pre-Qt5 conversion behavior. The last
> one implements navigation on a different way. I opted to keep this one
> in separate, as this one is a new feature.
> 
> Mauro Carvalho Chehab (5):
>   kconfig: qconf: cleanup includes
>   kconfig: qconf: ensure that only one item will be highlighted
>   kconfig: qconf: make search fully work again on split mode
>   kconfig: qconf: make debug links work again
>   kconfig: qconf: navigate menus on hyperlinks
> 
>  scripts/kconfig/qconf.cc | 120 +++++++++++++++++++++++++++++----------
>  scripts/kconfig/qconf.h  |  15 ++---
>  2 files changed, 98 insertions(+), 37 deletions(-)
> 
I tested the series, and it works really well here.
Thanks a lot for the patch 5, I indeed noted that 
I am not sure it ever worked like that but it does now, 
and it is a very good improvement in terms of usability.

There is still a tiny issue in patch 5 - sometimes already
selected items remain selected like it was in the search (now fixed).

Also I found another bug - clicking on '..' menu doesn't work:

For example select 'input device support', then 'keyboards' on the right,
then you can't get back. It also might be something that never worked,
and it doesn't bother me, I just note this for reference.

Thanks a lot for these fixes!
Best regards,
	Maxim Levitsky

Re: [PATCH v2 0/5] Fix split view search and debug info navigation

[Mauro Carvalho Chehab]

Em Mon, 29 Jun 2020 15:23:49 +0300
Maxim Levitsky <mlevitsk@redhat.com> escreveu:

> On Mon, 2020-06-29 at 11:35 +0200, Mauro Carvalho Chehab wrote:
> > This series fixes some issues with search while on split view and fix the
> > broken hyperlink navigation.
> > 
> > The first patches restore the pre-Qt5 conversion behavior. The last
> > one implements navigation on a different way. I opted to keep this one
> > in separate, as this one is a new feature.
> > 
> > Mauro Carvalho Chehab (5):
> >   kconfig: qconf: cleanup includes
> >   kconfig: qconf: ensure that only one item will be highlighted
> >   kconfig: qconf: make search fully work again on split mode
> >   kconfig: qconf: make debug links work again
> >   kconfig: qconf: navigate menus on hyperlinks
> > 
> >  scripts/kconfig/qconf.cc | 120 +++++++++++++++++++++++++++++----------
> >  scripts/kconfig/qconf.h  |  15 ++---
> >  2 files changed, 98 insertions(+), 37 deletions(-)
> >   
> I tested the series, and it works really well here.
> Thanks a lot for the patch 5, I indeed noted that 
> I am not sure it ever worked like that but it does now, 
> and it is a very good improvement in terms of usability.
> 
> There is still a tiny issue in patch 5 - sometimes already
> selected items remain selected like it was in the search (now fixed).

Yeah, I noticed that it was keeping something selected sometimes.
that's why I sent a followup patch:

	Date: Mon, 29 Jun 2020 12:57:45 +0200
	Subject: [PATCH] kconfig: qconf: re-implement setSelected()

Not sure why, but this one is not on lore yet.

> 
> Also I found another bug - clicking on '..' menu doesn't work:

I don't think that this is a bug. See below.

> For example select 'input device support', then 'keyboards' on the right,
> then you can't get back. It also might be something that never worked,
> and it doesn't bother me, I just note this for reference.

The logic at:

	void ConfigList::mouseReleaseEvent(QMouseEvent* e)

Which calls:

	emit parentSelected();

event when the "<-" button is pressed. This is used for the
configList when on singleMode, which makes sense:

	connect(configList, SIGNAL(parentSelected()),
		SLOT(goBack()));

There's no connect for menuList. So, this event is silently
ignored on split mode.

I guess the main question here is: what kind of action this were
supposed to do?

I mean, the "menuView" should always contain the leaf nodes,
as the rest of the menu hierarchy is at the configView.

So, what kind of action could be done in this case?

That's said, when looking into it, I noticed some dead code,
and some things that could be simplified.

I'll likely send a cleanup patch removing the dead code.

Thanks,
Mauro

Re: [PATCH v2 0/5] Fix split view search and debug info navigation

[Maxim Levitsky]

On Mon, 2020-06-29 at 16:46 +0200, Mauro Carvalho Chehab wrote:
> Em Mon, 29 Jun 2020 15:23:49 +0300
> Maxim Levitsky <mlevitsk@redhat.com> escreveu:
> 
> > On Mon, 2020-06-29 at 11:35 +0200, Mauro Carvalho Chehab wrote:
> > > This series fixes some issues with search while on split view and fix the
> > > broken hyperlink navigation.
> > > 
> > > The first patches restore the pre-Qt5 conversion behavior. The last
> > > one implements navigation on a different way. I opted to keep this one
> > > in separate, as this one is a new feature.
> > > 
> > > Mauro Carvalho Chehab (5):
> > >   kconfig: qconf: cleanup includes
> > >   kconfig: qconf: ensure that only one item will be highlighted
> > >   kconfig: qconf: make search fully work again on split mode
> > >   kconfig: qconf: make debug links work again
> > >   kconfig: qconf: navigate menus on hyperlinks
> > > 
> > >  scripts/kconfig/qconf.cc | 120 +++++++++++++++++++++++++++++----------
> > >  scripts/kconfig/qconf.h  |  15 ++---
> > >  2 files changed, 98 insertions(+), 37 deletions(-)
> > >   
> > I tested the series, and it works really well here.
> > Thanks a lot for the patch 5, I indeed noted that 
> > I am not sure it ever worked like that but it does now, 
> > and it is a very good improvement in terms of usability.
> > 
> > There is still a tiny issue in patch 5 - sometimes already
> > selected items remain selected like it was in the search (now fixed).
> 
> Yeah, I noticed that it was keeping something selected sometimes.
> that's why I sent a followup patch:
> 
> 	Date: Mon, 29 Jun 2020 12:57:45 +0200
> 	Subject: [PATCH] kconfig: qconf: re-implement setSelected()
> 
> Not sure why, but this one is not on lore yet.
> 
> > Also I found another bug - clicking on '..' menu doesn't work:
> 
> I don't think that this is a bug. See below.
> 
> > For example select 'input device support', then 'keyboards' on the right,
> > then you can't get back. It also might be something that never worked,
> > and it doesn't bother me, I just note this for reference.
> 
> The logic at:
> 
> 	void ConfigList::mouseReleaseEvent(QMouseEvent* e)
> 
> Which calls:
> 
> 	emit parentSelected();
> 
> event when the "<-" button is pressed. This is used for the
> configList when on singleMode, which makes sense:
> 
> 	connect(configList, SIGNAL(parentSelected()),
> 		SLOT(goBack()));
> 
> There's no connect for menuList. So, this event is silently
> ignored on split mode.
> 
> I guess the main question here is: what kind of action this were
> supposed to do?
> 
> I mean, the "menuView" should always contain the leaf nodes,
> as the rest of the menu hierarchy is at the configView.
> 
> So, what kind of action could be done in this case?

I also find it strange that sometimes the "menuView" (the area on the right)
contains non leaf nodes, but again this is really not an issue for me.

Thanks again for all the fixes!

Best regards,
	Maxim Levitsky

> 
> That's said, when looking into it, I noticed some dead code,
> and some things that could be simplified.
> 
> I'll likely send a cleanup patch removing the dead code.
> 
> Thanks,
> Mauro
> 

Re: [PATCH v2 5/5] kconfig: qconf: navigate menus on hyperlinks

[Masahiro Yamada]

On Mon, Jun 29, 2020 at 6:35 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Instead of just changing the helper window to show a
> dependency, also navigate to it at the config and menu
> widgets.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


Thanks, I think this is a nice improvement.

When I click a link, both the previous config
and the new one are highlighted.
Is this intentional behavior?

For example, put the following simple Kconfig
at the top-level.

From the info window of FOO,
click the 'select BAR' link,
and then both FOO and BAR are highlighted.

------------>8---------------
config FOO
       bool "foo"
       select BAR
       default y

config BAR
       bool "bar"
       default y
------------>8-----------------








> ---
>  scripts/kconfig/qconf.cc | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
> index 49f0688fceb8..d8870b15a178 100644
> --- a/scripts/kconfig/qconf.cc
> +++ b/scripts/kconfig/qconf.cc
> @@ -1232,7 +1232,6 @@ void ConfigInfoView::clicked(const QUrl &url)
>         char *data = new char[count + 1];
>         struct symbol **result;
>         struct menu *m = NULL;
> -       char type;
>
>         if (count < 1) {
>                 qInfo() << "Clicked link is empty";
> @@ -1242,7 +1241,6 @@ void ConfigInfoView::clicked(const QUrl &url)
>
>         memcpy(data, str.constData(), count);
>         data[count] = '\0';
> -       type = data[0];
>
>         /* Seek for exact match */
>         data[0] = '^';
> @@ -1255,15 +1253,8 @@ void ConfigInfoView::clicked(const QUrl &url)
>         }
>
>         sym = *result;
> -       if (type == 's') {
> -               symbolInfo();
> -               emit showDebugChanged(true);
> -               free(result);
> -               delete data;
> -               return;
> -       }
>
> -       /* URL is a menu */
> +       /* Seek for the menu which holds the symbol */
>         for (struct property *prop = sym->prop; prop; prop = prop->next) {
>                     if (prop->type != P_PROMPT && prop->type != P_MENU)
>                             continue;
> @@ -1271,17 +1262,11 @@ void ConfigInfoView::clicked(const QUrl &url)
>                     break;
>         }
>
> -       if (!m) {
> +       if (!m)
>                 qInfo() << "Clicked menu is invalid:" << data;
> -               free(result);
> -               delete data;
> -               return;
> -       }
> +       else
> +               emit menuSelected(m);
>
> -       _menu = m;
> -       menuInfo();
> -
> -       emit showDebugChanged(true);
>         free(result);
>         delete data;
>  }
> @@ -1750,6 +1735,7 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
>                         item->setSelected(true);
>                         list->scrollToItem(item);
>                         list->setFocus();
> +                       helpText->setInfo(menu);
>                 }
>         }
>  }
> --
> 2.26.2
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v2 5/5] kconfig: qconf: navigate menus on hyperlinks

[Masahiro Yamada]

On Mon, Jun 29, 2020 at 6:35 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Instead of just changing the helper window to show a
> dependency, also navigate to it at the config and menu
> widgets.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---


I noticed a regression.

Visit "Networking support",
and click "select: BPF"
from the window at the bottom.

Nothing happens except the following error message
in the console:

Clicked menu is invalid: ^BPF$


If I apply only 1-4 in this series,
this works fine.














>  scripts/kconfig/qconf.cc | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
> index 49f0688fceb8..d8870b15a178 100644
> --- a/scripts/kconfig/qconf.cc
> +++ b/scripts/kconfig/qconf.cc
> @@ -1232,7 +1232,6 @@ void ConfigInfoView::clicked(const QUrl &url)
>         char *data = new char[count + 1];
>         struct symbol **result;
>         struct menu *m = NULL;
> -       char type;
>
>         if (count < 1) {
>                 qInfo() << "Clicked link is empty";
> @@ -1242,7 +1241,6 @@ void ConfigInfoView::clicked(const QUrl &url)
>
>         memcpy(data, str.constData(), count);
>         data[count] = '\0';
> -       type = data[0];
>
>         /* Seek for exact match */
>         data[0] = '^';
> @@ -1255,15 +1253,8 @@ void ConfigInfoView::clicked(const QUrl &url)
>         }
>
>         sym = *result;
> -       if (type == 's') {
> -               symbolInfo();
> -               emit showDebugChanged(true);
> -               free(result);
> -               delete data;
> -               return;
> -       }
>
> -       /* URL is a menu */
> +       /* Seek for the menu which holds the symbol */
>         for (struct property *prop = sym->prop; prop; prop = prop->next) {
>                     if (prop->type != P_PROMPT && prop->type != P_MENU)
>                             continue;
> @@ -1271,17 +1262,11 @@ void ConfigInfoView::clicked(const QUrl &url)
>                     break;
>         }
>
> -       if (!m) {
> +       if (!m)
>                 qInfo() << "Clicked menu is invalid:" << data;
> -               free(result);
> -               delete data;
> -               return;
> -       }
> +       else
> +               emit menuSelected(m);
>
> -       _menu = m;
> -       menuInfo();
> -
> -       emit showDebugChanged(true);
>         free(result);
>         delete data;
>  }
> @@ -1750,6 +1735,7 @@ void ConfigMainWindow::setMenuLink(struct menu *menu)
>                         item->setSelected(true);
>                         list->scrollToItem(item);
>                         list->setFocus();
> +                       helpText->setInfo(menu);
>                 }
>         }
>  }
> --
> 2.26.2
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 4/5] kconfig: qconf: make debug links work again

[Masahiro Yamada]

On Mon, Jun 29, 2020 at 6:35 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> The Qt5 conversion broke support for debug info links.
>
> Restore the behaviour added by changeset
> ab45d190fd4a ("kconfig: create links in info window").
>
> The original approach were to pass a pointer for a data struct

"were" -> "was" ?


> via an <a href>. That doesn't sound a good idea, as, if something
> gets wrong, the app could crash. So, instead, pass the name of
> the symbol, and validate such symbol at the hyperlink handling
> logic.
>
> Link: https://lore.kernel.org/lkml/20200628125421.12458086@coco.lan/
> Reported-by: Maxim Levitsky <mlevitsk@redhat.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>



This patch does not cause segmentation fault any more.
Thanks.


> ---
>  scripts/kconfig/qconf.cc | 75 +++++++++++++++++++++++++++++++++++++---
>  scripts/kconfig/qconf.h  |  1 +
>  2 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
> index 85782da3e464..49f0688fceb8 100644
> --- a/scripts/kconfig/qconf.cc
> +++ b/scripts/kconfig/qconf.cc
> @@ -7,6 +7,7 @@
>  #include <QAction>
>  #include <QApplication>
>  #include <QCloseEvent>
> +#include <QDebug>
>  #include <QDesktopWidget>
>  #include <QFileDialog>
>  #include <QLabel>
> @@ -1012,7 +1013,7 @@ ConfigInfoView::ConfigInfoView(QWidget* parent, const char *name)
>         : Parent(parent), sym(0), _menu(0)
>  {
>         setObjectName(name);
> -
> +       setOpenLinks(false);
>
>         if (!objectName().isEmpty()) {
>                 configSettings->beginGroup(objectName());
> @@ -1085,7 +1086,7 @@ void ConfigInfoView::menuInfo(void)
>                         if (sym->name) {
>                                 head += " (";
>                                 if (showDebug())
> -                                       head += QString().sprintf("<a href=\"s%p\">", sym);
> +                                       head += QString().sprintf("<a href=\"s%s\">", sym->name);
>                                 head += print_filter(sym->name);
>                                 if (showDebug())
>                                         head += "</a>";
> @@ -1094,7 +1095,7 @@ void ConfigInfoView::menuInfo(void)
>                 } else if (sym->name) {
>                         head += "<big><b>";
>                         if (showDebug())
> -                               head += QString().sprintf("<a href=\"s%p\">", sym);
> +                               head += QString().sprintf("<a href=\"s%s\">", sym->name);
>                         head += print_filter(sym->name);
>                         if (showDebug())
>                                 head += "</a>";
> @@ -1145,7 +1146,7 @@ QString ConfigInfoView::debug_info(struct symbol *sym)
>                 switch (prop->type) {
>                 case P_PROMPT:
>                 case P_MENU:
> -                       debug += QString().sprintf("prompt: <a href=\"m%p\">", prop->menu);
> +                       debug += QString().sprintf("prompt: <a href=\"m%s\">", sym->name);
>                         debug += print_filter(prop->text);
>                         debug += "</a><br>";
>                         break;
> @@ -1217,13 +1218,74 @@ void ConfigInfoView::expr_print_help(void *data, struct symbol *sym, const char
>         QString str2 = print_filter(str);
>
>         if (sym && sym->name && !(sym->flags & SYMBOL_CONST)) {
> -               *text += QString().sprintf("<a href=\"s%p\">", sym);
> +               *text += QString().sprintf("<a href=\"s%s\">", sym->name);
>                 *text += str2;
>                 *text += "</a>";
>         } else
>                 *text += str2;
>  }
>
> +void ConfigInfoView::clicked(const QUrl &url)
> +{
> +       QByteArray str = url.toEncoded();
> +       const std::size_t count = str.size();
> +       char *data = new char[count + 1];
> +       struct symbol **result;
> +       struct menu *m = NULL;
> +       char type;
> +
> +       if (count < 1) {
> +               qInfo() << "Clicked link is empty";
> +               delete data;
> +               return;
> +       }
> +
> +       memcpy(data, str.constData(), count);
> +       data[count] = '\0';
> +       type = data[0];
> +
> +       /* Seek for exact match */
> +       data[0] = '^';
> +       strcat(data, "$");
> +       result = sym_re_search(data);
> +       if (!result) {
> +               qInfo() << "Clicked symbol is invalid:" << data;
> +               delete data;
> +               return;
> +       }
> +
> +       sym = *result;
> +       if (type == 's') {
> +               symbolInfo();
> +               emit showDebugChanged(true);
> +               free(result);
> +               delete data;
> +               return;
> +       }
> +
> +       /* URL is a menu */
> +       for (struct property *prop = sym->prop; prop; prop = prop->next) {
> +                   if (prop->type != P_PROMPT && prop->type != P_MENU)
> +                           continue;
> +                   m = prop->menu;
> +                   break;
> +       }
> +
> +       if (!m) {
> +               qInfo() << "Clicked menu is invalid:" << data;
> +               free(result);
> +               delete data;
> +               return;
> +       }
> +
> +       _menu = m;
> +       menuInfo();
> +
> +       emit showDebugChanged(true);
> +       free(result);
> +       delete data;
> +}
> +
>  QMenu* ConfigInfoView::createStandardContextMenu(const QPoint & pos)
>  {
>         QMenu* popup = Parent::createStandardContextMenu(pos);
> @@ -1497,6 +1559,9 @@ ConfigMainWindow::ConfigMainWindow(void)
>         helpMenu->addAction(showIntroAction);
>         helpMenu->addAction(showAboutAction);
>
> +       connect (helpText, SIGNAL (anchorClicked (const QUrl &)),
> +                helpText, SLOT (clicked (const QUrl &)) );
> +
>         connect(configList, SIGNAL(menuChanged(struct menu *)),
>                 helpText, SLOT(setInfo(struct menu *)));
>         connect(configList, SIGNAL(menuSelected(struct menu *)),
> diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
> index d913a02967ae..a193137f2314 100644
> --- a/scripts/kconfig/qconf.h
> +++ b/scripts/kconfig/qconf.h
> @@ -250,6 +250,7 @@ public slots:
>         void setInfo(struct menu *menu);
>         void saveSettings(void);
>         void setShowDebug(bool);
> +       void clicked (const QUrl &url);
>
>  signals:
>         void showDebugChanged(bool);
> --
> 2.26.2
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 5/5] kconfig: qconf: navigate menus on hyperlinks

[Mauro Carvalho Chehab]

Em Tue, 30 Jun 2020 12:36:27 +0900
Masahiro Yamada <masahiroy@kernel.org> escreveu:

> On Mon, Jun 29, 2020 at 6:35 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Instead of just changing the helper window to show a
> > dependency, also navigate to it at the config and menu
> > widgets.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---  
> 
> 
> I noticed a regression.
> 
> Visit "Networking support",
> and click "select: BPF"
> from the window at the bottom.
> 
> Nothing happens except the following error message
> in the console:
> 
> Clicked menu is invalid: ^BPF$
> 
> 
> If I apply only 1-4 in this series,
> this works fine.

Hmm... perhaps this is related to symbol visibility. If the clicked
link is not visible, it won't be able to present the Kconfig option.

I guess we can implement a mixed approach here: update just the
configInfo data if it can't find the clicked item as a menu, like
the enclosed patch.

I'll send an updated version with this hybrid approach.

Thanks,
Mauro

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index f49fbac91995..fd721c6c4c94 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1263,10 +1263,13 @@ void ConfigInfoView::clicked(const QUrl &url)
 		    break;
 	}
 
-	if (!m)
-		qInfo() << "Clicked menu is invalid:" << data;
-	else
+	if (!m) {
+		/* Symbol is not visible as a menu */
+		symbolInfo();
+		emit showDebugChanged(true);
+	} else {
 		emit menuSelected(m);
+	}
 
 	free(result);
 	delete data;