QPushButton's shortcut not work when the text change

ValentinMichelet

Hi,

First, why do you use a string list to store the button's text since obviously you only need one string?
Then, you should use const reference to avoid useless copy in your constructor:

CustomButton::CustomButton(QWidget *parent,QString text, QString const& shortcut)

That being said, what does your QString shortcut contain? If it's something like "CTRL+K", then you should try to wrap it with a QKeySequence:

setShortcut(QKeySequence(shortcut));

You can also print the shortcut with a qDebug in your ChangeNextText() method to check whether it changed or not.

ljc123456gogo

@ValentinMichelet

Thanks reply.

The text is assigned by user, so I don't know how many count of the Button name, so I use string list for the uniformity.
I know it will use copy construct, but the argument is temp variable , so I can't use reference.
Okay. I add QKeySequence to contain it now.

You give me a tip to debug the shortcut, I even not call to mind before.
And I try to qDebug:

void CustomButton::ChangeNextText()
{
	if (m_NameList.length() > 1)
	{
		m_NameList.push_back(m_NameList.front());
		m_NameList.pop_front();
		qDebug() << "before:" << shortcut();
		setText(m_NameList[0]);
		qDebug() << "later:" << shortcut();
	}
}

The amazing things is that the setText() method let the Shortcut doesn't work anymore in fact:

before: QKeySequence("Shift+T")
later: QKeySequence("")

It maybe impossible is the m_NameList[0] wrong, because I change m_NameList[0] to anyother string also will happen this question.
Isn't Qt Bug? Or have other something wrong?

SGaist

That's the correct behavior following the text property

ljc123456gogo

@SGaist
Oh! I see:

Any previous shortcut will be overwritten or cleared if no shortcut is defined by the text.

This property is really easy to make puzzle.
Maybe I should reset the shortcut after the text change.
Thanks.

SGaist

If you want to keep the same shortcut then yes.

ValentinMichelet

@ljc123456gogo said:

I know it will use copy construct, but the argument is temp variable , so I can't use reference.

I don't get it. Did you assume that the compiler would complain, or did you actually get a compilation error with a const& variable? In fact I'm not sure to understand what you mean by "temp variable".

class A {
public:
  void methodA(QString const& p_string) {
    /* do stuff with p_string, even assign it to a private member... */
  }
};

int main(int argc, char *argv[])
{
  A a;
  QString qstr("My string");
  a.methodA(qstr);

  return 0;
}

This works perfectly fine, and I think that qstr is a temp variable, right?

Anyway, I didn't know that calling setText() would remove the previous shortcut. This is so unintuitive. Does anyone know the reason behind this strange behavior?

ljc123456gogo

@ValentinMichelet
I said temp variable that means like:

int main(int argc, char *argv[])
{
  A a;
  a.methodA(QString("My string")); //When the fuction call end, this QString was "dead", it's memory maybe change by other pragram

  return 0;
}

And when the QString getout of it's scope, it also will "dead". It belong C++ knowledge.
Your code run normally because the QString until the pragram end it still exist, it is not what I mean.

ValentinMichelet

I still don't get it, sorry.

#include <QDebug>

class A {
public:
  void setString(QString const& p_string) {
    m_string = p_string;
  }
  QString m_string;
};

int main(int argc, char *argv[])
{
  A a;
  a.setString(QString("My string"));
  qDebug() << a.m_string;

  return 0;
}

This works like a charm.

ljc123456gogo

@ValentinMichelet
I say: it's memory maybe changed by other pragram later. Not immediate be changed. It would not have complie error. If you want to see error, you can let it wait some minutes and see it's data, it maybe changed.

ljc123456gogo

@ValentinMichelet
you also can see http://stackoverflow.com/questions/10540157/c-temporary-variable-lifetime

ValentinMichelet

@ljc123456gogo said:

@ValentinMichelet
you also can see http://stackoverflow.com/questions/10540157/c-temporary-variable-lifetime

This is a completely different issue. First, the reference is constant in my code, not in your link. Then, it's a parameter, not something you create inside your constructor.
The code I provided is a classic way to call a method with some parameters that you won't modify (read only) which insures that no copy will be done. There won't be any issue if you simply read it. It won't be overwritten when you are inside of your constructor. This is C++ knowledge.

Two problems can occur tho:

You const_cast it and modify it, but whatever happens, you deserve it.
You store the address of the temp, leading to a potential segfault.

Other than that, there is nothing wrong. In fact, it's a good practice to pass a const ref in a method/constructor as long as you do not have to modify it inside your method/constructor.

ljc123456gogo

@ValentinMichelet
Okay, I make some misunderstand, I overlook the "const" in your codes before. If doesn't have const, it's danger.
I should use const reference.

ValentinMichelet

@ljc123456gogo
I'm glad we both agree on that =)