Prevent illegal server info from GUI#1804
Prevent illegal server info from GUI#1804pljones wants to merge 1 commit intojamulussoftware:masterfrom
Conversation
pljones
force-pushed
the
bugfix/avoid-semicolons-in-serverinfo-gui
branch
2 times, most recently
from
ff4c045 to
3f01ec0
Compare
pljones
requested review from
a team,
ann0see and
softins
and removed request for
a team and
ann0see
softins
left a comment
softins
left a comment
There was a problem hiding this comment.
Comments made from reading, without trying to compile and execute.
src/serverdlg.cpp
Outdated
| BitmapSystemTrayInactive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreyArrow.png" ) ), | ||
| BitmapSystemTrayActive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreenArrow.png" ) ) | ||
| BitmapSystemTrayActive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreenArrow.png" ) ), | ||
| reSvrInfoIllegal ( QRegExp ( "[!;\n]*" ) ) |
There was a problem hiding this comment.
Is the ! an excluded character (don't allow !, ; or \n), or an attempt to invert the character class? If the latter, it should be ^.
This RE seems to be used for two conflicting purposes:
- When used to define a
QRegExpValidatorand applied to an edit box, it should be a RE that matches a valid input. So it would be[^;\n]*(or[^!;\n]*if the intent is also to exclude!), to which the validator adds implicit^and$at the start and end respectively. - When used by
QString::remove(), it should be the characters to remove, and so should be[;\n](or[!;\n]), which the opposite logic to the validator, and also the*is not needed asQString::remove()removes every occurrence anyway.
There was a problem hiding this comment.
! does't seem to be an invalid character (at least I doubt it's a problem). Waiting on @pljones
There was a problem hiding this comment.
I'd meant "!" to be "^" at one point...
src/serverdlg.cpp
Outdated
|
|
||
| edtServerName->setAccessibleName ( tr ( "Server name line edit" ) ); | ||
| const QRegExpValidator validatorServerName ( reSvrInfoIllegal, this ); | ||
| edtServerName->setValidator ( &validatorServerName ); |
src/serverdlg.cpp
Outdated
| { | ||
| // apply new setting to the server and update it | ||
| pServer->SetServerName ( strNewName ); | ||
| pServer->SetServerName ( QString ( strNewName ).remove ( reSvrInfoIllegal ) ); |
src/settings.h
Outdated
| { | ||
| public: | ||
| CServerSettings ( CServer* pNSerP, const QString& sNFiName ) : CSettings(), pServer ( pNSerP ) | ||
| CServerSettings ( CServer* pNSerP, const QString& sNFiName ) : CSettings(), pServer ( pNSerP ), reSvrInfoIllegal ( QRegExp ( "[!\n;]" ) ) |
There was a problem hiding this comment.
See comment above for serverdlg.cpp. I realise this particular RE is only used for QString::remove(), which would imply you do actually intend to remove ! characters too. (?)
There was a problem hiding this comment.
Was this PR raised to remain consistent with the CLI (which doesn't support \n and ;) or is it because these characters are problematic in any other way? Read the context: #1799 (comment)
src/serverdlg.cpp
Outdated
| BitmapSystemTrayInactive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreyArrow.png" ) ), | ||
| BitmapSystemTrayActive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreenArrow.png" ) ) | ||
| BitmapSystemTrayActive ( QString::fromUtf8 ( ":/png/LEDs/res/CLEDGreenArrow.png" ) ), | ||
| reSvrInfoIllegal ( QRegExp ( "[!;\n]*" ) ) |
There was a problem hiding this comment.
! does't seem to be an invalid character (at least I doubt it's a problem). Waiting on @pljones
pljones
force-pushed
the
bugfix/avoid-semicolons-in-serverinfo-gui
branch
2 times, most recently
from
09010c4 to
876e1d9
Compare
|
Hopefully it should be a lot better today. (I tested this one, too...) |
pljones
force-pushed
the
bugfix/avoid-semicolons-in-serverinfo-gui
branch
from
876e1d9 to
58450dc
Compare
pljones
force-pushed
the
bugfix/avoid-semicolons-in-serverinfo-gui
branch
from
58450dc to
95133d2
Compare
|
Going with a different approach, so this solution isn't needed. |
3 participants
No description provided.