Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to store settings in Manjaro Linux #17

Closed
mominul opened this issue Mar 28, 2018 · 3 comments
Closed

Fails to store settings in Manjaro Linux #17

mominul opened this issue Mar 28, 2018 · 3 comments

Comments

@mominul
Copy link
Member

mominul commented Mar 28, 2018

Topbar is always failing to store layout path setting.

@mominul
Copy link
Member Author

mominul commented Mar 30, 2018

Ah, found the culprit. 💥 🎉
For an unknown reason(??) Qt is always appending a & in Layout name setting.
So I have a workaround:

if (name.contains("&")) {
    name.replace("&", "");
}

I don't want to mess with this issue anymore and use this workaround 😠

@mominul
Copy link
Member Author

mominul commented Mar 30, 2018

Ah, the investigation hasn't been stopped folks!
If I am not getting things wrong, the main scenario happening is this:
In KDE environment, Qt automatically is inserting & in Layout names in the layout menu name so user can select the items pressing alt key.
If Layout name is &Bornona, user can select the method by pressing alt + b because the & symbol is present before B.
Long story short: the insertion of & character is messing up the whole thing. 💥

As I have committed the work workaround, there shouldn't be any problems for now but I will try to get myself an another solution.

@mominul
Copy link
Member Author

mominul commented Mar 31, 2018

I think I have figured out the main thing:
From Qt version 5.10, Qt automatically adds shortcuts to menu items. For that Qt includes a & character. So when we use QAction::text() function, we get a string including a & character and we mess all things up.

So this is not a KDE environment based issue. Most distributions use Qt 5.9 like Ubuntu 17.10 and so we didn't hit this issue. But Arch distributions use latest Qt and we hit this issue really hard 💥
I am working on a new commit which will simplify that workaround.

mominul added a commit that referenced this issue Mar 31, 2018
Reduce workaround code which I introduced for issue #17
Refactor TopBar::RefreshLayouts() to use efficient code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant