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

Timed messages in StatusBar #522

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jonathanrocher
Copy link
Collaborator

This PR exposes the ability to control the duration of a StatusBar message (qt only for now). I am creating this PR to start the discussion:

  • Would others agree that this is desirable?
  • How to handle Wx since its wx.StatusBar doesn't expose the same ability? What's pyface's policy for exposing toolkit specific capabilities? If that's not acceptable, a similar behavior could be implemented using a wx.Timer: https://stackoverflow.com/questions/23188042/wxpython-statusbar-temporary-text . That feels like a lot of complexity brought into pyface though.

@jonathanrocher jonathanrocher changed the title Timed messages in StatusBar Timed messages in StatusBar: feedback requested May 28, 2020
Copy link
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

This generally looks fine as an addition. I would prefer to measure time in seconds and then convert to integer milliseconds at the toolkit level.

For the wx case, we have timers exposed in a toolkit-independent fashion in pyface.timer, so it should be fairly easy to add the capability to remove after a fixed amount of time with a toolkit-independent timer object.

@@ -126,4 +129,5 @@ def _show_messages(self):
# decide to put all but the first message into separate widgets. We
# probably also need to extend the API to allow a "message" to be a
# widget - depends on what wx is capable of.
self.status_bar.showMessage(" ".join(self.messages))
self.status_bar.showMessage(" ".join(self.messages),
msecs=self.message_duration)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think duration in seconds would be better. That is what pyface.timer uses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed.

@jonathanrocher jonathanrocher changed the title Timed messages in StatusBar: feedback requested Timed messages in StatusBar Jul 11, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020

Codecov Report

Merging #522 into master will increase coverage by 0.95%.
The diff coverage is 62.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #522      +/-   ##
==========================================
+ Coverage   37.08%   38.04%   +0.95%     
==========================================
  Files         470      470              
  Lines       26027    26184     +157     
  Branches     3961     3975      +14     
==========================================
+ Hits         9652     9961     +309     
+ Misses      15948    15788     -160     
- Partials      427      435       +8     
Impacted Files Coverage Δ
pyface/__init__.py 82.60% <ø> (ø)
pyface/ui/wx/action/status_bar_manager.py 31.91% <31.25%> (+2.36%) ⬆️
pyface/ui/qt4/init.py 62.50% <33.33%> (ø)
pyface/ui/qt4/workbench/split_tab_widget.py 12.47% <50.00%> (ø)
pyface/qt/QtNetwork.py 75.00% <60.00%> (ø)
pyface/qt/QtOpenGL.py 75.00% <60.00%> (ø)
pyface/qt/QtSvg.py 75.00% <60.00%> (ø)
pyface/qt/QtTest.py 75.00% <60.00%> (ø)
pyface/qt/QtWebKit.py 68.42% <60.00%> (ø)
pyface/qt/QtCore.py 77.77% <62.50%> (ø)
... and 36 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c98d1d3...6f0dc56. Read the comment docs.

@jonathanrocher
Copy link
Collaborator Author

This is ready for a second look. WX support added, implementation redone for Qt, unit test added.

@jonathanrocher
Copy link
Collaborator Author

Test failure only happens with pyqt, not pyqt5 and is triggered by the following exception in the listener:

Exception occurred in traits notification handler.
Please check the log file for details.
Exception occurred in traits notification handler for object: <pyface.ui.qt4.action.status_bar_manager.StatusBarManager object at 0x127802888>, trait: messages, old value: ['goodbye world'], new value: []
Traceback (most recent call last):
  File "/Users/jrocher/.edm/envs/bootstrap3/lib/python3.6/site-packages/traits/trait_notifiers.py", line 381, in __call__
    self.handler(*args)
  File "/Users/jrocher/Projects/ETS_source/pyface/pyface/ui/qt4/action/status_bar_manager.py", line 100, in _messages_changed
    self._show_messages()
  File "/Users/jrocher/Projects/ETS_source/pyface/pyface/ui/qt4/action/status_bar_manager.py", line 142, in _show_messages
    self.status_bar.showMessage("  ".join(self.messages))
RuntimeError: wrapped C/C++ object of type QStatusBar has been deleted

Can't make sense of it. Any suggestions?

@kitchoi
Copy link
Contributor

kitchoi commented Jul 15, 2020

If the UI is closed before the timer gets to run its action, then when the timer's action is run, it would be accessing deleted objects. I think in the remove_status_bar method, we should stop and remove the timer if there is one. See if that resolves the error you saw?

@jonathanrocher
Copy link
Collaborator Author

Thanks for the suggestion @kitchoi . Didn't help with the test failure, but good suggestion anyway, so added.

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

Successfully merging this pull request may close these issues.

None yet

4 participants