Configure Keyboard Shortcuts #637

Pavithra-aa-Anand · 2025-02-19T01:27:25Z

Configured all the keyboard shortcuts of DLT Viewer

Signed-off by : RenuPriya Krishnamoorthy [email protected]

Configured all the keyboard shortcuts of DLT Viewer Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

src/mainwindow.cpp

vifactor · 2025-02-20T20:07:11Z

src/mainwindow.cpp

+void MainWindow::on_actionShortcuts_List_triggered(){
+    qDebug() <<"Shortcuts Triggered";
+
+    QDialog *shortcutDialog = new QDialog(this);


you are going to create new dialog every time this function is triggered, show it once and keep it on the heap whole app lifecycle with no possibility to show it again, effectively leaking the memory

Creating a static pointer will assure that QDialog is created only once and allocating a memory for it will handle memory leaking as well. May i know if your opinion on this, if i can proceed forward with this idea ?

Making it static will work, but then you'll need to wrap whole creation logic into if (dialog) to check if it was already created.

My approach (in order of preference) would be:

Create the dialog on the stack in this function

Have this dialog as the member of mainwindow class, create it once upon mainwindow creation

yes, I didnt wrap the whole logic in if(dialog). I have given a return if the dialog is already opened and the logic continues. I will try to implement your approach as well. Thank u for the input

Changes were made in mainwindow.h and merged the latest changes from upstream. Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

Update in QList and QDialog for summarising the shortcuts Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

vifactor · 2025-02-27T08:53:54Z

src/mainwindow.cpp

+    shortcutDialog.setWindowTitle("Shortcuts List");
+    shortcutDialog.resize(600, 400);
+
+    QTableView *table = new QTableView(&shortcutDialog);


with the way you changed it, you still "leak" memory for these allocations. My suggestion was to define shortcutDialog on the stack, but not as a class member

Okay. In that case, QDialog will be initialised inside the the function as "QDialog shortcutDialog(this)". Using this case dialog will be destroyed once its out of scope. Please let me know if my understanding correct ?

void MainWindow::on_actionShortcuts_List_triggered(){ QDialog shortcutDialog; [...] shortcutDialog.exec(); }

this is what I mean

vifactor · 2025-02-28T08:15:14Z

src/mainwindow.cpp

@@ -5547,6 +5548,69 @@ void MainWindow::on_action_menuHelp_Command_Line_triggered() {
                QDltOptManager::getInstance()->getHelpText());
 }

+void MainWindow::on_actionShortcuts_List_triggered(){
+    qDebug() <<"Shortcuts Triggered";


Suggested change

qDebug() <<"Shortcuts Triggered";

vifactor · 2025-02-28T08:15:28Z

src/mainwindow.cpp

+    layout->addWidget(table);
+
+    shortcutDialog.setLayout(layout);
+    shortcutDialog.show();


Suggested change

shortcutDialog.show();

shortcutDialog.exec();

vifactor · 2025-02-28T08:16:04Z

src/mainwindow.cpp

+
+    QTableView *table = new QTableView(&shortcutDialog);
+    table->setObjectName("Shortcuts Summarise Table");
+    QStandardItemModel *model = new QStandardItemModel(0, 2, this);


Suggested change

QStandardItemModel *model = new QStandardItemModel(0, 2, this);

QStandardItemModel *model = new QStandardItemModel(0, 2, &shortcutDialog);

vifactor · 2025-02-28T08:17:31Z

src/mainwindow.cpp

+    QFont BoldFont;
+    BoldFont.setBold(true);
+
+    QStandardItem *headerName = new QStandardItem("Name");


Suggested change

QStandardItem *headerName = new QStandardItem("Name");

QStandardItem *headerName = new QStandardItem("Action name");

vifactor · 2025-02-28T08:18:01Z

src/mainwindow.cpp

+    QFont BoldFont;
+    BoldFont.setBold(true);


Suggested change

QFont BoldFont;

BoldFont.setBold(true);

QFont headerFont;

headerFont.setBold(true);

vifactor · 2025-02-28T08:20:10Z

src/mainwindow.cpp

+    model->setHorizontalHeaderItem(1, headerFeature);
+
+    // Define shortcut list using QPair
+    QList<QPair<QString, QString>> shortcutsList = {


how can you ensure that this list of shortcuts remains valid over the time? For example, some contributor can come and change "Copy Payload", "Ctrl + P" to "Copy Payload", "Ctrl + J", then list will be outdated, but it would be hard to notice.

Shall I keep the table in an editable format, so that any contributor can edit it if incase there is any change ? Remaining changes I have already made.

my point is that you define shortcut combinations in two different unrelated places (where is Ctrl + P first mentioned?) with time this will lead to the issue, that this table contains outdated info because somebody did not fix it when changing shortcut combination. I do not see how making the table editable can help here. What might help is to define a central place where all combinations are defined like variables and use the corresponding variable in every place where it appears

for E.g :
QString Payload = "Ctrl + P";
QString New = " Ctrl + N";

The variables will be defined in a separate place and Payload, New, etc will be used for creating a summarised table. If any changes are made to the existing shortcuts this variable has to changed within the code and if a new shortcut is created a new variable has to be added. Is this the expected way of implementation ?

something like that, yes

const QString shortcutNew = "Ctrl + N"; const QString shortcutOpen = "Ctrl + O"; const QString shortcutSave = "Ctrl + S"; const QString shortcutClear = "Ctrl + E"; // Store shortcuts dynamically using a list of pairs QList<QPair<QString, QString>> shortcutsList = { {"New", shortcutNew}, {"Open", shortcutOpen}, {"Save As", shortcutSave}, {"Clear", shortcutClear} }; This is how the implementation looks. If any changes are made the variable has to be changed and then it has to be added to the QList. This process is more or less similar to the old one. In the old method it was one QList and here we have variables which are added to the QList. To display all the shortcuts in a table

, I want all the data in one container. Hence all the shortcuts has to be present in a consolidated containers like QList or QMap. May i know is it okay to go with this implementation ?

please provide a commit with the changes, it is hard to see what you are going to do exactly from the comment

Changes made for saving the list of shortcuts to variables and accessing them Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

Configure Keyboard Shortcuts

11e77ac

Configured all the keyboard shortcuts of DLT Viewer Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

vifactor requested changes Feb 20, 2025

View reviewed changes

vifactor added the enhancement label Feb 25, 2025

Resolved Merge Conflicts

c3ec584

Changes were made in mainwindow.h and merged the latest changes from upstream. Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

alexmucde added this to the Release v2.28.0 milestone Feb 25, 2025

Pavithra-aa-Anand and others added 2 commits February 26, 2025 09:52

Merge branch 'COVESA:master' into Feature-ConfigureKeyboardShortcuts

22cd787

Update in mainwindow.cpp

aec0909

Update in QList and QDialog for summarising the shortcuts Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

vifactor reviewed Feb 27, 2025

View reviewed changes

vifactor reviewed Feb 28, 2025

View reviewed changes

Pavithra-aa-Anand and others added 2 commits March 2, 2025 19:24

Merge branch 'COVESA:master' into Feature-ConfigureKeyboardShortcuts

1985da7

Changes in mainwindow.cpp

7e776e3

Changes made for saving the list of shortcuts to variables and accessing them Signed-off by : RenuPriya Krishnamoorthy <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure Keyboard Shortcuts #637

Configure Keyboard Shortcuts #637

Pavithra-aa-Anand commented Feb 19, 2025

vifactor Feb 20, 2025

Renu-Priya411 Feb 26, 2025

vifactor Feb 26, 2025

Renu-Priya411 Feb 26, 2025

vifactor Feb 27, 2025 •

edited

Loading

Renu-Priya411 Feb 27, 2025 •

edited

Loading

vifactor Feb 28, 2025

vifactor Feb 28, 2025

vifactor Feb 28, 2025

vifactor Feb 28, 2025

vifactor Feb 28, 2025

vifactor Feb 28, 2025

vifactor Feb 28, 2025

Renu-Priya411 Feb 28, 2025 •

edited

Loading

vifactor Feb 28, 2025 •

edited

Loading

Renu-Priya411 Feb 28, 2025

vifactor Feb 28, 2025

Renu-Priya411 Mar 4, 2025 •

edited

Loading

vifactor Mar 4, 2025 •

edited

Loading

	QStandardItemModel *model = new QStandardItemModel(0, 2, this);
	QStandardItemModel *model = new QStandardItemModel(0, 2, &shortcutDialog);

	QStandardItem *headerName = new QStandardItem("Name");
	QStandardItem *headerName = new QStandardItem("Action name");

Configure Keyboard Shortcuts #637

Are you sure you want to change the base?

Configure Keyboard Shortcuts #637

Conversation

Pavithra-aa-Anand commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vifactor Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Renu-Priya411 Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Renu-Priya411 Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

vifactor Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Renu-Priya411 Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

vifactor Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

vifactor Feb 27, 2025 •

edited

Loading

Renu-Priya411 Feb 27, 2025 •

edited

Loading

Renu-Priya411 Feb 28, 2025 •

edited

Loading

vifactor Feb 28, 2025 •

edited

Loading

Renu-Priya411 Mar 4, 2025 •

edited

Loading

vifactor Mar 4, 2025 •

edited

Loading