Package: x2goclient
Version: 4.1.2.3
Severity: wishlist
Tag: patch
There are a number of minor issues, oddities and limitations with the
current x2goclient gui which would be good to fix, resolve and improve on.
Like all projects however the core team have more important issues to
resolve. The code in its current state is difficult to understand and amend
and as it has no tests it is risky for developers to make functional
changes unless they spend a lot of time getting to know the code and doing
extensive manual testing.
Attached are 10 small refactorings which start extracting out some of the
code into smaller functions. As the code is separated out it becomes easier
to understand and change.
There are no functional changes in these patches, nor do they attempt or
claim to fix any known issues. On their own the benefit of these patches is
limited however the hope is that over time I will be able to submit more
refactorings as I learn more and then start fixing issues and improving the
UI. Extracting out functions is just one of many techniques that can be
used to make code cleaner but in my experience it's a good one to start
with. It should also be noted that sometimes code is extracted "as is" even
when it is clear further refactoring can be done to improve things, this is
by design in order to keep risk and size of patches to a minimum.
Cheers
Dave
From 549c2b6fadded11032368589f13be766b187c704 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 23:47:58 +0100
Subject: [PATCH 07/43] Refactor: Extract geometry setup
---
src/sessionbutton.cpp | 25 +++++++++++--------------
src/sessionbutton.h | 1 +
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 6073460..b25e674 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -82,13 +82,8 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
sid=id;
setupCommand();
-
- geomBox=createGeometryBox();
- geom=new QLabel ( this );
- geom->setMouseTracking ( true );
- geomIcon=new QLabel ( this );
- geomIcon->setMouseTracking ( true );
-
+ setupGeometry();
+
sessName=new QLabel ( this );
fnt=sessName->font();
fnt.setBold ( true );
@@ -277,15 +272,17 @@ void SessionButton::setupOptionsPalette( QPalette &cpal )
cpal.setColor ( QPalette::Window,backgroundColor );
}
-QComboBox* SessionButton::createGeometryBox()
+void SessionButton::setupGeometry()
{
- QComboBox* geomBox = new QComboBox( this );
- geomBox->setMouseTracking( true );
- geomBox->setFrame( false );
- geomBox->setEditable( false );
+ geomBox = new QComboBox ( this );
+ geomBox->setMouseTracking ( true );
+ geomBox->setFrame ( false );
+ geomBox->setEditable ( false );
geomBox->update();
-
- return geomBox;
+ geom=new QLabel ( this );
+ geom->setMouseTracking ( true );
+ geomIcon=new QLabel ( this );
+ geomIcon->setMouseTracking ( true );
}
void SessionButton::setupCommand(){
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index 6fca921..d68639c 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -92,6 +92,7 @@ private:
void setupOptionsPalette ( QPalette &cpal );
void setupCommand();
QComboBox* createGeometryBox();
+ void setupGeometry();
private slots:
void slotClicked();
--
2.27.0
From cb06190b79d3cdbcf7c574b345d3ed13a0a81087 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 23:57:36 +0100
Subject: [PATCH 08/43] Refactor: Extract sound setup
---
src/sessionbutton.cpp | 14 +++++++++-----
src/sessionbutton.h | 2 ++
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index b25e674..ff59e12 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -83,7 +83,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
setupCommand();
setupGeometry();
-
+
sessName=new QLabel ( this );
fnt=sessName->font();
fnt.setBold ( true );
@@ -96,10 +96,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
server=new QLabel ( this );
serverIcon=new QLabel ( this );
- sound=new QPushButton ( this );
- sound->setFlat ( true );
- sound->setMouseTracking ( true );
- soundIcon=new QLabel ( this );
+ setupSound();
QPalette optionsPalette = cmdBox->palette();
setupOptionsPalette ( optionsPalette );
@@ -295,6 +292,13 @@ void SessionButton::setupCommand(){
cmdBox->setFrame ( false );
}
+void SessionButton::setupSound(){
+ sound=new QPushButton ( this );
+ sound->setFlat ( true );
+ sound->setMouseTracking ( true );
+ soundIcon=new QLabel ( this );
+}
+
void SessionButton::slotClicked()
{
emit sessionSelected ( this );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index d68639c..962d796 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -93,6 +93,8 @@ private:
void setupCommand();
QComboBox* createGeometryBox();
void setupGeometry();
+ void setupSession();
+ void setupSound();
private slots:
void slotClicked();
--
2.27.0
From c1af33bd308e7baf68ee4fd98f2e13cbc719ec23 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sun, 17 May 2020 00:02:17 +0100
Subject: [PATCH 09/43] Refactor: Extract server setup
---
src/sessionbutton.cpp | 9 ++++++---
src/sessionbutton.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index ff59e12..46b0ed9 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -93,9 +93,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
icon=new QLabel ( this );
- server=new QLabel ( this );
- serverIcon=new QLabel ( this );
-
+ setupServer();
setupSound();
QPalette optionsPalette = cmdBox->palette();
@@ -292,6 +290,11 @@ void SessionButton::setupCommand(){
cmdBox->setFrame ( false );
}
+void SessionButton::setupServer(){
+ server=new QLabel ( this );
+ serverIcon=new QLabel ( this );
+}
+
void SessionButton::setupSound(){
sound=new QPushButton ( this );
sound->setFlat ( true );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index 962d796..c26ad44 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -95,6 +95,7 @@ private:
void setupGeometry();
void setupSession();
void setupSound();
+ void setupServer();
private slots:
void slotClicked();
--
2.27.0
From e9a15e0b134b5139176dcc8c73abe1bffd461724 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sun, 17 May 2020 00:06:01 +0100
Subject: [PATCH 10/43] Refactor: Extract edit button setup
---
src/sessionbutton.cpp | 8 ++++++--
src/sessionbutton.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 46b0ed9..0069a5d 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -105,8 +105,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
connect ( sound,SIGNAL ( clicked() ),this,
SLOT ( slot_soundClicked() ) );
- editBut=new QPushButton ( this );
- editBut->setMouseTracking ( true );
+ setupEditButton();
connect ( editBut,SIGNAL ( pressed() ),this,SLOT ( slotShowMenu() ) );
/* Load our edit button SVG file. */
@@ -302,6 +301,11 @@ void SessionButton::setupSound(){
soundIcon=new QLabel ( this );
}
+void SessionButton::setupEditButton(){
+ editBut=new QPushButton ( this );
+ editBut->setMouseTracking ( true );
+}
+
void SessionButton::slotClicked()
{
emit sessionSelected ( this );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index c26ad44..eb5f5f8 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -96,6 +96,7 @@ private:
void setupSession();
void setupSound();
void setupServer();
+ void setupEditButton();
private slots:
void slotClicked();
--
2.27.0
From 7aa4b1755f25fc68fcb697a0b9ddeebcf756972d Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 23:42:01 +0100
Subject: [PATCH 06/43] Refactor: Extract command setup
---
src/sessionbutton.cpp | 18 +++++++++++-------
src/sessionbutton.h | 1 +
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 0fb03f4..6073460 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -81,13 +81,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
sid=id;
- cmd=new QLabel ( this );
- cmd->setMouseTracking ( true );
- cmdIcon=new QLabel ( this );
- cmdIcon->setMouseTracking ( true );
- cmdBox=new QComboBox ( this );
- cmdBox->setMouseTracking ( true );
- cmdBox->setFrame ( false );
+ setupCommand();
geomBox=createGeometryBox();
geom=new QLabel ( this );
@@ -294,6 +288,16 @@ QComboBox* SessionButton::createGeometryBox()
return geomBox;
}
+void SessionButton::setupCommand(){
+ cmd=new QLabel ( this );
+ cmd->setMouseTracking ( true );
+ cmdIcon=new QLabel ( this );
+ cmdIcon->setMouseTracking ( true );
+ cmdBox=new QComboBox ( this );
+ cmdBox->setMouseTracking ( true );
+ cmdBox->setFrame ( false );
+}
+
void SessionButton::slotClicked()
{
emit sessionSelected ( this );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index e82d279..6fca921 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -90,6 +90,7 @@ private:
bool updated;
void setupOptionsPalette ( QPalette &cpal );
+ void setupCommand();
QComboBox* createGeometryBox();
private slots:
--
2.27.0
From f094a7a60e05eaa8cef56b1533f066da39d54b15 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 23:28:10 +0100
Subject: [PATCH 05/43] Refactor: Organise object creation by feature
Grouping components ready for further refactoring
---
src/sessionbutton.cpp | 42 +++++++++++++++++++++++-------------------
src/sessionbutton.h | 4 ++--
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index fd32c51..0fb03f4 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -81,38 +81,43 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
sid=id;
+ cmd=new QLabel ( this );
+ cmd->setMouseTracking ( true );
+ cmdIcon=new QLabel ( this );
+ cmdIcon->setMouseTracking ( true );
cmdBox=new QComboBox ( this );
cmdBox->setMouseTracking ( true );
cmdBox->setFrame ( false );
- QPalette optionsPalette = cmdBox->palette();
- setupOptionsPalette( optionsPalette );
-
- cmdBox->setPalette ( optionsPalette );
- geomBox=createGeometryBox( optionsPalette );
+ geomBox=createGeometryBox();
+ geom=new QLabel ( this );
+ geom->setMouseTracking ( true );
+ geomIcon=new QLabel ( this );
+ geomIcon->setMouseTracking ( true );
sessName=new QLabel ( this );
- sessStatus=new QLabel ( this );
fnt=sessName->font();
fnt.setBold ( true );
sessName->setFont ( fnt );
+
+ sessStatus=new QLabel ( this );
+
icon=new QLabel ( this );
- cmd=new QLabel ( this );
- cmd->setMouseTracking ( true );
- serverIcon=new QLabel ( this );
- geomIcon=new QLabel ( this );
- geomIcon->setMouseTracking ( true );
- cmdIcon=new QLabel ( this );
- cmdIcon->setMouseTracking ( true );
+
server=new QLabel ( this );
- geom=new QLabel ( this );
- geom->setMouseTracking ( true );
+ serverIcon=new QLabel ( this );
sound=new QPushButton ( this );
- soundIcon=new QLabel ( this );
- sound->setPalette ( optionsPalette );
sound->setFlat ( true );
sound->setMouseTracking ( true );
+ soundIcon=new QLabel ( this );
+
+ QPalette optionsPalette = cmdBox->palette();
+ setupOptionsPalette ( optionsPalette );
+ cmdBox->setPalette ( optionsPalette );
+ geomBox->setPalette ( optionsPalette );
+ sound->setPalette ( optionsPalette );
+
connect ( sound,SIGNAL ( clicked() ),this,
SLOT ( slot_soundClicked() ) );
@@ -278,14 +283,13 @@ void SessionButton::setupOptionsPalette( QPalette &cpal )
cpal.setColor ( QPalette::Window,backgroundColor );
}
-QComboBox* SessionButton::createGeometryBox( const QPalette &optionsPalette )
+QComboBox* SessionButton::createGeometryBox()
{
QComboBox* geomBox = new QComboBox( this );
geomBox->setMouseTracking( true );
geomBox->setFrame( false );
geomBox->setEditable( false );
geomBox->update();
- geomBox->setPalette( optionsPalette );
return geomBox;
}
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index e1a60f5..e82d279 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -89,8 +89,8 @@ private:
bool editable;
bool updated;
- void setupOptionsPalette( QPalette &cpal );
- QComboBox* createGeometryBox( const QPalette &optionsPalette );
+ void setupOptionsPalette ( QPalette &cpal );
+ QComboBox* createGeometryBox();
private slots:
void slotClicked();
--
2.27.0
From 2e6c4d980ef84366830c6f79dff9b65c0be34cef Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 22:44:06 +0100
Subject: [PATCH 04/43] Refactor: Remove unnecessary code
---
src/sessionbutton.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 93d4314..fd32c51 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -283,7 +283,6 @@ QComboBox* SessionButton::createGeometryBox( const QPalette &optionsPalette )
QComboBox* geomBox = new QComboBox( this );
geomBox->setMouseTracking( true );
geomBox->setFrame( false );
- geomBox->setEditable( true );
geomBox->setEditable( false );
geomBox->update();
geomBox->setPalette( optionsPalette );
--
2.27.0
From b940f4adffaca8433f5e0b47b9d8cc0776b04621 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 22:28:25 +0100
Subject: [PATCH 03/43] Refactor: Use constant for color
---
src/sessionbutton.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index d788340..93d4314 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -269,11 +269,13 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
SessionButton::~SessionButton()
{}
-void SessionButton::setupOptionsPalette(QPalette &cpal)
+void SessionButton::setupOptionsPalette( QPalette &cpal )
{
- cpal.setColor ( QPalette::Button,QColor ( 255,255,255 ) );
- cpal.setColor ( QPalette::Base,QColor ( 255,255,255 ) );
- cpal.setColor ( QPalette::Window,QColor ( 255,255,255 ) );
+ QColor backgroundColor = Qt::white;
+
+ cpal.setColor ( QPalette::Button,backgroundColor );
+ cpal.setColor ( QPalette::Base,backgroundColor );
+ cpal.setColor ( QPalette::Window,backgroundColor );
}
QComboBox* SessionButton::createGeometryBox( const QPalette &optionsPalette )
--
2.27.0
From 771dcc16792b8534bdf5be32cf575ea6566e4187 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Sat, 16 May 2020 22:22:49 +0100
Subject: [PATCH 02/43] Refactor: Extract geometry box setup
---
src/sessionbutton.cpp | 22 ++++++++++++++--------
src/sessionbutton.h | 1 +
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 6651833..d788340 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -89,14 +89,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
setupOptionsPalette( optionsPalette );
cmdBox->setPalette ( optionsPalette );
-
- geomBox=new QComboBox ( this );
- geomBox->setMouseTracking ( true );
- geomBox->setFrame ( false );
- geomBox->setEditable ( true );
- geomBox->setEditable ( false );
- geomBox->update();
- geomBox->setPalette ( optionsPalette );
+ geomBox=createGeometryBox( optionsPalette );
sessName=new QLabel ( this );
sessStatus=new QLabel ( this );
@@ -283,6 +276,19 @@ void SessionButton::setupOptionsPalette(QPalette &cpal)
cpal.setColor ( QPalette::Window,QColor ( 255,255,255 ) );
}
+QComboBox* SessionButton::createGeometryBox( const QPalette &optionsPalette )
+{
+ QComboBox* geomBox = new QComboBox( this );
+ geomBox->setMouseTracking( true );
+ geomBox->setFrame( false );
+ geomBox->setEditable( true );
+ geomBox->setEditable( false );
+ geomBox->update();
+ geomBox->setPalette( optionsPalette );
+
+ return geomBox;
+}
+
void SessionButton::slotClicked()
{
emit sessionSelected ( this );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index 8e21ccf..e1a60f5 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -90,6 +90,7 @@ private:
bool updated;
void setupOptionsPalette( QPalette &cpal );
+ QComboBox* createGeometryBox( const QPalette &optionsPalette );
private slots:
void slotClicked();
--
2.27.0
From 23c907e57b952511cba1b47d081418a05e52bf87 Mon Sep 17 00:00:00 2001
From: Dave Chamberlin-Kidd <d...@flamangoes.co.uk>
Date: Fri, 15 May 2020 21:56:57 +0100
Subject: [PATCH 01/43] Refactor: Extract palette setup
---
src/sessionbutton.cpp | 24 ++++++++++++++++--------
src/sessionbutton.h | 2 ++
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/sessionbutton.cpp b/src/sessionbutton.cpp
index 0a46ee6..6651833 100644
--- a/src/sessionbutton.cpp
+++ b/src/sessionbutton.cpp
@@ -84,12 +84,11 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
cmdBox=new QComboBox ( this );
cmdBox->setMouseTracking ( true );
cmdBox->setFrame ( false );
- QPalette cpal=cmdBox->palette();
- cpal.setColor ( QPalette::Button,QColor ( 255,255,255 ) );
- cpal.setColor ( QPalette::Base,QColor ( 255,255,255 ) );
- cpal.setColor ( QPalette::Window,QColor ( 255,255,255 ) );
- cmdBox->setPalette ( cpal );
+ QPalette optionsPalette = cmdBox->palette();
+ setupOptionsPalette( optionsPalette );
+
+ cmdBox->setPalette ( optionsPalette );
geomBox=new QComboBox ( this );
geomBox->setMouseTracking ( true );
@@ -97,7 +96,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
geomBox->setEditable ( true );
geomBox->setEditable ( false );
geomBox->update();
- geomBox->setPalette ( cpal );
+ geomBox->setPalette ( optionsPalette );
sessName=new QLabel ( this );
sessStatus=new QLabel ( this );
@@ -118,7 +117,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
sound=new QPushButton ( this );
soundIcon=new QLabel ( this );
- sound->setPalette ( cpal );
+ sound->setPalette ( optionsPalette );
sound->setFlat ( true );
sound->setMouseTracking ( true );
connect ( sound,SIGNAL ( clicked() ),this,
@@ -143,7 +142,7 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
editBut->setIconSize ( QSize ( 16,16 ) );
editBut->setFixedSize ( 24,24 );
editBut->setFlat ( true );
- editBut->setPalette ( cpal );
+ editBut->setPalette ( optionsPalette );
sessMenu=new QMenu ( this );
connect ( sessMenu,SIGNAL ( aboutToHide() ),this,
@@ -277,6 +276,13 @@ SessionButton::SessionButton ( ONMainWindow* mw,QWidget *parent, QString id )
SessionButton::~SessionButton()
{}
+void SessionButton::setupOptionsPalette(QPalette &cpal)
+{
+ cpal.setColor ( QPalette::Button,QColor ( 255,255,255 ) );
+ cpal.setColor ( QPalette::Base,QColor ( 255,255,255 ) );
+ cpal.setColor ( QPalette::Window,QColor ( 255,255,255 ) );
+}
+
void SessionButton::slotClicked()
{
emit sessionSelected ( this );
@@ -810,6 +816,7 @@ void SessionButton::slot_cmd_change ( const QString& command )
bool newRootless=rootless;
published=false;
QString cmd=command;
+
if ( command=="KDE" )
{
newRootless=false;
@@ -887,6 +894,7 @@ void SessionButton::slot_cmd_change ( const QString& command )
}
else
pix.load ( par->iconsPath ( "/16x16/X.png" ) );
+
cmdIcon->setPixmap ( pix );
X2goSettings st ( "sessions" );
diff --git a/src/sessionbutton.h b/src/sessionbutton.h
index d831e61..8e21ccf 100644
--- a/src/sessionbutton.h
+++ b/src/sessionbutton.h
@@ -89,6 +89,8 @@ private:
bool editable;
bool updated;
+ void setupOptionsPalette( QPalette &cpal );
+
private slots:
void slotClicked();
void slotEdit();
--
2.27.0
_______________________________________________
x2go-dev mailing list
x2go-dev@lists.x2go.org
https://lists.x2go.org/listinfo/x2go-dev
