Code review

[J.Hilk]

personally I would prefer multiple lambdas over the use of sender()

[sierdzio]

@qcoderpro said in Code review:

error: calling 'property' with incomplete return type 'QVariant'

#include <QVariant>

[qcoderpro]

@sierdzio

Probably you mean this:

void dialogTest::printButton()
{
     btnLabel->setText(sender()->property("text").toString());
}

Still with the ampersands!

@J-Hilk

personally I would prefer multiple lambdas over the use of sender()

I used both versions below:

connect(one, &QPushButton::clicked, this, [this]() { btnLabel->setText("&One"); });
 connect(two, &QPushButton::clicked, this,  [this]() { btnLabel->setText(two->text()); });
   

Still with the ampersands!

[qcoderpro]

Not a bug?
If it is, how to report that?

[mrjj]

@qcoderpro
Hi
Its not a bug. When the text contains a &, a shortcut is made for it but its not removed and not
shown. So when you ask for its text() , its still included.

[qcoderpro]

@mrjj
Hi,

and not shown. So when you ask for its text() , its still included.

It's what the programmer needs, to be included but "not shown". In the example above, it's shown, while it mustn't! It can clearly be a bug I think.

[Pl45m4]

@qcoderpro said in Code review:

It can clearly be a bug I think

The button text contains the & to create the shortcut. So why should btn->text() not return the full text with the ampersand?
btn->text() just returns the QString which is the displayed button text (without any pre-processing). Only QPushButton (i.e. its base class QAbtractButton) converts & into the shortcut functionality and for this you need the ampersand in your button text. The text-getter does not know about this, so the actual text with ampersand is returned.

Another example:
If you inspect HTML with a texteditor, you will see <br> and no line break, because the text editor doesn't interpret this as line break, it just shows the raw UTF8 (or whatever) encoded text.

[qcoderpro]

@Pl45m4

With this we have two options: either "not to use shortcuts" or "have the text including the ampersand"! Neither is wanted to me.
To me, the code behind "setText" should've been designed so that it could process the text and omit the ampersands when showing.

Do you have a better way?

[SGaist]

Hi,

You have here the text property. You set a text, you retrieve the same text. If you want something else, then it's up to you to a step that does transform the output the way you want it.

When setting a property, I am expecting the getter to return the exact same value. The special ampersand handling falls in that case. Since I want to use a shortcut then I will handle that fact in my code. If you want to not have to do that then use a normal text and create a QShortCut by hand.

[qcoderpro]

@SGaist

create a QShortCut by hand

I found it hard for my example.
Instead, used a slot:

.h :

...
private slots:
    QString removeAmpersand(QString);
...

.cpp:

...
connect(one, &QPushButton::clicked, this, [this]() {
       btnLabel->setText(removeAmpersand(one->text()));
   });

   connect(two, &QPushButton::clicked, this, [this]() {
       btnLabel->setText(removeAmpersand(two->text()));
   });
   connect(three, &QPushButton::clicked, this, [this]() {
       btnLabel->setText(removeAmpersand(three->text())); });
}

QString DialogTest::removeAmpersand(QString text)
{
    QString res = "";
    for(auto t : text)
     if(t != '&')
         res += t;
    return res;
}
...

Fine?

[SGaist]

That's not a slot just a normal method. QString::remove will make your code shorter.

[qcoderpro]

@SGaist

Yes, since it's not used in a connection, it's not a slot. Thanks. Solved.