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

Implement Stop button and progress bar #120

Merged
merged 11 commits into from
Dec 23, 2015
Merged

Implement Stop button and progress bar #120

merged 11 commits into from
Dec 23, 2015

Conversation

Shura1oplot
Copy link
Contributor

Added Stop button to the screen with status of running program. Depend on the OS it uses os.kill to terminale the process or TASKKILL.EXE windows program.

Added progress bar support. Two new Gooey() options added: progress_regex and progress_expr. The first one should be regular expression which is applied to each line of the program output. At least one group in the regex should be defined. If the regexp matchs a line, progress bar is updated. In case progress_expr is not set, the group should contain a float number between 0 and 100, otherwise progress_expr is applied as an argument to expr() function. All unnamed regexp groups are passed as x variable with list type, named groups are passed as variables. If a group value can be converted to float, it will be. For usage please refer to example_progress_bar_*.py.

gooey_progress_bar

@chriskiehl
Copy link
Owner

Hey @Shura1oplot!

This is awesome stuff! I'll start reviewing all these pull request shortly... Just a little buried under other projects right now, but I don't want you to think I'm ignoring these : )

@Shura1oplot
Copy link
Contributor Author

@chriskiehl I reverted "make quoting implicit" commit as we are discussing it in the different PR.

@@ -108,8 +171,8 @@ def required_section_complete(self):
def success_dialog(self):
self.show_dialog(i18n._("execution_finished"), i18n._('success_message'), wx.ICON_INFORMATION)

def error_dialog(self, error_msg):
self.show_dialog(i18n._('error_title'), i18n._('uh_oh').format(error_msg), wx.ICON_ERROR)
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you drop the error here (and in the translations file)?

Gooey should bubble up why the program failed.

Edit: Ah, I see now. The stop button pushes Gooey to the error screen, which would then show the stacktrace for the user purposefully stopping the program.

In the case of an actual error, the error message should still be shown to the user. So we need to either (a) conditionally show the stacktrace, or (b) route the stop button to a different screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chriskiehl Gooey catches stderr to show python exceptions in a error dialog. It doesn't work as designed when wrapped program uses stderr to print messages to user and stdout to optional data out. Lots of unix tools work like this, for example, wget and dd. Also it solves buffered output issue from frozen programs as stderr is always unbuffered.

Copy link
Owner

Choose a reason for hiding this comment

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

@Shura1oplot Hmm... this is interesting. I didn't know it was common to pump non error messages over stderr. You're exposing my noobish level of linux knowledge! ; )

After spending a few minutes reading up on the behavior, I yield to your explanation. I can see why programs would use stderr for things other than a simple error stream.

So... as you said, it seems Gooey's original behavior was indeed broken.

Good fix!

@Shura1oplot
Copy link
Contributor Author

@chriskiehl excuse me for a long delay, I have "busy season" on my work. I have fixed thing noted above.

@chriskiehl chriskiehl merged commit 59b70a8 into chriskiehl:master Dec 23, 2015
@chriskiehl
Copy link
Owner

Merged! Awesome work, man!

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

2 participants