Skip to content

Getting ready to Qt 6 (6/n). Replace QCoreApplication::quit() with QCoreApplication::exit(0) #586

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.

Sign up for GitHub

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

Jump to bottom
Merged
hebasto merged 1 commit into from
May 23, 2022
Merged

Getting ready to Qt 6 (6/n). Replace QCoreApplication::quit() with QCoreApplication::exit(0) #586

hebasto merged 1 commit into from
May 23, 2022

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Apr 16, 2022

Qt 5:

  • no behavior change.

See https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qcoreapplication.cpp?h=5.15#n2012:

void QCoreApplication::quit()
{
    exit(0);
}

Qt 6:

  • this change avoids sending a duplicated QEvent::Quit

We use QEvent::Quit to handle macOS dock menu events. Qt 6 uses QEvent::Quit more widely. We do not want a duplicated QEvent::Quit which fires Assert(node.args); in the Shutdown() function.

@hebasto hebasto added the Qt 6 Transition to Qt 6 label Apr 16, 2022
@hebasto hebasto mentioned this pull request Apr 16, 2022
Closed
@hebasto
Copy link
Member Author

hebasto commented Apr 16, 2022

Just for a quick reference, here is a (full?) list of ways to quit the GUI:

  1. Close the splash screen window
  2. Press q on the splash screen
  3. Close the main window (if the "Minimize on close" option is disabled)
  4. Menu File --> Exit. On macOS use an application menu
  5. Shortcut Ctrl+Q
  6. System tray icon menu Exit. On macOS use a Dock icon menu
  7. Run stop command in the Console tab
  8. Receive the stop RPC command (if the "Enable RPC server" option is checked)
  9. Press "Reset Options" button in the Options tab
  10. Ctrl+C in a shell which runs bitcoin-qt

w0xlt
w0xlt approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 4717ebe on Ubuntu 21.10, Qt 5.15.2.

Tested with all options mentioned in #586 (comment)

@hebasto
Copy link
Member Author

hebasto commented Apr 20, 2022

Updated 4717ebe -> e0930ad (pr586.01 -> pr586.02, diff):

  • added the same changes into the tests

promag
promag reviewed May 21, 2022
View reviewed changes
@hebasto
qt: Replace QCoreApplication::quit() with QCoreApplication::exit(0)
252f363 
Qt 5:
 - no behavior change

Qt 6:
 - this change avoids sending a duplicated `QEvent::Quit`
@hebasto
Copy link
Member Author

hebasto commented May 21, 2022

Updated e0930ad -> 252f363 (pr586.02 -> pr586.03, diff):

promag
promag approved these changes May 22, 2022
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 252f363.

@hebasto hebasto merged commit dfe11a1 into bitcoin-core:master May 23, 2022
@hebasto hebasto mentioned this pull request May 23, 2022
Closed
5 tasks
@hebasto hebasto deleted the 220416-quit branch May 23, 2022 07:06
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
@hebasto
Merge bitcoin-core/gui#586: Getting ready to Qt 6 (6/n). Replace `QCo…
98b8450
@bitcoin-core bitcoin-core locked and limited conversation to collaborators May 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@promag promag promag approved these changes

@w0xlt w0xlt w0xlt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Qt 6 Transition to Qt 6

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hebasto @promag @w0xlt