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

Add instructional prompt after loading project #469

Merged
merged 9 commits into from
Nov 26, 2020

Conversation

LeonMatthes
Copy link
Collaborator

Closes #456

@LinqLover
Copy link
Collaborator

Nice idea! Would it be helpful to print a message to stdout, too? When I'm running something from the terminal, I'll often expect the terminal to print any message when I'm required to take some action.

@LeonMatthes
Copy link
Collaborator Author

Nice idea! Would it be helpful to print a message to stdout, too? When I'm running something from the terminal, I'll often expect the terminal to print any message when I'm required to take some action.

I don't really see the reason for doing this.
If you're explicitly running in headful mode, I'd expect you to be able to see and pay attention to the running image.
Why display it on stdout as well?

@LinqLover
Copy link
Collaborator

Nice idea! Would it be helpful to print a message to stdout, too? When I'm running something from the terminal, I'll often expect the terminal to print any message when I'm required to take some action.

I don't really see the reason for doing this.
If you're explicitly running in headful mode, I'd expect you to be able to see and pay attention to the running image.
Why display it on stdout as well?

I was thinking of several modifications that can occur during test execution and break any part of the UI. E.g. a test could install a different ToolSet and forget to reset it or modify the visual appearance in some way that makes the UIManager prompt invisible. In this case, debugging would be easier if a hint was printed into the terminal, too.

Another reason is that the test execution can take a long time and while you are working in another program, the VM usually does not inform you when a dialog window appears. In contrast, many terminals can notify the user if a new portion of output was printed to a background terminal instance (for example, felixse/FluentTerminal#60 or microsoft/terminal#7955).

A third use case would be UI tests that disallow you to interact with the image which could have side effects on the tests. In such situations, I use to minimize the VM window while the tests are executing, so I would not see the VM prompt, again, but only any terminal message.

Do you see what I mean? :-)

@LinqLover
Copy link
Collaborator

Hm, I just realized that the same claim could also be made for all the progress bars that appear during the image setup/project loading stage. We could also duplicate all kinds of outputs in an extra ToolSet implementation and avoid any extra logic in this PR. In any case, it was not my intention to block this PR. :-)

@LeonMatthes
Copy link
Collaborator Author

Okay, you bring up some fair points. Most of them focus on the testing stage, however this PR does not change this.
All UI elements in this PR open after the installation and before the testing stage.

There actually is (or was at some point) a UIManager that uses the Command-line instead of Squeak GUI elements, so that one could be installed as well.
It's most likely best to try that route first, as it would also show other dialogs on the command line, and maybe even progress bars (not sure about that though).

@LinqLover
Copy link
Collaborator

All UI elements in this PR open after the installation and before the testing stage.

Ah, I missed that, fair point. There should be a rare number of installation scripts that corrupt the overall UI appearance.

There actually is (or was at some point) a UIManager that uses the Command-line instead of Squeak GUI elements, so that one could be installed as well.

Afaik there is no such implementation for Squeak, but one for Pharo. But a CommandLineUIManager is already on my wish-list, see http://forum.world.st/CommandLineUIManager-td5106538.html. I would be glad to both implement it for Squeak and integrate it in SCI. :-)

tl;dr: Please pardon the interruption and go ahead with your changes! :-)

Copy link
Member

@fniephaus fniephaus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for this!

I've changed SmalltalkCI>>#promptToProceedImpl so that proceeds by default, reverted redundant changes to various metadata files, and fixed the author initials.

I'll merge this once CI has passed again...

@fniephaus fniephaus merged commit 03d224e into hpi-swa:master Nov 26, 2020
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.

--headful walkback if image is not saved and missing docs
3 participants